Skip to content

Fix five small converter correctness bugs (data URIs, RSS, zip, plaintext, CLI) - #2329

Open
5awtooth wants to merge 1 commit into
microsoft:mainfrom
5awtooth:fix/small-converter-correctness-bugs
Open

Fix five small converter correctness bugs (data URIs, RSS, zip, plaintext, CLI)#2329
5awtooth wants to merge 1 commit into
microsoft:mainfrom
5awtooth:fix/small-converter-correctness-bugs

Conversation

@5awtooth

Copy link
Copy Markdown

Summary

Five small, independent correctness fixes in the core converters, each with a regression test:

  1. _uri_utils.parse_data_uri — accept unpadded base64. base64.b64decode raises binascii.Error: Incorrect padding when a data: URI's base64 payload length isn't a multiple of 4. Many encoders omit the = padding, so e.g. convert_uri("data:text/plain;base64,SGVsbG8") crashed. Re-pad before decoding.
  2. RssConverterUnboundLocalError on a channel with no title/description. md_text was only bound inside if channel_title:, so an RSS <channel> lacking both <title> and <description> raised UnboundLocalError in the item loop. Initialize md_text = "" first.
  3. ZipConverter — don't print `None` as the archive name. When converting raw zip bytes via convert_stream (no url/local_path/filename), the header read Content from the zip file `None`:. Fall back to "unknown.zip".
  4. PlainTextConverter — don't emit the literal "None". charset_normalizer's .best() returns None when nothing matches; str(None) made the document content "None". Guard against it.
  5. CLI _handle_output — tolerate sys.stdout.encoding is None. sys.stdout.encoding can be None for some redirected/embedded streams, which turned the output step into AttributeError. Fall back to UTF-8.

How these were found

I did a whole-repository read of markitdown with an LLM to surface candidate issues, then verified each one by hand — reading the surrounding code and running a minimal repro. Only fixes I could confirm are included, and each is deliberately minimal and idiomatic to the surrounding code. Happy to split this into separate PRs if you'd prefer one change per PR.

Test plan

  • New tests in packages/markitdown/tests/test_converter_bugfixes.py; each fails before its fix and passes after.
  • Full suite: 340 passed, 4 skipped locally on Python 3.13 (the one unrelated failure, test_speech_transcription, is a network/service-dependent test that also fails on a clean checkout in this environment).

🤖 Generated with Claude Code

- parse_data_uri: accept unpadded base64 data URIs (was binascii.Error)
- RssConverter: initialize md_text so channels without title/description
  don't raise UnboundLocalError
- ZipConverter: fall back to "unknown.zip" instead of printing `None`
- PlainTextConverter: guard charset_normalizer .best() returning None so
  content is never the literal "None"
- CLI: fall back to UTF-8 when sys.stdout.encoding is None

Each fix has a regression test in tests/test_converter_bugfixes.py.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@5awtooth

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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.

1 participant