fix: std.base64 rejects string codepoints outside byte range - #1108
Closed
He-Pin wants to merge 1 commit into
Closed
fix: std.base64 rejects string codepoints outside byte range#1108He-Pin wants to merge 1 commit into
He-Pin wants to merge 1 commit into
Conversation
Motivation: The Jsonnet standard library defines std.base64 input as a string or an array whose codepoints or numbers are in the 0 to 255 range. sjsonnet already rejected out-of-range array values, but the string path passed every non-ASCII string to PlatformBase64 and UTF-8 encoded invalid codepoints instead of reporting an error. Modification: Validate non-AsciiSafeStr string inputs codepoint-by-codepoint before encoding and fail on the first codepoint above 255. Keep the existing ASCII fast path and existing UTF-8 behavior for in-range non-ASCII codepoints. Update Scala tests and file-test fixtures, including the official go_test_suite high-codepoint case, to cover the rejection behavior across JVM, JS, and Native file-test harnesses. Result: std.base64 now rejects string inputs outside the byte-string domain while preserving existing behavior for valid inputs and fast ASCII strings.
Contributor
Author
|
Closing: this change is wrong for this project. Re-checking the behavior across implementations:
sjsonnet's string-path behavior is a deliberate design decision (the skipped go test This PR would have regressed that extension and broken parity with jrsonnet. Same conclusion applies to #1008. |
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.
Motivation
The Jsonnet standard library defines
std.base64input as a string or an array whose codepoints/numbers are in the 0–255 range. sjsonnet already rejected out-of-range array values, but the string path passed every non-ASCII string straight toPlatformBase64, which silently UTF-8-encoded codepoints above 255:The same logical input (
std.char(256)vs[256]) behaved differently, and the string result does not round-trip throughstd.base64Decodeon other implementations. This supersedes #1008, which went stale.Modification
EncodingModule.scala: validate non-ASCII-safe string inputs codepoint-by-codepoint before encoding; fail on the first codepoint above 255 with a message mirroring the array path (base64 encountered invalid codepoint value in the string (must be 0 <= X <= 255), got N). The ASCII fast path and existing UTF-8 behavior for in-range (≤255) non-ASCII codepoints are unchanged.go_test_suite/builtinBase64_string_high_codepoint.jsonnet(JVM/Native and JS harnesses) and update its golden to the error output.new_test_suite/error.base64_string_codepoint_out_of_range.jsonnetanderror.base64_string_mid_codepoint_out_of_range.jsonnetcovering the boundary (256) and a mid-string CJK codepoint.Base64Tests.scalaandbase64_comprehensive.jsonnetso they no longer assert encoding of >255 codepoints.Result
Verified against go-jsonnet v0.22.0 — behavior now matches:
std.base64(std.char(256))std.base64("hello" + std.char(19990))std.base64(std.char(255))"w78=""w78="std.base64(std.char(200))"w4g=""w4g="std.base64("café")"Y2Fmw6k=""Y2Fmw6k="Test plan
./mill 'sjsonnet.jvm[2.13.18]'.test— all pass (including the un-skipped go_test_suite case and the two new error tests)./mill __.checkFormat— clean