Skip to content

Conversation

@d-mitrofanov-v
Copy link
Contributor

… constructor in xml

Q A
Branch? 7.3
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #...
License MIT

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

alexandre-daubois and others added 30 commits July 18, 2025 09:57
* 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
fabpot and others added 7 commits October 4, 2025 09:31
…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
$this->assertSame('123', $obj->foo);
}

public function testCollectDenormalizationErrorsWithUnionConstructorTypesInXML()
Copy link
Member

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,
    ]);
}

Copy link
Contributor Author

@d-mitrofanov-v d-mitrofanov-v Oct 8, 2025

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.

@xabbuh
Copy link
Member

xabbuh commented Oct 7, 2025

looking at the low deps failure I guess we need a similar fix in validateAndDenormalizeLegacy()

@xabbuh
Copy link
Member

xabbuh commented Oct 7, 2025

I have checked again and in fact we need to make this change on the 6.4 as it suffers the same.

@d-mitrofanov-v d-mitrofanov-v changed the base branch from 7.3 to 6.4 October 8, 2025 04:08
@d-mitrofanov-v
Copy link
Contributor Author

Sorry everyone for accidental spam, my bad. Going to close this PR and redo it from scratch on 6.4 branch

nicolas-grekas added a commit that referenced this pull request Oct 8, 2025
… 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.