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

Serialization of union type will return bool(true) instead of int(1) #1573

Open
fabianerni opened this issue Nov 26, 2024 · 1 comment · May be fixed by #1575
Open

Serialization of union type will return bool(true) instead of int(1) #1573

fabianerni opened this issue Nov 26, 2024 · 1 comment · May be fixed by #1575
Labels
Union Types Support for Union Types

Comments

@fabianerni
Copy link

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no

Steps required to reproduce the problem

  1. Create an entity with a (virtual) property with a union (return) type definition (ex: function getValue(): int|string|bool|float)
  2. Serialize the entity/property to JSON and return int(1) from getValue(). Expect int(1) in the serialized data
  3. Serializer will detect a boolean value instead of an int and serialize bool(true)

Expected Result

  • For serialization, the returned types should be handled strict. So if an int(1) is returned, it should be int(1) and not bool(true)

Actual Result

  • int(1) is detected as primitive type bool and will get serialized as bool

Related Code

  • UnionHandler::testPrimitive()
  • TypePropertiesDriver::reorderTypes()

Problem

  • Despite the doc block on TypePropertiesDriver::reorderTypes(), which says int should come before bool, bool is sorted before int and will be tested first in UnionHandler::testPrimitive().
  • UnionHandler::testPrimitive() uses dynamic type conversions ((string) and (bool)) to check for the types. Therefore, int(1) will first get checked for bool and the dynamic conversion will result in a positive test.

Bug

Will result in a positive match for bool type for int(1) and int(0)
(string) (bool) $data === (string) $data
https://github.com/schmittjoh/serializer/blob/master/src/Handler/UnionHandler.php#L134

Suggested Solution

(Sorry for not providing a pull request. But I'm not that experienced with the internals of the serializer compontent and such a change might have unexpected results I'm not aware of).

  • Change the type order in TypePropertiesDriver::reorderTypes(), so int is checked before bool
  • Make the checks in UnionHandler::testPrimitive() more strict, at least for serialization, since here we can expect exact types coming from the entities
@scyzoryck
Copy link
Collaborator

@fabianerni thanks for report! The proposal for changes makes sense - could you please make pull request with the changes please? I can help in case of issues.

Best, Marcin.

fabianerni added a commit to teleboy/serializer that referenced this issue Dec 2, 2024
detect int before boolean and exclude int(1) from boolean detection
fixes schmittjoh#1573
@fabianerni fabianerni linked a pull request Dec 2, 2024 that will close this issue
@scyzoryck scyzoryck added the Union Types Support for Union Types label Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Union Types Support for Union Types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants