Skip to content

Read package manifests with utf-8-sig so a BOM does not drop the package - #5321

Open
arpitjain099 wants to merge 2 commits into
aboutcode-org:developfrom
arpitjain099:fix/manifest-bom
Open

arpitjain099 wants to merge 2 commits into
aboutcode-org:developfrom
arpitjain099:fix/manifest-bom

Conversation

@arpitjain099

Copy link
Copy Markdown

A package.json with a UTF-8 BOM makes the whole package vanish from the scan, and scan_errors stays empty, so nothing in the output says anything went wrong.

Same file twice, one with the three BOM bytes in front, scanned with the released scancode-toolkit 32.5.0:

$ scancode --package --json-pp - plain | jq '[.packages[].purl], (.dependencies|length)'
["pkg:npm/demo@1.0.0"]
1

$ scancode --package --json-pp - bom | jq '[.packages[].purl], (.dependencies|length), .files[1].scan_errors'
[]
0
[]

The input is not malformed. node -e "require('./package.json')" reads the BOM copy fine, and a BOM is what PowerShell's Out-File and several Windows editors write by default, so this shows up on real repositories rather than only in constructed cases.

The cause is the codec. Python's utf-8 does not consume a BOM, it decodes those bytes to U+FEFF, and json.load then raises with a message that names the fix:

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

recognize.py:155-162 catches that and continues without recording anything, which is why the failure is silent. Its sibling one layer up at plugin_package.py:481 does append the traceback to resource.scan_errors, so assembly failures are visible and parse failures are not.

I fixed the decode rather than the swallow, because the file is valid input and the package should be detected, not reported as an error. utf-8-sig is identical to utf-8 except that it consumes a leading BOM if one is present, so the swap is safe wherever a manifest is read, and grep -rn "utf-8-sig\|ufeff\|strip_bom" --include="*.py" src/ returned nothing before this, meaning there was no BOM handling anywhere to be consistent with. That is all 25 encoding='utf-8' sites in src/packagedcode/, across 15 handlers.

The swallow in recognize.py is worth fixing too, but it is a separate change with a wider blast radius, so I left it alone here.

Verification

tests/packagedcode/test_utf8_bom.py parametrizes four handlers that between them cover the JSON manifest shape (npm, bower, Composer, haxelib), once with a BOM and once without. The four BOM cases fail on develop with the JSONDecodeError above and pass with the change; the four no-BOM cases pass either way and are there so a regression in the plain path cannot hide.

For regressions I ran the tests for every handler I touched, before and after, against develop at 384b62a9: test_npm 94, test_rpm 76, test_conda 20, test_readme 17, test_conan 12, test_godeps 11, test_swift 11, test_phpcomposer 10, test_freebsd 9, test_bower 6, test_haxe 5, test_about 4, test_cran 2 all passed, and test_chef was 4 failed / 8 passed in both runs, so those four are pre-existing and unrelated.

Python's utf-8 codec does not consume a byte order mark. It decodes the three
bytes to U+FEFF, and json.load then raises "Unexpected UTF-8 BOM (decode using
utf-8-sig)". packagedcode opens every manifest with encoding='utf-8', and
recognize.py _parse() swallows the exception and continues, so a manifest that
carries a BOM disappears from the scan with scan_errors left empty.

The input is valid. npm, Composer and the other package managers read these
files, and a BOM is what several Windows editors and PowerShell's Out-File
write by default.

utf-8-sig differs from utf-8 only in consuming a leading BOM if one is there,
so this is a safe swap at all 25 manifest read sites in packagedcode. There was
no BOM-aware decode anywhere in the tree before this.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
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