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

Do not append missing XML declaration in formatted responses #1156

Closed
BoboTiG opened this issue Sep 13, 2021 · 6 comments · Fixed by #1227
Closed

Do not append missing XML declaration in formatted responses #1156

BoboTiG opened this issue Sep 13, 2021 · 6 comments · Fixed by #1227
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone

Comments

@BoboTiG
Copy link
Contributor

BoboTiG commented Sep 13, 2021

Minimal reproduction code and steps

  1. Create the file repro.xml with that content:
<!-- comment --><root><element key='value'>text</element><element>text</element>tail<empty-element/></root>
  1. Serve that file using:
$ python -m http.server
  1. And GET the file:
$ http :8000/repro.xml

Current result

<?xml version="1.0" encoding="utf-8"?>
<!-- comment -->
<root>
  <element key="value">text</element>
  <element>text</element>
  tail
  <empty-element/>
</root>

Note the additional XML declaration (it is not present from the original response).

Expected result

The responses should not be altered:

<!-- comment -->
<root>
  <element key="value">text</element>
  <element>text</element>
  tail
  <empty-element/>
</root>

Additional information, screenshots, or code examples

When pretty-printing a XML response that does not contain a XML declaration, minidom will append one (source).

@BoboTiG BoboTiG added bug Something isn't working new Needs triage. Comments are welcome! and removed new Needs triage. Comments are welcome! labels Sep 13, 2021
@BoboTiG BoboTiG self-assigned this Sep 13, 2021
@BoboTiG BoboTiG removed their assignment Oct 7, 2021
@BoboTiG BoboTiG added the help wanted Extra attention is needed label Oct 7, 2021
@this-is-r-gaurav
Copy link
Contributor

Hi @BoboTiG can i raise a PR for that fix?

@BoboTiG
Copy link
Contributor Author

BoboTiG commented Oct 15, 2021

Of course @this-is-r-gaurav 👍 :)

@this-is-r-gaurav
Copy link
Contributor

Its a dirty hack, not sure whether its acceptable, if any further suggestion i can update it @BoboTiG #1183

@this-is-r-gaurav
Copy link
Contributor

this-is-r-gaurav commented Oct 15, 2021

Two test cases are failing after this PR:

I think after this fix formatted file will need to remove xml prolog. If i need to fix those test, do let me know please. :)

@BoboTiG
Copy link
Contributor Author

BoboTiG commented Oct 15, 2021

I think after this fix formatted file will need to remove xml prolog. If i need to fix those test, do let me know please. :)

Indeed, simply removing the XML declaration is needed, let's do that in the same PR :)

@this-is-r-gaurav
Copy link
Contributor

Thanks @BoboTiG , learned new things today. I have updated the fixture files as well as logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
3 participants