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.

Sign up for GitHub

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

Jump to bottom

Do not append missing XML declaration in formatted responses #1183

Closed
wants to merge 7 commits into from
Closed

Do not append missing XML declaration in formatted responses #1183

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
932ca74
Do not append missing XML declaration in formatted responses
Oct 15, 2021
d44a623
Fixed UnboundLocalError
Oct 15, 2021
05dd780
corrected fixture files and simpliy logic
Oct 15, 2021
39c6399
Added Deleted new lines
Oct 18, 2021
6e32d11
Update httpie/output/formatters/xml.py
Oct 18, 2021
45382db
Made Prolog same as document if present
Oct 23, 2021
73038e0
Added Fixture file with simple prolog
Oct 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions httpie/output/formatters/xml.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,20 @@ def format_body(self, body: str, mime: str):

from xml.parsers.expat import ExpatError
from defusedxml.common import DefusedXmlException

this-is-r-gaurav marked this conversation as resolved.
Show resolved Hide resolved
this-is-r-gaurav marked this conversation as resolved.
Show resolved Hide resolved
try:
parsed_body = parse_xml(body)
except ExpatError:
pass # Invalid XML, ignore.
except DefusedXmlException:
pass # Unsafe XML, ignore.
else:
else:
has_declaration = body.startswith('<?xml version')
body = pretty_xml(parsed_body,
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps something less invasive could be used? An idea:

        try: ...
        else:
            has_declaration = body.startswith('<?xml version')
            body = pretty_xml(parsed_body,
                              encoding=parsed_body.encoding,
                              indent=self.format_options['xml']['indent'],
                              standalone=parsed_body.standalone)
            if not has_declaration:
                body = body[body.index('\n') + 1:]

         return body

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes seems more intuitive. I was not aware about the ordering of prolog attributes matter. so i choose to find out what is the first tag in original xml document.
But i tried one test case:
If unformatted file is this

<?xml version="1.0"?>
<root><test>testValue</test></root>

what would be expected, this

<?xml version="1.0"?>
<root>
  <test>testValue</test>
</root>

or

<?xml version="1.0" encoding="utf-8"?>
<root>
  <test>testValue</test>
</root>

Copy link
Contributor

@BoboTiG BoboTiG Oct 18, 2021 •
edited

Choose a reason for hiding this comment

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

xmlminidom will set the encoding. I think it is good as-is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the previous answer. I did a mistake.
HTTPie should not alter the response content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh okay, sorry been busy with personal work will try to fix that as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BoboTiG addressed the above test case as well.
But assuming if test case file is:

<root><test>testValue</test></root>

Output will be

<?xml version="1.0" ?>
<root>
  <test>testValue</test>
</root>

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that input, the output should be:

<root>
  <test>testValue</test>
</root>

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add tests for different cases also 🙏 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I will add them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not Adding test case

<root><test>testValue</test></root>

because it can be addressed by simple_raw.xml

encoding=parsed_body.encoding,
indent=self.format_options['xml']['indent'],
standalone=parsed_body.standalone)

if not has_declaration:
body = body[body.find('\n') + 1:]

return body
this-is-r-gaurav marked this conversation as resolved.
Show resolved Hide resolved
1 change: 0 additions & 1 deletion tests/fixtures/xmldata/valid/simple-ns_formatted.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
<?xml version="1.0" encoding="utf-8"?>
<?pi data?>
<!-- comment -->
<root xmlns="namespace">
Expand Down
1 change: 0 additions & 1 deletion tests/fixtures/xmldata/valid/simple_formatted.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- comment -->
<root>
<element key="value">text</element>
Expand Down