Fix broken --create (JSON repack) round-trip - #13
Open
grub-basket wants to merge 1 commit into
Open
Conversation
Repacking a JSON dump back into a resource fork was broken in several ways: 1. Critical: do_pack() passed encoding=args.encoding to json_to_resource_fork(), which takes no such argument, so every 'rsrcdump -c' invocation raised TypeError before doing any work. The encoding is already applied globally via set_global_encoding(), so drop the extra kwarg. 2. Resource-type JSON keys were written with res_type.decode(), but repack parses them with parse_type_name() (URL-unquote). These aren't inverses: a FourCC with a high byte (e.g. b'\xA9icn') or trailing spaces fails to round-trip. Emit the key with sanitize_type_name(), matching the resource sub-directory name and parse_type_name(). 3. Repack detected the metadata block with 'len(type_name) > 4', which also dropped legitimate URL-encoded type keys longer than 4 chars (e.g. '%A9icn'). Match the '_metadata' key explicitly instead. Without this, fix (2)'s encoded keys would be silently skipped on repack. 4. JSONEncoderBase16Fallback.default() returned a new encoder instance for non-bytes objects instead of delegating to super().default(), which would recurse into RecursionError instead of raising a clear TypeError. Delegate to super(). Verified an extract-key -> repack round-trip for both normal and high-byte FourCCs.
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
1 participant
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.
Problem
Repacking a JSON dump back into a resource fork (
rsrcdump -c) was broken:--createis dead on arrival.do_pack()callsjson_to_resource_fork(..., encoding=args.encoding), but that function takes noencodingparameter, so everyrsrcdump -cinvocation raisesTypeErrorbefore doing any work. The encoding is already applied globally viaset_global_encoding(), so the extra kwarg is just wrong.res_type.decode(...), but repack parses it back withparse_type_name()(URL-unquote). Those aren't inverses — a FourCC with a high byte (e.g.b'\xA9icn') or trailing spaces fails to round-trip. Emit the key withsanitize_type_name(), matching the resource sub-directory name andparse_type_name().len(type_name) > 4, which also silently dropped legitimate URL-encoded type keys longer than 4 chars (e.g.%A9icn). Without fixing this, change (2) wouldn't actually help. Match the_metadatakey explicitly.JSONEncoderBase16Fallback.default()returned a new encoder instance for non-bytesobjects instead ofsuper().default(), which would recurse intoRecursionErrorinstead of raising a clearTypeError.Verification
Confirmed an extract-key →
json_to_resource_fork→fork.pack()round-trip for both a normal FourCC and a high-byte one (%A9ZZ→b'\xA9ZZ ').Prepared with Claude Fable 5 (Low effort mode). Automated finding, manually verified — please review before merging.