Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type mismatch should throw an exception instead of coercing when deserializing JSON #561

Closed
reieRMeister opened this issue Mar 7, 2016 · 7 comments

Comments

@reieRMeister
Copy link

Hi,

when I try to deserialize the JSON string

{ "id" : "123" }

into an entity where id is defined as an integer the original type (string) gets juggled into an integer. The result is int(123). And there is no way for me to check if really an integer was handed over.

And it gets worse. When I try to deserialize

{ "id" : "fail123" }

it gets deserialized into int(0) which is totally wrong.

The only correct way to submit an integer should be

{ "id" : 123 }

Setting up a Type validator does not work because it gets called after the whole type juggling.

Is there a way to prevent the type juggling or let JMS serializer throw an exception on type mismatch?

Greetings

@maennchen
Copy link

maennchen commented Apr 7, 2016

EDIT: This solution does not work. Move on to my next comment.

I'd recommend checking that in the setter, which you can define via the Accessor annotation.

(Just guessing, hope it works that way.)

<?php
namespace JMS\Serializer\Tests\Serializer\yml;

use JMS\Serializer\Annotation\Accessor;
use JMS\Serializer\Annotation\Expose;
use JMS\Serializer\Annotation\Type;

class Something
{
    /**
     * @var int
     *
     * @Expose
     * @Type("integer")
     * @Accessor(getter="getId", setter="setId")
     */
    private $id;

    /**
     * @return int
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * @param int $id
     */
    public function setId($id)
    {
        // Do check here and maybe throw error
        $this->id = (int) $id;
    }
}

@maennchen
Copy link

You could also register a custom type & handler. (Also not tested, there may be typos etc.)

Use it with @Type("strict_integer").

<?php
namespace JMS\Serializer\Tests\Serializer\yml;

use JMS\Serializer\Context;
use JMS\Serializer\Handler\SubscribingHandlerInterface;
use JMS\Serializer\JsonDeserializationVisitor;
use JMS\Serializer\JsonSerializationVisitor;

class StrictIntegerHandler implements SubscribingHandlerInterface
{
    /**
     * Return format:
     *
     *      array(
     *          array(
     *              'direction' => GraphNavigator::DIRECTION_SERIALIZATION,
     *              'format' => 'json',
     *              'type' => 'DateTime',
     *              'method' => 'serializeDateTimeToJson',
     *          ),
     *      )
     *
     * The direction and method keys can be omitted.
     *
     * @return array
     */
    public static function getSubscribingMethods()
    {
        return [
            [
                'direction' => GraphNavigator::DIRECTION_DESERIALIZATION,
                'format' => 'json',
                'type' => 'strict_integer',
                'method' => 'deserializeStrictIntegerFromJSON',
            ],
            [
                'direction' => GraphNavigator::DIRECTION_SERIALIZATION,
                'format' => 'json',
                'type' => 'strict_integer',
                'method' => 'serializeStrictIntegerToJSON',
            ],
        ];
    }

    /**
     * @param JsonDeserializationVisitor $visitor
     * @param mixed                      $data
     * @param array                      $type
     * @return int
     */
    public function deserializeStrictIntegerFromJSON(JsonDeserializationVisitor $visitor, $data, array $type)
    {
        // Do checks on data
        return (int) $data;
    }

    /**
     * @param JsonSerializationVisitor $visitor
     * @param mixed                    $data
     * @param array                    $type
     * @param Context                  $context
     * @return int
     */
    public function serializeStrictIntegerFromJSON(JsonSerializationVisitor $visitor, $data, array $type, Context $context)
    {
        return $visitor->visitInteger($data, $type, $context);
    }
}

@reieRMeister
Copy link
Author

Thank you, I will have a look into it!

@reieRMeister
Copy link
Author

@maennchen I've just checked the first suggested solution. Unfortunately the value is already coerced into an integer in the setter. :(

I have my field set up like this in YAML:

foo:
    expose: true
    type: integer
    access_type: public_method
    getter: getFoo
    setter: setFoo

And the setter is written like this in the entity:

public function setFoo($foo)
{
    dump($foo); die;
    $this->foo = $foo;
    return $this;
}

The dump shows $foo is already of type integer:

"foo": 1234 turns into 1234
"foo": "1234" turns into 1234 (but should be "1234" and result in an error)
"foo": "f1234" turns into 0 (but should be "f1234" and result in an error)

I will now try your second suggestion.

@reieRMeister
Copy link
Author

Okay, @maennchen your second suggestion solves my problem. Using a custom deserialisation handler allows for pushing the data unchanged through and leave it to the validation to deal with wrong types. Thank you!

@schmittjoh To be honest in case of JSON the way JMS serializer deals with types just feels wrong. JSON knows about (some) types and coercing them will always lead to problems. So there should be at least a simple option to turn on strict handling of types or turn off type coercion.

@dnshio
Copy link

dnshio commented Jun 30, 2016

Primitives such as int, float and string are decoded correctly via the native json_decode and so should not be coerced. I now have to write my own custom handlers for each of these types just to bypass the incorrect default behaviour.

+1 @reieRMeister for bringing this up. +1 @maennchen for coming up with a solution

@TiMESPLiNTER
Copy link

TiMESPLiNTER commented Feb 22, 2017

I solved it by writing an own DeserializationVisitor which behaves correctly for my needs:

use JMS\Serializer\Context;
use JMS\Serializer\JsonDeserializationVisitor;
use JMS\Serializer\Exception\RuntimeException;

/**
 * Class TypeSafeJsonDeserializationVisitor
 */
class TypeSafeJsonDeserializationVisitor extends JsonDeserializationVisitor
{

    /**
     * @param mixed   $data
     * @param array   $type
     * @param Context $context
     * @return integer|mixed
     * @throws RuntimeException
     */
    public function visitInteger($data, array $type, Context $context)
    {
        $castedValue = parent::visitInteger($data, $type, $context);

        if ((string) $castedValue !== (string) $data) {
            return $data;
        }

        return $castedValue;
    }
}

Then I override the default JSON deserialization class in my services.yml:

parameters:
    jms_serializer.json_deserialization_visitor.class: TypeSafeJsonDeserializationVisitor

and then everything works as expected (at least for int).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants