-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Serializer] Fix unexpected type in denormalization errors when float|int union type used in constructor with non numeric string in form-data request #61969
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
Conversation
* 7.2: [VarDumper] Fix dumping on systems that don't have a working iconv [Console] fix profiler with overridden `run()` method [Config] Fix support for attributes on class constants and enum cases
* 6.4: [FrameworkBundle] Add missing html5-allow-no-tld to XSD file
* 7.2: [FrameworkBundle] Add missing html5-allow-no-tld to XSD file
…ect as target (soyuka)
This PR was merged into the 7.3 branch.
Discussion
----------
[ObjectMapper] update promoted properties w/ an object as target
| Q | A
| ------------- | ---
| Branch? | 7.3
| Bug fix? | yes
| License | MIT
When using promoted properties properties would not be mapped when the target was an object, this test case was failing:
```php
public function testUpdateObjectWithConstructorPromotedProperties(ObjectMapperInterface $mapper)
{
$a = new PromotedConstructorSource(1, 'foo');
$b = new PromotedConstructorTarget(1, 'bar');
$v = $mapper->map($a, $b);
$this->assertSame($v->name, 'foo');
}
```
Commits
-------
759df62 [ObjectMapper] update promoted properties w/ an object as target
* 6.4: [Console] Windows Test Error - change completecar for windows for passing test Increase compatibility of debug toolbar compatibility with Turbo
…missing script (santysisi) This PR was merged into the 7.2 branch. Discussion ---------- [Lock] Fallback to `eval` when `LOAD` fails due to missing script | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#59795 | License | MIT * First attempts to execute the script using `evalSha`. * If the script is missing (NOSCRIPT), tries to load it using script `LOAD`. * If script `LOAD` is not supported, falls back to `eval`. Commits ------- 420d996 [Lock] Fallback to `eval` when `LOAD` fails due to missing script
* 6.4: [Console] Windows Test Error - change completecar for windows for passing test Increase compatibility of debug toolbar compatibility with Turbo
* 7.2: [Lock] Fallback to `eval` when `LOAD` fails due to missing script
* 6.4: [Form][Security][Validator] Review catalan translation
* 7.2: [Form][Security][Validator] Review catalan translation
This PR was merged into the 7.3 branch. Discussion ---------- [ObjectMapper] initialize lazy objects | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | Deprecations? | no | Issues | Fix symfony#61050 | License | MIT Commits ------- 3b15ab7 [ObjectMapper] initialize lazy objects
* 6.4: [FrameworkBundle] Don't use Email::VALIDATION_MODES in Configuration
* 7.2: [FrameworkBundle] Don't use Email::VALIDATION_MODES in Configuration
This PR was merged into the 7.4 branch. Discussion ---------- [ObjectMapper] add missing legacy group | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | | License | MIT Commits ------- f1940f9 add missing legacy group
…Interface::checkPostAuth() (nicolas-grekas) This PR was merged into the 7.2 branch. Discussion ---------- [Security] Fix added $token argument to UserCheckerInterface::checkPostAuth() | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT An added argument must be optional and must be declared using ``@param``, which will allow spotting all places that have to be updated in cascade. This PR fixes all that. Not sure how we messed up so badly in symfony#57773 😅 Commits ------- c819110 [Security] Fix added $token argument to UserCheckerInterface::checkPostAuth()
…o generate snapshots anymore (KevinVanSonsbeek) This PR was merged into the 7.2 branch. Discussion ---------- [Config] Fix GeneratedConfigTest not being able to generate snapshots anymore | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | Fix symfony#61139 | License | MIT PR symfony#57614 Removed multiple uses of `uniqid`. In the specific PR changes were made to the GeneratedConfigTest, to change how the directory was built for the config. This inadvertedly broke the generating of snapshot files, due to the outputDir being changed to a by reference argument, and always overwriting the value to a temp dir. (While the snapshot files should not be written to a temp dir) Commits ------- 8e8daf1 bugfix(symfony#61139): Only generate an outputDir if none is set.
* 7.2: [Security] Fix added $token argument to UserCheckerInterface::checkPostAuth() bugfix(symfony#61139): Only generate an outputDir if none is set.
… (nicolas-grekas) This PR was merged into the 7.3 branch. Discussion ---------- [ObjectMapper] Fix test using LazyObjectInterface | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT Instead of symfony#61191 and symfony#61192 Commits ------- 168b3a3 [ObjectMapper] Fix test using LazyObjectInterface
* 7.2: Add missing use statement for @param
…`JsonStreamer` (santysisi) This PR was merged into the 7.3 branch. Discussion ---------- [JsonStreamer] update `.gitattributes` paths for `JsonStreamer` | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | no | License | MIT `JsonEncoder` was renamed to `JsonStreamer`, but `.gitattributes` paths were not updated. Commits ------- d2a9974 [JsonStreamer] update `.gitattributes` paths for `JsonStreamer`
* 6.4: add back setAccessible() for PHP 7 compatibility Update BrevoRequestParser.php [Form][PhpUnitBridge] Remove usage of noop `ReflectionProperty::setAccessible()` fix compatibility with different Relay versions [Console] Fix JSON description for negatable input flags
…ble instead of string (Dean151) This PR was merged into the 7.3 branch. Discussion ---------- [TypeInfo] ArrayShape can resolve key type as callable instead of string | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no <!-- if yes, also update src/**/CHANGELOG.md --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Issues | N/A | License | MIT In PHP, array key type is either string or int. Although callable is not entirely false as long as it's a string callable, using callable as a key type for an array seems wrong, and it's unlikely to be what we expect. This is peculiarly true in environment when key name might collide with global function name. Proposed change is to check for int/string instead of resolving type. Because the input is an array, the key is already narrowed down to string|int in the input This bugfix is also a change of behavior, as callable was likely to be fed in keyTypes, and it won't longer occur with that change. Commits ------- 5b9d547 [TypeInfo] ArrayShape can resolve key type as callable instead of string
…age_logger_listener`" (kochen) This PR was merged into the 7.3 branch. Discussion ---------- [Mailer] Revert " Fix memory leak with `mailer.message_logger_listener`" As [discussed](symfony#61873) in profiler should not be forced in order to make use of mailer assertions. This reverts commit b63317d. | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no <!-- if yes, also update src/**/CHANGELOG.md --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Issues | Fix symfony#61873 <!-- prefix each issue number with "Fix #"; no need to create an issue if none exists, explain below --> | License | MIT Commits ------- 1f25522 Revert "[Mailer] Fix memory leak with `mailer.message_logger_listener`"
…mtarld) This PR was merged into the 7.3 branch. Discussion ---------- [TypeInfo] Fix type alias with template resolving | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#61715 | License | MIT Add templates to the type context before resolving type aliases. Also, too advantage of that PR to leverage data providers in `testCollectTypeAliases` Commits ------- 93b0e3d [TypeInfo] Fix type alias with template resolving
…type with templates (HypeMC) This PR was merged into the 7.3 branch. Discussion ---------- [PropertyInfo][TypeInfo] Fix resolving constructor type with templates | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT symfony#61752 broke the resolution of templated types. Consider the following code: ```php /** * `@template` T of object */ abstract class ParentClass { /** * `@param` list<T> $items */ public function __construct( public array $items, ) { } } /** * `@template` T of SomeClass * * `@extends` ParentClass<T> */ class ChildClass extends ParentClass { } $serializer->deserialize($jsonPayload, ChildClass::class, 'json'); ``` Before that PR, after deserialization, `$items` would correctly contain a list of `SomeClass` instances. Now it no longer works, `$items` is a list of associative arrays instead. Commits ------- 7ad51ea [PropertyInfo][TypeInfo] Fix resolving constructor type with templates
… constructor in xml
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithUnion.php
Outdated
Show resolved
Hide resolved
| $this->assertSame('123', $obj->foo); | ||
| } | ||
|
|
||
| public function testCollectDenormalizationErrorsWithUnionConstructorTypesInXML() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test looks too bloated to me especially given that we want to test something that goes wrong in the normalizer.
Let's use something like this (example for the ObjectNormalizerTest):
public function testConstructorWithNotMatchingUnionTypes()
{
$data = [
'value' => 'string',
'value2' => 'string',
];
$normalizer = new ObjectNormalizer(new ClassMetadataFactory(new AttributeLoader()), null, null, new PropertyInfoExtractor([], [new ReflectionExtractor()]));
$this->expectException(NotNormalizableValueException::class);
$this->expectExceptionMessage('The type of the "value" attribute for class "Symfony\Component\Serializer\Tests\Fixtures\DummyWithUnion" must be one of "float", "int" ("string" given).');
$normalizer->denormalize($data, DummyWithUnion::class, 'xml', [
AbstractNormalizer::ALLOW_EXTRA_ATTRIBUTES => false,
]);
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, your test is much better. Used it and deleted mine.
|
looking at the low deps failure I guess we need a similar fix in |
|
I have checked again and in fact we need to make this change on the |
|
Sorry everyone for accidental spam, my bad. Going to close this PR and redo it from scratch on 6.4 branch |
… when float|int union type used in constructor with non numeric string in form-data request (d-mitrofanov-v) This PR was merged into the 6.4 branch. Discussion ---------- [Serializer] Fix unexpected type in denormalization errors when float|int union type used in constructor with non numeric string in form-data request | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no <!-- if yes, also update src/**/CHANGELOG.md --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Issues | Fix #... <!-- prefix each issue number with "Fix #"; no need to create an issue if none exists, explain below --> | License | MIT Remake of [broken pull request](#61969), sorry once again for that, that fixes getting empty types in expectedTypes of NotNormalizableValueException if constructor properties have int|float union types and request has form-data field with non numeric string Commits ------- d77fdd9 fix unexpected type in denormalization errors when union type used in constructor in xml
… constructor in xml
Fixes getting empty types in expectedTypes of NotNormalizableValueException if constructor properties have int|float union types and request has form-data field with non numeric string