Skip to content

Design Document for changing SerializerInterface

Igor Miniailo edited this page Jul 17, 2018 · 19 revisions

Table of Contents

Current Contract for Serialization in Magento2

Since Magento 2.2 to make Magento code more secure and prevent PHP Object Injection, (OWASP) vulnerability it was decided to introduce additional abstraction layer into Magento Serialization mechanism, which would give an ability to substitute default implementation of PHP Serialization (unsafe because exploitable) in favour of JSON serialization. You can read more about these changes on Magento DevDocs.

The abstraction layer is represented by SerializerInterface with to implementations represented by Serialize (PHP Serialization) and Json (serialization based on json_encode/json_decode. Default implementation):

namespace Magento\Framework\Serialize;

/**
 * Interface for serializing
 *
 * @api
 * @since 100.2.0
 */
interface SerializerInterface
{
    /**
     * Serialize data into string
     *
     * @param string|int|float|bool|array|null $data
     * @return string|bool
     * @throws \InvalidArgumentException
     * @since 100.2.0
     */
    public function serialize($data);

    /**
     * Unserialize the given string
     *
     * @param string $string
     * @return string|int|float|bool|array|null
     * @throws \InvalidArgumentException
     * @since 100.2.0
     */
    public function unserialize($string);
}

Problem Description

Since we introduced JSON serialization, represented by \Magento\Framework\Serialize\Serializer\Json class, the class became an independent contract by its own for all the business logic which needs to make json_encode/json_decode, but not a "real" serialization of objects. For example, a business logic which handles AJAX requests coming from UI dependent directly on \Magento\Framework\Serialize\Serializer\Json, but not on SerializerInterface. And that's correct from business standpoint, there is no sense to use general interface if there is no business requirement to handle anything but JSON, for example, if Magento establish an integration with 3rd party system which protocol supports only JSON, it makes sense to use contract of \Magento\Framework\Serialize\Serializer\Json, but not SerializerInterface in this case.

Because of this JSON implementation has been marked in the codebase with @api annotation, which states that JSON class represents own contract.

/**
 * Serialize data to JSON, unserialize JSON encoded data
 *
 * @api
 * @since 100.2.0
 */
class Json implements SerializerInterface

But being used more and more not for Serialization purposes but as a layer of abstraction on top of low-level functions json_encode/json_decode it appears that the contract of JSON Serialization is too narrow to cover all the business cases where json_encode/json_decode could be applied.

Contract of json_encode:

string json_encode ( mixed $value [, int $options = 0 [, int $depth = 512 ]] )

Contract of json_decode:

mixed json_decode ( string $json [, bool $assoc = FALSE [, int $depth = 512 [, int $options = 0 ]]] )

So that community started to find a way how the option parameter could be passed to JSON Serialization. The high-level problem statement is that there are two contracts in fact:

  1. Serialization Contract
  2. JSON encoding/decoding contract which is broader than just pure serialization

But currently, in Magento codebase these two contracts are identical.

Extending JSON class contract - Option 1

This option implies the extension of JSON object interface to provide an ability to pass additional parameters for json_encode/decode but keeping the support of the Serialization contract along with that.

It could be implemented like this:

/**
 * Serialize data to JSON, unserialize JSON encoded data
 *
 * @api
 * @since 100.2.0
 */
class Json implements SerializerInterface
{
    /**
     * {@inheritDoc}
     * @since 100.2.0
     */
    public function serialize($data, int $option = 0)   //Optional parameter added 
    {
        $result = json_encode($data, $option);
        if (false === $result) {
            throw new \InvalidArgumentException('Unable to serialize value.');
        }
        return $result;
    }

    /**
     * {@inheritDoc}
     * @since 100.2.0
     */
    public function unserialize($string, int $option = 0) //Optional parameter added 
    {
        $result = json_decode($string, true, 512, $option);
        if (json_last_error() !== JSON_ERROR_NONE) {
            throw new \InvalidArgumentException('Unable to unserialize value.');
        }
        return $result;
    }
}

Pros

  • Easy to implement
  • Backward Compatible for most of Magento 2 installations (only those who already inherit from JSON object and extend its methods would be affected)

Cons

  • Not proper OOP implementation, as we have two different contracts combined into one object. Contracts are overlapping so that it would not be possible to extend one contract not affecting other.
  • Optional parameter in the interface is an additional "smell" that there is some violation of OOP

Add Constructor and Use Virtual Types for JSON - Option 2

MSI Documentation:

  1. Technical Vision. Catalog Inventory
  2. Installation Guide
  3. List of Inventory APIs and their legacy analogs
  4. MSI Roadmap
  5. Known Issues in Order Lifecycle
  6. MSI User Guide
  7. DevDocs Documentation
  8. User Stories
  9. User Scenarios:
  10. Technical Designs:
  11. Admin UI
  12. MFTF Extension Tests
  13. Weekly MSI Demos
  14. Tutorials

Clone this wiki locally