Skip to content

Default to compact XML output and make pretty-print opt-in - #382

Merged
vinitkumar merged 9 commits into
masterfrom
codex/evaluate-hardened-xml-parser-alternatives
Aug 12, 2026
Merged

Default to compact XML output and make pretty-print opt-in#382
vinitkumar merged 9 commits into
masterfrom
codex/evaluate-hardened-xml-parser-alternatives

Conversation

@vinitkumar

@vinitkumar vinitkumar commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Prevent unbounded CPU/memory amplification caused by reparsing serializer output into a DOM by making compact output the default instead of reparsing with minidom on the common path.
  • Keep secure pretty-printing available as an explicit opt-in and use a hardened parser (defusedxml.minidom.parseString) on that opt-in path to mitigate XML parsing risks.

Description

  • Change the Json2xml constructor default from pretty: bool = True to pretty: bool = False so to_xml() returns serializer bytes by default and avoids a second DOM parse. (json2xml/json2xml.py).
  • Make the CLI --pretty default False and update the argument help text so the CLI opt-in behavior matches the library. (json2xml/cli.py).
  • Update documentation to describe compact output as the default and that pretty printing is explicit: README.rst, docs/usage.rst, and lat.md/ behavioral docs were updated accordingly.
  • Add regression tests ensuring the default path does not invoke the pretty DOM parser and that explicit pretty printing still returns Unicode text; adjust an existing pretty-error test to pass pretty=True when exercising the parser branch. (tests/test_json2xml.py, tests/test_cli.py).
  • Keep the pretty path parser import isolated to the opt-in branch and use defusedxml.minidom.parseString for hardened parsing to avoid unsafe stdlib minidom usage on the pretty path.

Testing

  • Ran pytest -q tests/test_cli.py, which passed (53 tests passed).
  • Executed a small Python smoke test verifying the default Json2xml({'name': 'Ada'}).to_xml() returns bytes and Json2xml(..., pretty=True).to_xml() returns str, which passed (compact-default and explicit-pretty smoke test passed).
  • Ran ruff lint checks on the changed modules and they completed without error.
  • Full collection of pytest -q tests/test_json2xml.py tests/test_cli.py initially failed during collection because xmltodict was unavailable and dependency installation was blocked by the environment network restrictions, so a complete end-to-end run could not be performed here; lat tooling also was unavailable in the environment and could not be executed.

Codex Task

Summary by Sourcery

Default Json2xml conversion now returns compact serializer bytes by default, with pretty-printed XML available via explicit opt-in in both the library and CLI.

New Features:

  • Expose compact XML output as the default conversion mode for Json2xml and the CLI, with pretty printing controlled by an explicit flag.

Enhancements:

  • Document compact output as the safe default and clarify that pretty printing is an opt-in behavior across usage and behavior guides.
  • Add tests to assert the default path returns compact bytes, avoids invoking the pretty-print DOM parser, and that the CLI parser reflects the new pretty default.

@sourcery-ai

sourcery-ai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR makes compact XML output the default for both the Json2xml library and CLI, keeps pretty-printing as an explicit opt-in using a hardened XML parser, and updates tests and docs to reflect the new behavior and type contracts.

Sequence diagram for Json2xml.to_xml compact vs pretty output

sequenceDiagram
    actor Caller
    participant Json2xml
    participant dicttoxml_fast
    participant defusedxml_minidom

    Caller->>Json2xml: to_xml()
    Json2xml->>dicttoxml_fast: dicttoxml(data, wrapper, root, attr_type, item_wrap, cdata, list_headers)
    dicttoxml_fast-->>Json2xml: xml_bytes (UTF-8)

    alt pretty is False (default)
        Json2xml-->>Caller: xml_bytes (bytes)
    else pretty is True (opt_in)
        Json2xml->>defusedxml_minidom: parseString(xml_bytes)
        defusedxml_minidom-->>Json2xml: xml_dom
        Json2xml->>Json2xml: xml_dom.toprettyxml()
        Json2xml-->>Caller: xml_text (str)
    end

    opt invalid XML with pretty=True
        defusedxml_minidom-->>Json2xml: [parse error]
        Json2xml-->>Caller: InvalidDataError
    end
Loading

File-Level Changes

Change Details Files
Default Json2xml conversion now returns compact serializer bytes instead of reparsing into a DOM for pretty output.
  • Changed Json2xml.init default pretty parameter from True to False so to_xml() uses the compact path by default.
  • Ensured compact output continues to return UTF-8 bytes while explicit pretty output returns Unicode text.
  • Kept defusedxml.minidom.parseString import isolated to the pretty-print branch for hardened parsing.
json2xml/json2xml.py
lat.md/behavior.md
lat.md/tests.md
Pretty-printing via the CLI is now opt-in and defaults to compact output.
  • Updated CLI flag documentation to state that pretty printing defaults to false.
  • Changed the --pretty argument to default to False while still being enabled via --pretty or -p.
  • Added a parser test asserting that the default parsed args have pretty set to False.
json2xml/cli.py
tests/test_cli.py
Tests and behavioral documentation now assert compact output as the safe default and verify pretty-print behavior explicitly.
  • Added a regression test that monkeypatches defusedxml.minidom.parseString to assert it is not called on the default compact path and that default output is bytes.
  • Adjusted the pretty-print error wrapping test to pass pretty=True so it exercises the hardened pretty parser path.
  • Documented that default conversion returns serializer bytes without building a second DOM copy and that pretty printing remains an explicit opt-in.
  • Updated documentation to describe compact output as the default, and that pretty printing acts as an opt-in validation step.
tests/test_json2xml.py
lat.md/behavior.md
lat.md/tests.md
README.rst
docs/usage.rst

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (826439f) to head (2f63afa).

Additional details and impacted files
@@            Coverage Diff             @@
##            master      #382    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            7         7            
  Lines          920      1037   +117     
==========================================
+ Hits           920      1037   +117     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread tests/test_json2xml.py Fixed
@vinitkumar
vinitkumar merged commit 48dfd38 into master Aug 12, 2026
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants