Skip to content

Support xml.sax pyexpat setup APIs#8238

Open
doma17 wants to merge 1 commit into
RustPython:mainfrom
doma17:fix-pyexpat-sax-setup
Open

Support xml.sax pyexpat setup APIs#8238
doma17 wants to merge 1 commit into
RustPython:mainfrom
doma17:fix-pyexpat-sax-setup

Conversation

@doma17

@doma17 doma17 commented Jul 8, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

Adds the pyexpat setup APIs that xml.sax expects when initializing the Expat reader.

Problem

xml.sax.parseString() and related SAX paths create a parser through xml.parsers.expat, then call setup methods such as SetParamEntityParsing().

RustPython's native pyexpat parser did not expose those setup APIs, so normal SAX parsing could fail before it started parsing XML.

Fixes

  1. Expose the XML_PARAM_ENTITY_PARSING_* constants.
  2. Add SetParamEntityParsing() as a compatibility shim for SAX setup.
  3. Add SetBase() and GetBase() for parser base URI state used by locator-related SAX paths.
  4. Add regression coverage in extra_tests/snippets/stdlib_xml.py.
  5. Remove expectedFailure markers from the SAX tests that now pass.

This does not implement full Expat-style external entity parsing. The current backend still uses xml-rs, which does not expose that configuration.

Testing

  • PATH=/tmp/pyshim:$PATH prek run --all-files
  • cargo run --quiet -- extra_tests/snippets/stdlib_xml.py
  • cargo run --release -- -m test test_sax
  • cargo run --release -- -m test test_pyexpat
  • cargo test --workspace --exclude rustpython-capi --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher --features threading --no-default-features --features stdlib,importlib,stdio,encodings,sqlite,ssl-rustls-aws-lc,host_env && (cd crates/capi && cargo test)
  • PYO3_CONFIG_FILE=$PWD/crates/capi/pyo3-rustpython.config cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher
  • cargo build --release --features sqlite && cd extra_tests && /tmp/rustpython-extra-tests-venv/bin/python -m pytest -v
  • cargo clippy --workspace --exclude rustpython-capi --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher --features threading --no-default-features --features stdlib,importlib,stdio,encodings,sqlite,ssl-rustls-aws-lc,host_env

AI use

Assisted by Codex:gpt-5.5 for implementation, test drafting, and verification. I reviewed and verified the changes locally.

Summary by CodeRabbit

  • New Features

    • Added support for XML parser parameter entity parsing constants and related compatibility methods.
    • Added base URI getter/setter support for XML parsers.
    • Added XML parsing event coverage for a simple start/end element flow.

  • Tests

    • Added tests for parser constants, accepted input types, overflow handling, and invalid argument errors.
    • Added tests confirming base URI behavior and XML event sequencing.

@doma17
Let xml.sax initialize pyexpat-backed parsers
03e2d0f 
RustPython already exposes an Expat-like XML parser, but xml.sax expects a few setup APIs before parsing. Add compatible parser setup methods and constants without pretending to implement full Expat external entity behavior.

Constraint: Lib/test changes only remove expectedFailure markers for tests that now pass; test bodies and assertions are unchanged.
Rejected: Implementing full Expat parameter entity parsing | xml-rs does not expose that configuration and the immediate failure is the missing setup API.
Confidence: high
Scope-risk: narrow
Directive: Keep SetParamEntityParsing as a compatibility shim unless the backend grows real Expat-style entity parsing support.
Tested: PATH=/tmp/pyshim:$PATH prek run --all-files
Tested: cargo run --quiet -- extra_tests/snippets/stdlib_xml.py
Tested: cargo run --release -- -m test test_sax
Tested: cargo run --release -- -m test test_pyexpat
Tested: cargo test --workspace --exclude rustpython-capi --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher --features threading --no-default-features --features stdlib,importlib,stdio,encodings,sqlite,ssl-rustls-aws-lc,host_env && (cd crates/capi && cargo test)
Tested: PYO3_CONFIG_FILE=$PWD/crates/capi/pyo3-rustpython.config cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher
Tested: cargo build --release --features sqlite && cd extra_tests && /tmp/rustpython-extra-tests-venv/bin/python -m pytest -v
Tested: cargo clippy --workspace --exclude rustpython-capi --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher --features threading --no-default-features --features stdlib,importlib,stdio,encodings,sqlite,ssl-rustls-aws-lc,host_env
Assisted-by: Codex:gpt-5.5
@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 158c9dc8-9313-4841-9234-2348c8bb212a

📥 Commits

Reviewing files that changed from the base of the PR and between c41180d and 03e2d0f.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_sax.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/stdlib/src/pyexpat.rs
  • extra_tests/snippets/stdlib_xml.py
📝 Walkthrough

Walkthrough

This PR adds XML_PARAM_ENTITY_PARSING_* constants, a base state field, and SetParamEntityParsing, SetBase, and GetBase methods to the PyExpatLikeXmlParser class in the pyexpat stdlib module, along with corresponding test coverage in a new stdlib XML test file.

Changes

Expat Parser Compatibility Methods

Layer / File(s) Summary
Parser constants, base state, and new methods
crates/stdlib/src/pyexpat.rs		 Adds ArgPrimitiveIndex import, three XML_PARAM_ENTITY_PARSING_* constants, a base: PyRwLock<Option<String>> field initialized to None, and SetParamEntityParsing, SetBase, GetBase pymethods.
Test coverage for new expat methods
extra_tests/snippets/stdlib_xml.py		 New test file asserting constant values, SetParamEntityParsing/SetBase/GetBase behavior and error handling, and a ContentHandler-based SAX parsing test.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PythonCode
  participant PyExpatLikeXmlParser
  PythonCode->>PyExpatLikeXmlParser: SetParamEntityParsing(flag)
  PyExpatLikeXmlParser-->>PythonCode: returns 1
  PythonCode->>PyExpatLikeXmlParser: SetBase(base_str)
  PyExpatLikeXmlParser->>PyExpatLikeXmlParser: store base in PyRwLock
  PythonCode->>PyExpatLikeXmlParser: GetBase()
  PyExpatLikeXmlParser-->>PythonCode: returns stored base or None
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding pyexpat setup APIs for xml.sax compatibility.
Linked Issues check ✅ Passed The PR adds the missing pyexpat setup APIs and constants needed to avoid the SAX setup TypeError reported in #4983.
Out of Scope Changes check ✅ Passed The described changes stay focused on xml.sax/pyexpat compatibility and related regression tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/asyncio
[ ] test: cpython/Lib/test/test_asyncio (TODO: 35)

dependencies:

  • asyncio

dependent tests: (7 tests)

  • asyncio: test_asyncio test_external_inspection test_inspect test_logging test_os test_pdb test_unittest

[x] test: cpython/Lib/test/test_float.py (TODO: 4)
[x] test: cpython/Lib/test/test_strtod.py (TODO: 2)

dependencies:

dependent tests: (no tests depend on float)

[x] test: cpython/Lib/test/test_frame.py (TODO: 14)

dependencies:

dependent tests: (no tests depend on frame)

[x] lib: cpython/Lib/xml
[x] test: cpython/Lib/test/test_xml_etree.py (TODO: 61)
[x] test: cpython/Lib/test/test_xml_etree_c.py
[x] test: cpython/Lib/test/test_minidom.py (TODO: 25)
[x] test: cpython/Lib/test/test_pulldom.py (TODO: 4)
[ ] test: cpython/Lib/test/test_pyexpat.py (TODO: 50)
[x] test: cpython/Lib/test/test_sax.py (TODO: 31)
[x] test: cpython/Lib/test/test_xml_dom_minicompat.py
[x] test: cpython/Lib/test/test_xml_dom_xmlbuilder.py

dependencies:

  • xml

dependent tests: (34 tests)

  • xml: test_doctest test_minidom test_pulldom test_pydoc test_pyexpat test_regrtest test_sax test_typing test_xml_dom_minicompat test_xml_dom_xmlbuilder test_xml_etree
    • plistlib: test_plistlib
      • platform: test__locale test__osx_support test_asyncio test_baseexception test_builtin test_cmath test_ctypes test_fcntl test_math test_mimetypes test_os test_platform test_posix test_shutil test_socket test_ssl test_strptime test_sysconfig test_time test_winreg test_wsgiref
    • xmlrpc.client: test_xmlrpc

[ ] test: cpython/Lib/test/test_generators.py (TODO: 11)
[ ] test: cpython/Lib/test/test_genexps.py (TODO: 4)
[x] test: cpython/Lib/test/test_generator_stop.py
[x] test: cpython/Lib/test/test_yield_from.py (TODO: 1)

dependencies:

dependent tests: (no tests depend on generator)

[x] lib: cpython/Lib/inspect.py
[ ] test: cpython/Lib/test/test_inspect (TODO: 33)

dependencies:

  • inspect

dependent tests: (96 tests)

  • inspect: test_abc test_argparse test_asyncgen test_buffer test_builtin test_clinic test_code test_collections test_coroutines test_decimal test_enum test_functools test_generators test_grammar test_inspect test_monitoring test_ntpath test_operator test_patma test_posixpath test_pydoc test_signal test_sqlite3 test_traceback test_turtle test_type_annotations test_types test_typing test_unittest test_yield_from test_zipimport test_zipimport_support test_zoneinfo
    • ast: test_ast test_compile test_compiler_codegen test_dis test_fstring test_future_stmt test_peepholer test_peg_generator test_site test_ssl test_type_comments test_ucn test_unparse
      • annotationlib: test_annotationlib test_reprlib test_type_params
      • dbm.dumb: test_dbm_dumb
      • pyclbr: test_pyclbr
      • traceback: test_asyncio test_code_module test_contextlib test_contextlib_async test_dictcomps test_exceptions test_http_cookiejar test_importlib test_iter test_listcomps test_pyexpat test_setcomps test_socket test_subprocess test_sys test_threadedtempfile test_threading test_unittest test_with
    • asyncio: test_asyncio test_external_inspection test_logging test_os test_pdb
    • bdb: test_bdb
    • cmd: test_cmd
      • pstats: test_profile test_pstats
    • dataclasses: test__colorize test_copy test_ctypes test_genericalias test_pprint test_regrtest
      • pprint: test_htmlparser test_sys_setprofile
    • importlib.metadata: test_importlib
    • pkgutil: test_pkgutil test_pyrepl test_runpy
    • pydoc:
      • xmlrpc.server: test_docxmlrpc test_xmlrpc
    • rlcompleter: test_pyrepl test_rlcompleter
    • trace: test_trace

[x] lib: cpython/Lib/io.py
[x] lib: cpython/Lib/_pyio.py
[ ] test: cpython/Lib/test/test_io.py (TODO: 14)
[x] test: cpython/Lib/test/test_bufio.py
[x] test: cpython/Lib/test/test_fileio.py (TODO: 1)
[ ] test: cpython/Lib/test/test_memoryio.py (TODO: 27)

dependencies:

  • io

dependent tests: (108 tests)

  • io: test__colorize test_android test_argparse test_ast test_asyncio test_base64 test_buffer test_bufio test_builtin test_bz2 test_calendar test_cmd test_cmd_line_script test_codecs test_compile test_compileall test_compiler_assemble test_concurrent_futures test_configparser test_contextlib test_csv test_dbm_dumb test_descr test_dis test_email test_enum test_file test_fileinput test_fileio test_ftplib test_generated_cases test_getpass test_gzip test_hashlib test_http_cookiejar test_httplib test_httpservers test_importlib test_inspect test_io test_json test_largefile test_logging test_lzma test_mailbox test_marshal test_memoryio test_memoryview test_mimetypes test_minidom test_multibytecodec test_optparse test_pathlib test_pdb test_peg_generator test_pickle test_pickletools test_platform test_plistlib test_pprint test_print test_profile test_pstats test_pty test_pulldom test_pydoc test_pyexpat test_pyrepl test_quopri test_regrtest test_robotparser test_sax test_shlex test_shutil test_site test_smtplib test_socket test_socketserver test_subprocess test_support test_sys test_tarfile test_tempfile test_threadedtempfile test_timeit test_tokenize test_traceback test_types test_typing test_unittest test_univnewlines test_urllib test_urllib2 test_uuid test_wave test_webbrowser test_winconsoleio test_wsgiref test_xml_dom_xmlbuilder test_xml_etree test_xml_etree_c test_xmlrpc test_xpickle test_zipapp test_zipfile test_zipimport test_zoneinfo test_zstd

[x] test: cpython/Lib/test/test_descr.py (TODO: 34)
[ ] test: cpython/Lib/test/test_descrtut.py (TODO: 3)

dependencies:

dependent tests: (no tests depend on descr)

[x] lib: cpython/Lib/ssl.py
[x] test: cpython/Lib/test/test_ssl.py (TODO: 15)

dependencies:

  • ssl

dependent tests: (53 tests)

  • ssl: test_asyncio test_ftplib test_httplib test_httpservers test_imaplib test_logging test_poplib test_ssl test_urllib test_urllib2_localnet test_venv test_xmlrpc
    • asyncio.selector_events: test_asyncio
    • ftplib: test_urllib2
      • urllib.request: test_http_cookiejar test_pathlib test_pydoc test_sax test_site test_urllib2net test_urllibnet
    • http.client: test_docxmlrpc test_hashlib test_ucn test_unicodedata test_wsgiref
      • logging.handlers: test_concurrent_futures test_pkgutil
    • http.server: test_robotparser
      • pydoc: test_enum
    • smtplib: test_smtplib test_smtpnet
    • urllib.request:
      • pathlib: test_ast test_dbm_sqlite3 test_ensurepip test_importlib test_json test_launcher test_pathlib test_peg_generator test_pyrepl test_runpy test_tarfile test_tempfile test_tomllib test_tools test_traceback test_unparse test_winapi test_zipapp test_zipfile test_zoneinfo test_zstd

[x] lib: cpython/Lib/pdb.py
[ ] test: cpython/Lib/test/test_pdb.py (TODO: 47)

dependencies:

  • pdb

dependent tests: (1 tests)

  • pdb: test_pdb

[x] lib: cpython/Lib/traceback.py
[x] test: cpython/Lib/test/test_traceback.py (TODO: 5)

dependencies:

  • traceback

dependent tests: (161 tests)

  • traceback: test_asyncio test_builtin test_code_module test_contextlib test_contextlib_async test_coroutines test_dictcomps test_exceptions test_http_cookiejar test_importlib test_iter test_listcomps test_pyexpat test_setcomps test_socket test_ssl test_subprocess test_sys test_threadedtempfile test_threading test_traceback test_unittest test_with test_zipimport
    • code:
      • pdb: test_pdb
      • sqlite3.main: test_sqlite3
    • concurrent.futures.process: test_compileall test_concurrent_futures
    • http.cookiejar: test_urllib2
      • urllib.request: test_pathlib test_pydoc test_sax test_site test_urllib test_urllib2_localnet test_urllib2net test_urllibnet
    • logging: test_asyncio test_decimal test_genericalias test_hashlib test_logging test_pkgutil test_support test_unittest
      • hashlib: test_hmac test_smtplib test_tarfile test_unicodedata
      • multiprocessing.util: test_asyncio test_concurrent_futures
      • venv: test_venv
    • multiprocessing: test_fcntl test_memoryview test_multiprocessing_main_handling test_re
    • py_compile: test_argparse test_cmd_line_script test_importlib test_modulefinder test_py_compile test_runpy
      • zipfile: test_shutil test_zipapp test_zipfile test_zipfile64 test_zipimport_support
    • pydoc: test_enum
      • xmlrpc.server: test_docxmlrpc test_xmlrpc
    • socketserver: test_imaplib test_socketserver test_wsgiref
    • threading: test_android test_asyncio test_bytes test_bz2 test_code test_concurrent_futures test_context test_ctypes test_email test_external_inspection test_fork1 test_frame test_ftplib test_functools test_gc test_httplib test_httpservers test_importlib test_inspect test_io test_ioctl test_itertools test_largefile test_linecache test_opcache test_pathlib test_poll test_poplib test_pyrepl test_queue test_robotparser test_sched test_signal test_sqlite3 test_super test_syslog test_termios test_threading_local test_time test_weakref test_winreg test_zstd
      • bdb: test_bdb
      • dummy_threading: test_dummy_threading
      • importlib.util: test_asdl_parser test_ctypes test_doctest test_importlib test_reprlib
      • queue: test_dummy_thread
      • subprocess: test_asyncio test_atexit test_audit test_c_locale_coercion test_cmd_line test_ctypes test_dtrace test_embed test_faulthandler test_file_eintr test_gzip test_json test_launcher test_msvcrt test_ntpath test_os test_osx_env test_peg_generator test_platform test_plistlib test_pyrepl test_quopri test_regrtest test_repl test_script_helper test_select test_sys_settrace test_sysconfig test_tempfile test_unittest test_utf8_mode test_wait3 test_webbrowser test_xpickle
      • sysconfig: test_posix test_tools
      • trace: test_trace
    • timeit: test_timeit

[ ] test: cpython/Lib/test/test_monitoring.py (TODO: 7)

dependencies:

dependent tests: (no tests depend on monitoring)

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

ShaharNaveh
ShaharNaveh approved these changes Jul 8, 2026
View reviewed changes

@ShaharNaveh ShaharNaveh 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.

tysm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ShaharNaveh ShaharNaveh ShaharNaveh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TypeError when RustPython parses XML string

2 participants

@doma17 @ShaharNaveh