fix(bundler): decode a downloaded (non-zip) bundle manifest as UTF-8 - #4190
Open
Noor-ul-ain001 wants to merge 1 commit into
Open
fix(bundler): decode a downloaded (non-zip) bundle manifest as UTF-8#4190Noor-ul-ain001 wants to merge 1 commit into
Noor-ul-ain001 wants to merge 1 commit into
Conversation
_download_remote_manifest's non-zip branch fed the downloaded bytes straight to `yaml.safe_load(io.BytesIO(raw))`. PyYAML's Reader auto-detects a UTF-16 BOM on a byte stream, so a well-formed UTF-16 bundle.yml (a realistic PowerShell `Out-File`/`>` output) was silently *accepted* here, while `yamlio.load_yaml` decodes local sources strictly as UTF-8 and rejects the identical content with "Could not read ...". BEFORE: a UTF-16 manifest downloaded via `bundle info`/`install` parses successfully -- exit code 0, no warning. AFTER: rejected with "... could not be read: ..." -- exit code 1, matching local directory and .zip sources. This is the same divergence, in the sibling branch of the same function, that was just fixed for the .zip case in commit 56aec8a (PR github#3958): "feeding PyYAML the byte stream let its Reader honour a UTF-16 BOM and accept a manifest yamlio.load_yaml rejects, so zip and directory sources diverged." That fix covered `_local_manifest_source`'s `.zip` branch (which this same function calls for zip artifacts); the direct raw-YAML-download branch a few lines below it had the identical bug. Also drops the now-unused `import io` from this function. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_download_remote_manifest's non-zip branch (src/specify_cli/commands/bundle/__init__.py) fed the downloaded bytes straight toyaml.safe_load(io.BytesIO(raw)). PyYAML'sReaderauto-detects a UTF-16 BOM on a byte stream, so a well-formed UTF-16bundle.yml(a realistic PowerShellOut-File/>output) was silently accepted — whileyamlio.load_yamldecodes local sources strictly as UTF-8 and rejects the identical content with"Could not read ...".This is the same divergence, in the sibling branch of the same function, that was just fixed for the
.zipcase in commit 56aec8a (#3958): "feeding PyYAML the byte stream let its Reader honour a UTF-16 BOM and accept a manifest yamlio.load_yaml rejects, so zip and directory sources diverged." That fix covered_local_manifest_source's.zipbranch, which this same function (_download_remote_manifest) calls for zip artifacts a few lines above — but the direct raw-YAML-download branch right after it had the identical unfixed bug.Also drops the now-unused
import iofrom the function.Test plan
test_bundle_info_rejects_utf16_remote_manifest_like_local_sourcestotests/contract/test_bundle_cli.py, downloading a UTF-16-encoded manifest via a mockedopen_urland assertingbundle info --jsonfails with a "could not be read" errorexit_code == 0, silently accepted) and passes with itpytest tests/contract/test_bundle_cli.py— 43 passed, 1 pre-existing failure unrelated to this change (test_build_escapes_markup_in_output_path, a Windows path-separator/markup quirk, reproduces identically on unmodifiedupstream/main)pytest tests/integration/test_bundler_local_install.py— 17 passed (no regression in the sibling.zipfix)ruff checkon both changed files — clean🤖 Generated with Claude Code