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

Handle invalid XML better #2122

Merged
merged 3 commits into from
Mar 7, 2017
Merged

Handle invalid XML better #2122

merged 3 commits into from
Mar 7, 2017

Conversation

akrabat
Copy link
Member

@akrabat akrabat commented Jan 8, 2017

If the request's body contains invalid XML, don't throw an exception or
PHP warning. Instead, just return NULL from the medica parser.
This matches the way we handle JSON.

Fixes #2106

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 97.82% when pulling 2314455 on akrabat:silent-xml-body-parser into fb5356a on slimphp:3.x.

@silentworks
Copy link
Member

Is this not a BC break, since we are now suppressing a PHP error?

@akrabat
Copy link
Member Author

akrabat commented Jan 12, 2017

I think it's a bug fix. Will go with majority opinion though.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 97.82% when pulling be20f3f on akrabat:silent-xml-body-parser into fb5356a on slimphp:3.x.

@akrabat akrabat added this to the 3.8.0 milestone Jan 13, 2017
@codeguy
Copy link
Member

codeguy commented Mar 7, 2017

@akrabat Forward port to 4.x too?

@akrabat
Copy link
Member Author

akrabat commented Mar 7, 2017

@akrabat Forward port to 4.x too?

Yes.

@codeguy
Copy link
Member

codeguy commented Mar 7, 2017

Do we want a test for this?

@akrabat
Copy link
Member Author

akrabat commented Mar 7, 2017

Do we want a test for this?

Almost certainly :)

@codeguy
Copy link
Member

codeguy commented Mar 7, 2017

Will merge once you get a test added.

akrabat added 3 commits March 7, 2017 16:42
If the request's body contains invalid XML, don't throw an exception or
PHP warning. Instead, just return NULL from the medica parser.
This matches the way we handle JSON.

Fixes #2106
@akrabat
Copy link
Member Author

akrabat commented Mar 7, 2017

Rebased and added unit tests.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 98.149% when pulling 7e0556c on akrabat:silent-xml-body-parser into ef36faa on slimphp:3.x.

@akrabat
Copy link
Member Author

akrabat commented Mar 7, 2017

Success, @codeguy :)

@codeguy codeguy merged commit 7e0556c into slimphp:3.x Mar 7, 2017
@akrabat
Copy link
Member Author

akrabat commented Mar 7, 2017

Can this go into 4.x too, please?

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

Successfully merging this pull request may close these issues.

4 participants