Skip to content

fix(ipynb): strip the UTF-8 BOM so a notebook is not emitted as raw JSON - #2425

Open
kevin (kevin9327) wants to merge 1 commit into
microsoft:mainfrom
kevin9327:fix/ipynb-utf8-bom
Open

fix(ipynb): strip the UTF-8 BOM so a notebook is not emitted as raw JSON#2425
kevin (kevin9327) wants to merge 1 commit into
microsoft:mainfrom
kevin9327:fix/ipynb-utf8-bom

Conversation

@kevin9327

Copy link
Copy Markdown

What this changes

A .ipynb saved with a UTF-8 BOM is not converted as a notebook. It comes back as its own
raw JSON:

{"nbformat": 4, "nbformat_minor": 5, "metadata": {}, "cells": [{"cell_type": "markdown",
"source": ["# Quarterly notes\n"], "metadata": {}}, {"cell_type": "code", "source":
["print('hello')\n"], "metadata": {}}]}

instead of:

# Quarterly notes

```python
print('hello')
```

Why

IpynbConverter.convert hands the decoded text straight to json.loads:

encoding = stream_info.charset or "utf-8"
notebook_content = file_stream.read().decode(encoding=encoding)
return self._convert(json.loads(notebook_content))

json.loads refuses a leading U+FEFF by design:

json.decoder.JSONDecodeError: Unexpected UTF-8 BOM (decode using utf-8-sig): line 1 column 1 (char 0)

The failure is not visible to the caller. _convert in _markitdown.py records the exception
as a failed attempt and keeps walking the converter list, and PlainTextConverter -- which
accepts anything textual -- succeeds. So the notebook is silently emitted as JSON source, and
a caller that only looks at the markdown has no way to tell.

A BOM on a .ipynb is not exotic: Windows PowerShell's Out-File/> and several editors
write UTF-8 with a signature, and nbformat itself tolerates the file on the way back in.

Fix

Strip the BOM before parsing, which is what _csv_converter already does with the same
one-liner for the same reason:

# _csv_converter.py
content = content.lstrip("")

Nothing else moves. lstrip on a string with no BOM is a no-op, so a notebook without one is
byte-identical; a caller that declares charset="utf-8-sig" has already consumed the BOM at
decode time and is likewise unaffected.

Tests

Added to packages/markitdown/tests/test_module_misc.py:

  • a BOM-prefixed notebook converts, keeping its heading, code fence and title
  • the same notebook through MarkItDown().convert_stream does not come back as raw JSON
  • pinned: a notebook with no BOM produces exactly the same markdown as before, asserted
    against the full string
  • pinned: charset="utf-8-sig" still works and is not double-stripped

Against unmodified main:

E  json.decoder.JSONDecodeError: Unexpected UTF-8 BOM (decode using utf-8-sig): line 1 column 1 (char 0)
E  assert False
E   +  where False = '{"nbformat": 4, "nbformat_minor": 5, ...}'.startswith('# Quarterly notes')
2 failed, 4 passed, 64 deselected

With the fix:

6 passed, 64 deselected

How I tested

Windows 11, Python 3.12, editable install of packages/markitdown[all].

pytest tests/test_module_misc.py -k ipynb   ->  2 failed, 4 passed  (before)
pytest tests/test_module_misc.py -k ipynb   ->  6 passed            (after)
pytest tests/                               ->  18 failed, 430 passed, 4 skipped

Those 18 are unchanged by this PR: main gives 18 failed, 426 passed, 4 skipped on this
machine before any edit, and the 4 added tests account for the difference. They are the CLI
stdout-encoding, Windows file-URI and speech-transcription tests that need a UTF-8 console, a
case-sensitive path and network/ffmpeg.

black --check clean on both files.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified this on a local checkout (Arch Linux, Python 3.14.7, markitdown from source at b6e8bbd).

The bug reproduces before the fix. With the test file from this PR applied to main without the source change, 2 tests fail with the BOM surfacing in the output:

'{"nbformat": 4, "nbformat_minor": 5, ... }' = DocumentConverterResult.markdown
2 failed, 3 passed

Calling the converter directly on main raises JSONDecodeError: Unexpected UTF-8 BOM (decode using utf-8-sig), so the raw-JSON output seen through the public API is the fallback after that exception — consistent with the description.

With the fix, the 5 new tests pass. I also ran the full test_module_misc.py: 13 failed / 55 passed here vs 13 failed / 51 passed on main — same 13 failures before and after (docx, xlsx, remote, speech, llm tests), so they are pre-existing in my environment and unrelated to this change. The PR adds 4 passing tests and no regressions.

One note, not a blocker: lstrip("\ufeff") strips any number of leading BOMs, where at most one is expected. Harmless here, and it matches the existing CSV converter at _csv_converter.py:82.

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.

2 participants