Design Document for changing SerializerInterface

Table of Contents

• Current Contract for Serialization in Magento2
• Problem Description
• Extending JSON class contract - Option 1
• Add Constructor and Use Virtual Types for JSON - Option 2
• Introduce New dedicated Contract for JSON encoding/decoding operation - Option 3

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 class 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 (merged on the level of physical methods) so that it would not be possible to extend one contract not affecting another.
• Violation of Liskov Substitution principle
• 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

Another option is to add a constructor to JSON class which accepts $option parameter and provide an ability to configure Magento VirtualTypes passing different values for $option argument and pre-creating objects corresponding for each particular JSON encode/decode parameter when needed.

In the code these changes will look like:

/**
 * Serialize data to JSON, unserialize JSON encoded data
 *
 * @api
 * @since 100.2.0
 */
class Json implements SerializerInterface
{
    /**
     * @var int
     */
    private $option;
    
    /**
     * @param int $option
     */   
    public function __construct(int $option)
    {
        $this->option = $option;
    }
    
    
    /**
     * {@inheritDoc}
     * @since 100.2.0
     */
    public function serialize($data)
    {
        $result = json_encode($data, $this->option);
        if (false === $result) {
            throw new \InvalidArgumentException('Unable to serialize value.');
        }
        return $result;
    }

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

And DI.xml configuration

    <virtualType name="\Magento\Framework\Serialize\Serializer\Json\PrettyPrint" type="\Magento\Framework\Serialize\Serializer\Json">
        <arguments>
            <argument name="option" xsi:type="int"> JSON_PRETTY_PRINT </argument>
        </arguments>
    </virtualType>

Pros

• Easy to implement
• Backward Compatible for most of Magento 2 installations (only those who already inherit from JSON class and introduce own constructor would be affected)

Cons

• Not proper OOP implementation, as in fact here we are trying to make two different contracts (Serialization and JSON decode/encode) to follow identical semantics. Moreover, we are trying to neglect the fact that we have an independent JSON contract at all
• For any new option usage - VirtualType has to be introduced, there is a possibility that different extension developers would introduce own virtual types for the same option parameters, which can bring some mess

Introduce New dedicated Contract for JSON encoding-decoding operation - Option 3

For this option implementation, we don't introduce any changes neither to \Magento\Framework\Serialize\SerializerInterface nor for \Magento\Framework\Serialize\Serializer\Json classes.

But introduce a new interface for JSON encoding/decoding, something like this:

interface JsonEncoderInterface
{
    /**
     * Serialize data into string
     *
     * @param string|int|float|bool|array|null $data
     * @param int
     * @param int
     * @return string
     * @throws \InvalidArgumentException
     */
    public function encode($data, int $option, int $depth);

    /**
     * Unserialize the given string
     *
     * @param string $string
     * @param int
     * @param int
     * @return string|int|float|bool|array|null
     * @throws \InvalidArgumentException
     */
    public function decode($string, int $option, int $depth);
    
    //it's possible there are some other methods inside
}

And provide an implementation which would use low-level PHP json_encode/json_decode under the hood. But introducing of this interface will mean, that we need to refactor all the existing business logic which is currently dependent on JSON class, and substitute it with this new service JsonEncoderInterface, because having two contracts means that it's incorrect anymore to depend directly on JSON object (particular implementation) for JSON encoding purposes, that's a violation of polymorphism.

Pros

• The most correct from from OOP point of view as we have two independent interfaces for two different business operations
• Follow the Interface Segregation Principle (SOLID)
• Get us closer to the Desired State of Serializer / Encoding Design

Cons

• Backward Incompatible as introduces a lot of refactoring substituting existing direct usages of JSON object with newly introduced contract JsonEncoderInterface