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

Support whitelist for xml document types #183

Merged
merged 5 commits into from
Oct 3, 2012

Conversation

michelsalib
Copy link

As discussed here (5a9b7e6), here is a proposal for a whitelist support concerning xml deserialization with document types.

I updated the doc to give an example:

jms_serializer:
    visitors:
        xml:
            document_whitelist:
                - '<!DOCTYPE authorized SYSTEM "http://some_url">'

@michelsalib
Copy link
Author

@schmittjoh should I pull your automatic review ?

@michelsalib
Copy link
Author

Ho, and I forgot to update two old tests too...

@stof
Copy link
Contributor

stof commented Oct 2, 2012

@michelsalib most of the automatic changes are right. I'm not sure about the one modifying the call to getDeclaringClass

@michelsalib
Copy link
Author

Thanks @stof, I will update tomorrow

@@ -67,7 +68,13 @@ public function prepare($data)
$dom->loadXML($data);
foreach ($dom->childNodes as $child) {
if ($child->nodeType === XML_DOCUMENT_TYPE_NODE) {
throw new \InvalidArgumentException('Document types are not allowed.');
$internalSubset = str_replace(PHP_EOL, '', $child->internalSubset);
if (!in_array($internalSubset, $this->documentWhitelist)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add true as third arg here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure !

@michelsalib
Copy link
Author

@schmittjoh I wanted to have your feedback about the configuration part that I added. Especially about namings.

<visitors>
<xml>
<document_whitelist>
<!DOCTYPE authorized SYSTEM "http://some_url">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably doesn't work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely, actually I never used xml configuration in such a way, do you know how it should look like?

@michelsalib
Copy link
Author

@schmittjoh Done !

schmittjoh added a commit that referenced this pull request Oct 3, 2012
Support whitelist for xml document types
@schmittjoh schmittjoh merged commit 0d3f43b into schmittjoh:master Oct 3, 2012
@schmittjoh
Copy link
Owner

Thanks, merged!

@michelsalib
Copy link
Author

You are very welcome ;)

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