Navigation Menu

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

[Config][XmlReferenceDumper] Prevent potential \TypeError #35537

Merged

Conversation

fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Jan 31, 2020

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets #34841
License MIT
Doc PR -

$key can be null and setName() is now typed with string. Fallbacking on an empty string restores the behavior (and output) of < 5.0.

However, that shows that's a case we don't handle (yet) properly. But that's another task 😃

@nicolas-grekas
Copy link
Member

We don't handle dumping null names, that's what you mean? Any suggestions?
Worth a test case?
Shouldn't this be merged in 4.4?

@fancyweb
Copy link
Contributor Author

fancyweb commented Feb 3, 2020

We don't handle dumping null names, that's what you mean?

We don't handle a PrototypedArrayNode that has no key attribute and that have another PrototypedArrayNode as its child.

Worth a test case?

I don't think so. A test should be added once we support it.

Shouldn't this be merged in 4.4?

No because the string arg type on setName() was added in 5.0.

@fancyweb fancyweb changed the base branch from 5.0 to 4.4 February 3, 2020 17:10
@fancyweb fancyweb force-pushed the config-fix-xml-reference-dumper branch from 998b53e to e8ba15e Compare February 3, 2020 17:10
@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

nicolas-grekas added a commit that referenced this pull request Feb 3, 2020
…(fancyweb)

This PR was merged into the 4.4 branch.

Discussion
----------

[Config][XmlReferenceDumper] Prevent potential \TypeError

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #34841
| License       | MIT
| Doc PR        | -

`$key` can be null and `setName()` is now typed with `string`. Fallbacking on an empty string restores the behavior (and output) of < 5.0.

However, that shows that's a case we don't handle (yet) properly. But that's another task 😃

Commits
-------

e8ba15e [Config][XmlReferenceDumper] Prevent potential \TypeError
@nicolas-grekas nicolas-grekas merged commit e8ba15e into symfony:4.4 Feb 3, 2020
@fancyweb fancyweb deleted the config-fix-xml-reference-dumper branch February 3, 2020 17:15
This was referenced Feb 29, 2020
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.

None yet

3 participants