Skip to content

fix: std.base64 rejects string codepoints outside byte range - #1008

Closed
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:worktree-fix-base64-codepoint-range
Closed

fix: std.base64 rejects string codepoints outside byte range#1008
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:worktree-fix-base64-codepoint-range

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Motivation

std.base64(input) accepts string or array inputs, but the Jsonnet stdlib defines the input domain as codepoints/numbers in [0, 255].

sjsonnet already validated array elements, but string input had a gap: non-ASCII strings went straight to PlatformBase64, so codepoints above 255 were UTF-8 encoded instead of rejected. The observable bug is std.base64(std.char(256)): before this PR sjsonnet returned "xIA=", while go-jsonnet rejects the input with got 256.

This PR is intentionally scoped to that validation gap. It does not change the existing in-range string encoding behavior for [128, 255]; local checks show std.base64(std.char(233)) remains "w6k=", matching go-jsonnet 0.22.0 and jrsonnet 0.5.0-pre99.

Modification

  • Add validateBase64StringCodepoints in EncodingModule.scala and call it for non-Val.AsciiSafeStr string inputs before encoding.
  • Reject the first string codepoint above 255 with: base64 encountered invalid codepoint value in the string (must be 0 <= X <= 255), got <value>.
  • Keep the Val.AsciiSafeStr fast path unchanged; those values are known to be printable ASCII-safe and therefore in range.
  • Preserve existing behavior for valid non-ASCII codepoints in [128, 255].
  • Update Base64Tests.scala for CJK, emoji, large non-ASCII rejection, and in-range non-ASCII roundtrip.
  • Re-enable go_test_suite/builtinBase64_string_high_codepoint.jsonnet in JVM/Native and JS file tests, and update its golden output.
  • Add new_test_suite fixtures for std.char(256) and a mid-string out-of-range codepoint.

Result

std.base64 now rejects string inputs outside the byte-string domain, aligning the out-of-range behavior with the Jsonnet stdlib contract and the go-jsonnet high-codepoint fixture, while preserving valid ASCII and in-range non-ASCII behavior.

Behavior comparison:

Input sjsonnet before sjsonnet after go-jsonnet 0.22.0 jrsonnet 0.5.0-pre99
std.base64("hello") "aGVsbG8=" "aGVsbG8=" "aGVsbG8=" "aGVsbG8="
std.base64(std.char(0)) "AA==" "AA==" "AA==" "AA=="
std.base64(std.char(233)) "w6k=" "w6k=" "w6k=" "w6k="
std.base64(std.char(255)) "w78=" "w78=" "w78=" "w78="
std.base64(std.char(256)) "xIA=" error: got 256 error: got 256 "xIA="
std.base64(std.char(19990)) UTF-8 encoded error: got 19990 error: got 19990 UTF-8 encoded

The intended behavior change is limited to string codepoints above 255. Valid ASCII and in-range non-ASCII cases remain unchanged.

Verification:

  • ./mill sjsonnet.jvm.3_3_7.reformat passed.
  • ./mill sjsonnet.jvm.3_3_7.test passed: 143/143.
  • ./mill sjsonnet.js.3_3_7.test passed: 475/475.
  • ./mill sjsonnet.native.3_3_7.test passed: 488/488.
  • std.base64(std.char(233)) remains "w6k=".
  • std.base64(std.char(256)) fails with got 256.
  • PR branch is a single commit: 44cbec46 fix: std.base64 rejects string codepoints outside byte range.

References

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.
@He-Pin He-Pin changed the title fix: std.base64 rejects string codepoints outside [0, 255] fix: std.base64 rejects string codepoints outside byte range Jun 20, 2026
@He-Pin
He-Pin force-pushed the worktree-fix-base64-codepoint-range branch from 95e6c61 to 44cbec4 Compare June 20, 2026 09:23
@He-Pin
He-Pin marked this pull request as ready for review June 20, 2026 09:25
@He-Pin He-Pin closed this Jun 20, 2026
@He-Pin He-Pin reopened this Jun 20, 2026
@CertainLach

Copy link
Copy Markdown

Didn't we agree that the official behavior is incorrect here? #793 (comment)

@CertainLach

Copy link
Copy Markdown

If you want to limit the codepoint range - then the limit should be 127, and not 255: #793 (comment)

@He-Pin
He-Pin marked this pull request as draft June 20, 2026 12:00
@He-Pin

He-Pin commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the reminder. @CertainLach

@He-Pin

He-Pin commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR.

I re-read #793 and the latest comments here. This PR was based on a strict reading of the current stdlib wording / go-jsonnet's high-codepoint rejection, but it conflicts with the
direction recorded in #793: sjsonnet's UTF-8 byte semantics for string input were treated as by-design.

The patch also leaves us in a hybrid state: it rejects codepoints >255 while still UTF-8-encoding codepoints in 128..255, so it neither fully matches official C++ Jsonnet's codepoint-
byte behavior nor the agreed UTF-8 behavior.

Any future change here should be a separate policy PR after consensus, likely either preserving UTF-8 semantics explicitly or moving to an ASCII-only restriction for string input.

@He-Pin He-Pin closed this Jun 20, 2026
@He-Pin

He-Pin commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

@CertainLach, would you do a final release of jrsonnet soon?

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