From 22499d44c712c5aba05871b546ca51f133f67d52 Mon Sep 17 00:00:00 2001 From: bplu4t2f Date: Sat, 24 Oct 2026 10:53:07 +0200 Subject: [PATCH 1/6] encoding/base64: Fix buggy test output --- tests/core/encoding/base64/base64.odin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/encoding/base64/base64.odin b/tests/core/encoding/base64/base64.odin index b842b0120..233da3744 100644 --- a/tests/core/encoding/base64/base64.odin +++ b/tests/core/encoding/base64/base64.odin @@ -487,7 +487,7 @@ test_newline_rejected :: proc(t: ^testing.T) { } for c in strict_cases { decoded, err := base64.decode(c, options = {.Strict}) - testing.expectf(t, err != nil, "strict decode(%q): expected rejection, got %q", c, decoded) + testing.expectf(t, err != nil, "strict decode(%q): expected rejection, got %q", c, err) delete(decoded) } From 2c2163f7f71c4b8196cdab7ebb2d40a360bf06f2 Mon Sep 17 00:00:00 2001 From: bplu4t2f Date: Sat, 24 Oct 2026 13:22:02 +0200 Subject: [PATCH 2/6] encoding/base64: Add tests to demonstrate holes in the current implementation. --- tests/core/encoding/base64/base64.odin | 142 +++++++++++++++++- .../test_issue_7763/test_issue_7763.odin | 2 +- 2 files changed, 140 insertions(+), 4 deletions(-) diff --git a/tests/core/encoding/base64/base64.odin b/tests/core/encoding/base64/base64.odin index 233da3744..dd784bf6b 100644 --- a/tests/core/encoding/base64/base64.odin +++ b/tests/core/encoding/base64/base64.odin @@ -126,7 +126,7 @@ test_strict_reject :: proc(t: ^testing.T) { options: base64.Decode_Options, err: base64.Decode_Error, } { - {"A", {.Strict}, .Invalid_Padding}, + {"A", {.Strict}, .Invalid_Padding}, // Debatable -> should be malformed quantum {"AA", {.Strict}, .Invalid_Padding}, {"Zg", {.Strict}, .Invalid_Padding}, {"Zm8", {.Strict}, .Invalid_Padding}, @@ -140,7 +140,7 @@ test_strict_reject :: proc(t: ^testing.T) { {"WvLTlMrX9NpYDQlEIFlnDB==", {.Strict}, .Non_Canonical}, {"Zg==", {.Strict, .No_Padding}, .Invalid_Padding}, {"Zm8=", {.Strict, .No_Padding}, .Invalid_Padding}, - {"Z", {.Strict, .No_Padding}, .Invalid_Padding}, + {"Z", {.Strict, .No_Padding}, .Invalid_Padding}, // Debatable -> should be malformed quantum {"QR", {.Strict, .No_Padding}, .Non_Canonical}, {"!!!!", {.Strict}, .Invalid_Character}, {"Zm9!", {.Strict}, .Invalid_Character}, @@ -467,7 +467,7 @@ test_rfc4648_and_go_vectors :: proc(t: ^testing.T) { } @(test) -test_newline_rejected :: proc(t: ^testing.T) { +test_newline_rejected_strict :: proc(t: ^testing.T) { // Inputs from Go's TestNewLineCharacters. Go's decoder deliberately ignores // "\r" and "\n" (MIME legacy); RFC 4648 section 3.3 says implementations // MUST reject characters outside the alphabet, so Odin must not accept them. @@ -490,10 +490,14 @@ test_newline_rejected :: proc(t: ^testing.T) { testing.expectf(t, err != nil, "strict decode(%q): expected rejection, got %q", c, err) delete(decoded) } +} +@(test) +test_newline_rejected_lenient :: proc(t: ^testing.T) { // Lenient mode only scans the input span `decoded_len` implies, so bytes // past the decoded tail are not always inspected (e.g. "c3VyZQ\n==" // currently decodes to "sure"); interior ones are always seen. + // NOTE(bplu4t2f): I would argue that ^ this is a bug. lenient_cases := []string{ "c3VyZQ==\n", "c3VyZQ=\n=", @@ -505,3 +509,135 @@ test_newline_rejected :: proc(t: ^testing.T) { testing.expect(t, decoded == nil) } } + +@(test) +reject_lenient_malformed_quantum_1 :: proc(t: ^testing.T) { + decoded, err := base64.decode("Q===") + // Debatable -> should be malformed quantum + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +reject_strict_malformed_quantum_1 :: proc(t: ^testing.T) { + decoded, err := base64.decode("Q===", options = { .Strict }) + // Debatable -> should be malformed quantum + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +reject_lenient_malformed_quantum_2 :: proc(t: ^testing.T) { + decoded, err := base64.decode("QUJDQ===") + // Debatable -> should be malformed quantum + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +reject_strict_malformed_quantum_2 :: proc(t: ^testing.T) { + decoded, err := base64.decode("QUJDQ===", options = { .Strict }) + // Debatable -> should be malformed quantum + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +reject_lenient_malformed_quantum_3 :: proc(t: ^testing.T) { + decoded, err := base64.decode("Q===QUJD") + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +reject_strict_malformed_quantum_3 :: proc(t: ^testing.T) { + decoded, err := base64.decode("Q===QUJD", options = { .Strict }) + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(private) +dynamic_array_stream_proc :: proc(stream_data: rawptr, mode: io.Stream_Mode, p: []byte, offset: i64, whence: io.Seek_From) -> (n: i64, err: io.Error) { + b := (^[dynamic]byte)(stream_data) + #partial switch mode { + case .Write: + written, err := append(b, ..p) + if err != nil { + return auto_cast written, .EOF + } else { + return auto_cast written, nil + } + case .Size: + n = i64(len(b)) + return + case .Destroy: + delete(b^) + return + case .Query: + return io.query_utility({.Write, .Size, .Destroy, .Query}) + } + return 0, .Unsupported +} + +@(test) +writer_path_malformed_tails :: proc(t: ^testing.T) { + backing := make([dynamic]byte) + defer delete(backing) + writer := io.Writer { procedure = dynamic_array_stream_proc, data = cast(rawptr)&backing } + + err := base64.decode_into(writer, "QUJDQ") + // Debatable -> should be malformed quantum + testing.expect(t, err != nil) +} + +@(test) +invalid_padding_error_lenient :: proc(t: ^testing.T) { + decoded, err := base64.decode("A=AA") + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +invalid_padding_error_strict :: proc(t: ^testing.T) { + decoded, err := base64.decode("A=AA", options = {.Strict}) + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +invalid_padding_error_end_lenient :: proc(t: ^testing.T) { + decoded, err := base64.decode("QUJDA=A") + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +invalid_padding_error_end_strict :: proc(t: ^testing.T) { + decoded, err := base64.decode("QUJDA=A", options = {.Strict}) + testing.expect_value(t, err, .Invalid_Padding) + testing.expect(t, decoded == nil) +} + +@(test) +lenient_ignore_excess_padding :: proc(t: ^testing.T) { + // Test different well-formed inputs with varying amount of padding, + // e.g. "QQ=====" + cases := []struct { + input: string, + output: string, + } { + { input = "", output = "", }, + { input = "QQ", output = "A", }, + { input = "QUI", output = "AB", }, + { input = "QUJD", output = "ABC", }, + } + for base in cases { + for num_padding in 1 ..= 8 { + s := strings.concatenate([]string { base.input, strings.repeat("=", num_padding, context.temp_allocator) }, context.temp_allocator) + decoded, err := base64.decode(s) + defer delete(decoded) + testing.expect_value(t, err, nil) + testing.expect_value(t, transmute(string)decoded, base.output) + } + } +} diff --git a/tests/issues/test_issue_7763/test_issue_7763.odin b/tests/issues/test_issue_7763/test_issue_7763.odin index 31de953ef..c55ed6941 100644 --- a/tests/issues/test_issue_7763/test_issue_7763.odin +++ b/tests/issues/test_issue_7763/test_issue_7763.odin @@ -47,7 +47,7 @@ test_issue_7763__3eq :: proc(t: ^testing.T) { test_issue_7763__Aeq :: proc(t: ^testing.T) { input := "A=" decoded, err := base64.decode(input) - testing.expect(t, err == nil) + testing.expect_value(t, err, .Invalid_Padding) testing.expect(t, decoded == nil) } From a20304f5d543b9cae708be92b03fad0a004a8b82 Mon Sep 17 00:00:00 2001 From: bplu4t2f Date: Sat, 24 Oct 2026 10:54:25 +0200 Subject: [PATCH 3/6] encoding/base64: Analyze input more closely before starting to decode Refactors the code slightly so that a `decode_analyze` step is performed first. This is needed because the previous architecture was a bit broken and did not allow fixing certain edge cases with broken input. Automatically fixes `writer_path_malformed_tails`, `test_newline_rejected_lenient`, and `lenient_ignore_excess_padding` edge cases. --- core/encoding/base64/base64.odin | 146 +++++++++++++++++-------- tests/core/encoding/base64/base64.odin | 18 ++- 2 files changed, 113 insertions(+), 51 deletions(-) diff --git a/core/encoding/base64/base64.odin b/core/encoding/base64/base64.odin index de764139d..808c5a4d2 100644 --- a/core/encoding/base64/base64.odin +++ b/core/encoding/base64/base64.odin @@ -370,72 +370,105 @@ validate_strict_decode :: proc(data: string, dec_tbl: [256]i8, options: Decode_O return .None } -decode :: proc(data: string, DEC_TBL := DEC_TABLE, dst: []byte = nil, allocator := context.allocator, options := Decode_Options{}) -> (decoded: []byte, err: Error) { - if derr := validate_strict_decode(data, DEC_TBL, options); derr != .None { - return nil, derr +@(private) +Decode_Info :: struct { + out_length: int, + trimmed_in: string, +} + +@(private) +decode_analyze :: proc(data: string, DEC_TBL := DEC_TABLE, options: Decode_Options) -> (info: Decode_Info, err: Decode_Error) { + err = validate_strict_decode(data, DEC_TBL, options) + if err != nil { + return } - out_length := decoded_len(data) - if out_length == 0 { + // Calculate trimmed input length based on stripped trailing padding. + length_in := len(data) + for length_in > 0 && data[length_in - 1] == PADDING { + length_in -= 1 + } + + quanta := length_in >> 2 + rem_in := length_in & 3 + rem_out: int + switch rem_in { + case 0: + case 1: + err = .Invalid_Padding // Debatable; should be malformed quantum + return + case 2: rem_out = 1 + case 3: rem_out = 2 + case: unreachable() + } + + info = { + out_length = quanta * 3 + rem_out, + trimmed_in = data[:length_in], + } + return +} + +decode :: proc(data: string, DEC_TBL := DEC_TABLE, dst: []byte = nil, allocator := context.allocator, options := Decode_Options{}) -> (decoded: []byte, err: Error) { + decode_info := decode_analyze(data, DEC_TBL, options) or_return + + if decode_info.out_length == 0 { return nil, nil } buf: []byte - if buf, err = make([]byte, out_length, allocator); err != nil { + if buf, err = make([]byte, decode_info.out_length, allocator); err != nil { return } - decoded, err = decode_impl(buf, data, DEC_TBL) + decoded, err = decode_impl(buf, decode_info, DEC_TBL) if err != nil { delete(buf, allocator) } - assert(err != nil || len(decoded) == out_length, "buffer unexpectedly resized, `decoded_len` was wrong") + assert(err != nil || len(decoded) == decode_info.out_length, "buffer unexpectedly resized, `analyze_decode` was wrong") return } decode_into_buf :: proc(dst: []byte, data: string, DEC_TBL := DEC_TABLE, options := Decode_Options{}) -> (decoded: []byte, err: Error) { - if derr := validate_strict_decode(data, DEC_TBL, options); derr != .None { - return nil, derr - } + decode_info := decode_analyze(data, DEC_TBL, options) or_return - out_length := decoded_len(data) - if out_length == 0 { + if decode_info.out_length == 0 { return } - return decode_impl(dst, data, DEC_TBL) + return decode_impl(dst, decode_info, DEC_TBL) } decode_into :: proc(w: io.Writer, data: string, DEC_TBL := DEC_TABLE, options := Decode_Options{}) -> Error { - if derr := validate_strict_decode(data, DEC_TBL, options); derr != .None { - return derr - } + decode_info := decode_analyze(data, DEC_TBL, options) or_return - _, err := decode_impl(w, data, DEC_TBL) + _, err := decode_impl(w, decode_info, DEC_TBL) return err } @(private) -decode_impl :: proc(dst: $T, data: string, DEC_TBL := DEC_TABLE) -> ([]byte, Error) where T == io.Writer || T == []byte { - length := decoded_len(data) +decode_impl :: proc(dst: $T, info: Decode_Info, DEC_TBL := DEC_TABLE) -> ([]byte, Error) where T == io.Writer || T == []byte { + dst_cursor: int when T == []byte { - if len(dst) < length { + if len(dst) < info.out_length { return nil, io.Error.Short_Buffer } - off: int } else { - if length == 0 { + if info.out_length == 0 { return nil, nil } buf: [3]byte } + // Note: decode_analyze handles removing padding. + data := info.trimmed_in + c0, c1, c2, c3: int d0, d1, d2, d3: i8 b0, b1, b2: int - i, j: int - for ; j + 3 <= length; i, j = i + 4, j + 3 { + i: int + for ; i + 4 <= len(data); i += 4 { #no_bounds_check { d0 = DEC_TBL[data[i]] d1 = DEC_TBL[data[i + 1]] @@ -453,10 +486,10 @@ decode_impl :: proc(dst: $T, data: string, DEC_TBL := DEC_TABLE) -> ([]byte, Err b2 = (c2 << 6) | c3 when T == []byte { - dst[off+0] = byte(b0) - dst[off+1] = byte(b1) - dst[off+2] = byte(b2) - off += 3 + dst[dst_cursor+0] = byte(b0) + dst[dst_cursor+1] = byte(b1) + dst[dst_cursor+2] = byte(b2) + dst_cursor += 3 } else { buf[0] = byte(b0) buf[1] = byte(b1) @@ -468,18 +501,20 @@ decode_impl :: proc(dst: $T, data: string, DEC_TBL := DEC_TABLE) -> ([]byte, Err if _, err := io.write_full(dst, buf[:]); err != .None { return nil, err } + dst_cursor += 3 } } - rest := length - j - if rest > 0 { + rest_in := len(data) - i + if rest_in > 0 { + when T == io.Writer { + rest_out: int + } + #no_bounds_check { - // Note: decoded_len handles removing padding. - d0 = DEC_TBL[data[i]] - d1 = DEC_TBL[data[i + 1]] - if d2 = 0; rest == 2 { - d2 = DEC_TBL[data[i + 2]] - } + d0 = DEC_TBL[data[i + 0]] + d1 = DEC_TBL[data[i + 1]] if rest_in >= 2 else 0 + d2 = DEC_TBL[data[i + 2]] if rest_in >= 3 else 0 if intrinsics.unlikely((d0 | d1 | d2) & ~i8(0x3f) != 0) { return nil, Decode_Error.Invalid_Character @@ -491,33 +526,52 @@ decode_impl :: proc(dst: $T, data: string, DEC_TBL := DEC_TABLE) -> ([]byte, Err b1 = (c1 << 4) | (c2 >> 2) when T == []byte { - switch rest { - case 2: - dst[off+1] = byte(b1) - fallthrough + switch rest_in { case 1: - dst[off] = byte(b0) + // Last quantum is malformed: "Q===" + return nil, .Invalid_Padding // Debatable; should be malformed quantum. + case 2: + dst[dst_cursor + 0] = byte(b0) + dst_cursor += 1 + case 3: + dst[dst_cursor + 0] = byte(b0) + dst[dst_cursor + 1] = byte(b1) + dst_cursor += 2 } } else { - buf[0] = byte(b0) - buf[1] = byte(b1) + switch rest_in { + case 1: + // Last quantum is malformed: "Q===" + return nil, .Invalid_Padding // Debatable; should be malformed quantum. + case 2: + buf[0] = byte(b0) + rest_out = 1 + case 3: + buf[0] = byte(b0) + buf[1] = byte(b1) + rest_out = 2 + } } } when T == io.Writer { - if _, err := io.write_full(dst, buf[:rest]); err != .None { + if _, err := io.write_full(dst, buf[:rest_out]); err != .None { return nil, err } + dst_cursor += rest_out } } + assert(dst_cursor == info.out_length) + when T == io.Writer { return nil, nil } else { - return dst[:length], nil + return dst[:dst_cursor], nil } } +// TODO: This API is fundamentally broken because it is public, but has aboslutely no error reporting strategy. decoded_len :: proc(data: string) -> int { length := len(data) if length == 0 { diff --git a/tests/core/encoding/base64/base64.odin b/tests/core/encoding/base64/base64.odin index dd784bf6b..4664ce1cc 100644 --- a/tests/core/encoding/base64/base64.odin +++ b/tests/core/encoding/base64/base64.odin @@ -494,18 +494,26 @@ test_newline_rejected_strict :: proc(t: ^testing.T) { @(test) test_newline_rejected_lenient :: proc(t: ^testing.T) { - // Lenient mode only scans the input span `decoded_len` implies, so bytes - // past the decoded tail are not always inspected (e.g. "c3VyZQ\n==" - // currently decodes to "sure"); interior ones are always seen. - // NOTE(bplu4t2f): I would argue that ^ this is a bug. lenient_cases := []string{ + "c3VyZQ\n==", "c3VyZQ==\n", "c3VyZQ=\n=", "c3V\ryZ\rQ==", } for c in lenient_cases { decoded, err := base64.decode(c) - testing.expectf(t, err == base64.Decode_Error.Invalid_Character, "lenient decode(%q): got %v, want Invalid_Character", c, err) + // NOTE(bplu4t2f): + // Ideally we would like to see Invalid_Character for all of these, however, that + // would imply an O(N) linear scan through the entire rest of the input data, even + // though we already know for sure that it contains an error. + // Ex.: QQ==QQ==Q\nQQQQ== + // ^ ^^^^^ + // | this quantum is not even inspected. + // interior padding; decoding failure is already set in stone. + // + // Instead, the implementation bails out early on error. + // That means that this can either yield Invalid_Padding or Invalid_Character, for now. + testing.expectf(t, err != nil, "lenient decode(%q): expected rejection, got %q", c, err) testing.expect(t, decoded == nil) } } From e0c71140d1a4d0c7abcd74e6f9c5da90b0a5d42e Mon Sep 17 00:00:00 2001 From: bplu4t2f Date: Sat, 24 Oct 2026 12:04:36 +0200 Subject: [PATCH 4/6] encoding/base64: Make `decoded_len` use the more accurate `decode_analyze`. TMP fix cbor decoded_len --- core/encoding/base64/base64.odin | 24 +++++++----------------- core/encoding/cbor/tags.odin | 15 +++++++++++++-- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/core/encoding/base64/base64.odin b/core/encoding/base64/base64.odin index 808c5a4d2..e27e6e992 100644 --- a/core/encoding/base64/base64.odin +++ b/core/encoding/base64/base64.odin @@ -571,21 +571,11 @@ decode_impl :: proc(dst: $T, info: Decode_Info, DEC_TBL := DEC_TABLE) -> ([]byte } } -// TODO: This API is fundamentally broken because it is public, but has aboslutely no error reporting strategy. -decoded_len :: proc(data: string) -> int { - length := len(data) - if length == 0 { - return 0 - } - - padding: int - if data[length - 1] == PADDING { - if length > 1 && data[length - 2] == PADDING { - padding = 2 - } else { - padding = 1 - } - } - - return max(0, ((length * 6) >> 3) - padding) +decoded_len :: proc(data: string, DEC_TBL := DEC_TABLE, options: Decode_Options = {}) -> (n: int, err: Decode_Error) { + // NOTE(bplu4t2f): + // It's not clear whether this is intended to be a public API. + // There are no regression tests for it. + info := decode_analyze(data, DEC_TBL, options) or_return + n = info.out_length + return } diff --git a/core/encoding/cbor/tags.odin b/core/encoding/cbor/tags.odin index ad0a85913..32282fa40 100644 --- a/core/encoding/cbor/tags.odin +++ b/core/encoding/cbor/tags.odin @@ -306,7 +306,12 @@ tag_base64_unmarshal :: proc(_: ^Tag_Implementation, d: Decoder, _: Tag_Number, case reflect.Type_Info_String: assert(t.encoding == .UTF_8) if t.is_cstring { - length := base64.decoded_len(bytes) + // Convert this using an auxiliary builder so we get a terminating null character. + length, derr := base64.decoded_len(bytes) + if derr != nil { + err = err_from_b64(derr) + return + } builder := strings.builder_make(0, length+1) b64_decode_into(strings.to_stream(&builder), bytes) or_return @@ -347,9 +352,15 @@ tag_base64_unmarshal :: proc(_: ^Tag_Implementation, d: Decoder, _: Tag_Number, if elem_base.id != byte { return _unsupported(v, hdr) } - if base64.decoded_len(bytes) > t.count { return _unsupported(v, hdr) } + decoded_length, derr := base64.decoded_len(bytes) + if derr != nil { + err = err_from_b64(derr) + return + } + if decoded_length > t.count { return _unsupported(v, hdr) } slice := ([^]byte)(v.data)[:len(bytes)] + // TODO: base64 can decode directly into a byte buffer. copy(slice, b64_decode(bytes) or_return) return } From b83cd571a8346ad05ca5faea920eb8e504258f7e Mon Sep 17 00:00:00 2001 From: bplu4t2f Date: Sat, 24 Oct 2026 12:19:30 +0200 Subject: [PATCH 5/6] encoding/base64: Lenient decode prioritizes `Invalid_Padding` over `Invalid_Character`. Applies only if unexpected interior padding is the only problem in a quantum. This is to be consistent with the error that would be returned by strict decode in that case (-> `Invalid_Padding`). --- core/encoding/base64/base64.odin | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/core/encoding/base64/base64.odin b/core/encoding/base64/base64.odin index e27e6e992..6c1ca838d 100644 --- a/core/encoding/base64/base64.odin +++ b/core/encoding/base64/base64.odin @@ -476,7 +476,17 @@ decode_impl :: proc(dst: $T, info: Decode_Info, DEC_TBL := DEC_TABLE) -> ([]byte d3 = DEC_TBL[data[i + 3]] if intrinsics.unlikely((d0 | d1 | d2 | d3) & ~i8(0x3f) != 0) { - return nil, Decode_Error.Invalid_Character + // To be consistent with validate_strict_decode, this will return + // Invalid_Padding if the input string contains invalid padding + // characters, but would otherwise be valid. + if (data[i + 0] == PADDING || d0 & ~i8(0x3f) == 0) && + (data[i + 1] == PADDING || d1 & ~i8(0x3f) == 0) && + (data[i + 2] == PADDING || d2 & ~i8(0x3f) == 0) && + (data[i + 3] == PADDING || d3 & ~i8(0x3f) == 0) { + return nil, .Invalid_Padding + } else { + return nil, .Invalid_Character + } } c0, c1, c2, c3 = int(d0), int(d1), int(d2), int(d3) @@ -517,7 +527,16 @@ decode_impl :: proc(dst: $T, info: Decode_Info, DEC_TBL := DEC_TABLE) -> ([]byte d2 = DEC_TBL[data[i + 2]] if rest_in >= 3 else 0 if intrinsics.unlikely((d0 | d1 | d2) & ~i8(0x3f) != 0) { - return nil, Decode_Error.Invalid_Character + // To be consistent with validate_strict_decode, this will return + // Invalid_Padding if the input string contains invalid padding + // characters, but would otherwise be valid. + if ( (data[i + 0] == PADDING || d0 & ~i8(0x3f) == 0)) && + (rest_in < 2 || (data[i + 1] == PADDING || d1 & ~i8(0x3f) == 0)) && + (rest_in < 3 || (data[i + 2] == PADDING || d2 & ~i8(0x3f) == 0)) { + return nil, .Invalid_Padding + } else { + return nil, .Invalid_Character + } } c0, c1, c2 = int(d0), int(d1), int(d2) From 1ad267f1ade9eefa6b9708c373fa0fb845368bf3 Mon Sep 17 00:00:00 2001 From: bplu4t2f Date: Sat, 24 Oct 2026 13:28:25 +0200 Subject: [PATCH 6/6] encoding/base64: Malformed Quantum error case is distinct from Invalid Padding. --- core/encoding/base64/base64.odin | 34 ++++++++++++++----- tests/core/encoding/base64/base64.odin | 29 ++++++++-------- .../test_issue_7763/test_issue_7763.odin | 4 +-- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/core/encoding/base64/base64.odin b/core/encoding/base64/base64.odin index 6c1ca838d..5f4316da2 100644 --- a/core/encoding/base64/base64.odin +++ b/core/encoding/base64/base64.odin @@ -132,6 +132,7 @@ Decode_Error :: enum { Invalid_Character, Invalid_Padding, Non_Canonical, + Malformed_Quantum, } // Decode_Option selects optional validation performed by the decode routines. @@ -308,42 +309,53 @@ validate_strict_decode :: proc(data: string, dec_tbl: [256]i8, options: Decode_O si -= 1 pad += 1 } - if pad > 2 { - return .Invalid_Padding - } for j in 0 ..< si { if data[j] == PADDING { + // Interior padding, e.g. QQ=Q return .Invalid_Padding } } } + // `si` is the amount of meaningful data, excluding trailing padding. + + // Number of non-padding characters in the last quantum (e.g. 2 for "QQ==", 3 for "QUI="). rem := si % 4 - if .No_Padding in options && rem == 1 { - return .Invalid_Padding + if rem == 1 { + // "QUJDQ" || "QUJDQ===" + // -> unsalvageable (this is not just missing padding or non-canonical). + return .Malformed_Quantum } if .Strict in options { if .No_Padding not_in options { if n % 4 != 0 { + // "QQ" || "QUI" + // -> MUST be "QQ==" || "QUI=" return .Invalid_Padding } switch rem { case 0: if pad != 0 { + // "QUJD=" || "QUJD==" + // -> MUST be "QUJD" return .Invalid_Padding } case 2: if pad != 2 { + // "QQ" || "QQ=" + // -> MUST be "QQ==" return .Invalid_Padding } case 3: if pad != 1 { + // "QUI" || "QUI==" + // -> MUST be "QUI=" return .Invalid_Padding } case: - return .Invalid_Padding + unreachable() } } @@ -354,6 +366,8 @@ validate_strict_decode :: proc(data: string, dec_tbl: [256]i8, options: Decode_O return .Invalid_Character } if (c & 0x0f) != 0 { + // In the last quantum, the second encoded character would decode into + // non-zero bits in the second decoded byte, which is beyond the decoded range. return .Non_Canonical } case 3: @@ -362,6 +376,8 @@ validate_strict_decode :: proc(data: string, dec_tbl: [256]i8, options: Decode_O return .Invalid_Character } if (c & 0x03) != 0 { + // In the last quantum, the third encoded character would decode into + // non-zero bits in the third decoded byte, which is beyond the decoded range. return .Non_Canonical } } @@ -395,7 +411,7 @@ decode_analyze :: proc(data: string, DEC_TBL := DEC_TABLE, options: Decode_Optio switch rem_in { case 0: case 1: - err = .Invalid_Padding // Debatable; should be malformed quantum + err = .Malformed_Quantum return case 2: rem_out = 1 case 3: rem_out = 2 @@ -548,7 +564,7 @@ decode_impl :: proc(dst: $T, info: Decode_Info, DEC_TBL := DEC_TABLE) -> ([]byte switch rest_in { case 1: // Last quantum is malformed: "Q===" - return nil, .Invalid_Padding // Debatable; should be malformed quantum. + return nil, .Malformed_Quantum case 2: dst[dst_cursor + 0] = byte(b0) dst_cursor += 1 @@ -561,7 +577,7 @@ decode_impl :: proc(dst: $T, info: Decode_Info, DEC_TBL := DEC_TABLE) -> ([]byte switch rest_in { case 1: // Last quantum is malformed: "Q===" - return nil, .Invalid_Padding // Debatable; should be malformed quantum. + return nil, .Malformed_Quantum case 2: buf[0] = byte(b0) rest_out = 1 diff --git a/tests/core/encoding/base64/base64.odin b/tests/core/encoding/base64/base64.odin index 4664ce1cc..d0a0e43eb 100644 --- a/tests/core/encoding/base64/base64.odin +++ b/tests/core/encoding/base64/base64.odin @@ -126,7 +126,7 @@ test_strict_reject :: proc(t: ^testing.T) { options: base64.Decode_Options, err: base64.Decode_Error, } { - {"A", {.Strict}, .Invalid_Padding}, // Debatable -> should be malformed quantum + {"A", {.Strict}, .Malformed_Quantum}, {"AA", {.Strict}, .Invalid_Padding}, {"Zg", {.Strict}, .Invalid_Padding}, {"Zm8", {.Strict}, .Invalid_Padding}, @@ -140,7 +140,7 @@ test_strict_reject :: proc(t: ^testing.T) { {"WvLTlMrX9NpYDQlEIFlnDB==", {.Strict}, .Non_Canonical}, {"Zg==", {.Strict, .No_Padding}, .Invalid_Padding}, {"Zm8=", {.Strict, .No_Padding}, .Invalid_Padding}, - {"Z", {.Strict, .No_Padding}, .Invalid_Padding}, // Debatable -> should be malformed quantum + {"Z", {.Strict, .No_Padding}, .Malformed_Quantum}, {"QR", {.Strict, .No_Padding}, .Non_Canonical}, {"!!!!", {.Strict}, .Invalid_Character}, {"Zm9!", {.Strict}, .Invalid_Character}, @@ -521,46 +521,48 @@ test_newline_rejected_lenient :: proc(t: ^testing.T) { @(test) reject_lenient_malformed_quantum_1 :: proc(t: ^testing.T) { decoded, err := base64.decode("Q===") - // Debatable -> should be malformed quantum - testing.expect_value(t, err, .Invalid_Padding) + testing.expect_value(t, err, .Malformed_Quantum) testing.expect(t, decoded == nil) } @(test) reject_strict_malformed_quantum_1 :: proc(t: ^testing.T) { decoded, err := base64.decode("Q===", options = { .Strict }) - // Debatable -> should be malformed quantum - testing.expect_value(t, err, .Invalid_Padding) + testing.expect_value(t, err, .Malformed_Quantum) testing.expect(t, decoded == nil) } @(test) reject_lenient_malformed_quantum_2 :: proc(t: ^testing.T) { decoded, err := base64.decode("QUJDQ===") - // Debatable -> should be malformed quantum - testing.expect_value(t, err, .Invalid_Padding) + testing.expect_value(t, err, .Malformed_Quantum) testing.expect(t, decoded == nil) } @(test) reject_strict_malformed_quantum_2 :: proc(t: ^testing.T) { decoded, err := base64.decode("QUJDQ===", options = { .Strict }) - // Debatable -> should be malformed quantum - testing.expect_value(t, err, .Invalid_Padding) + testing.expect_value(t, err, .Malformed_Quantum) testing.expect(t, decoded == nil) } @(test) reject_lenient_malformed_quantum_3 :: proc(t: ^testing.T) { decoded, err := base64.decode("Q===QUJD") - testing.expect_value(t, err, .Invalid_Padding) + // Debatable: The current implementation treats any interior padding as + // "Invalid_Padding", even though in this case an argument could + // be made for "Malformed_Quantum" instead. + testing.expect(t, err != nil) testing.expect(t, decoded == nil) } @(test) reject_strict_malformed_quantum_3 :: proc(t: ^testing.T) { decoded, err := base64.decode("Q===QUJD", options = { .Strict }) - testing.expect_value(t, err, .Invalid_Padding) + // Debatable: The current implementation treats any interior padding as + // "Invalid_Padding", even though in this case an argument could + // be made for "Malformed_Quantum" instead. + testing.expect(t, err != nil) testing.expect(t, decoded == nil) } @@ -594,8 +596,7 @@ writer_path_malformed_tails :: proc(t: ^testing.T) { writer := io.Writer { procedure = dynamic_array_stream_proc, data = cast(rawptr)&backing } err := base64.decode_into(writer, "QUJDQ") - // Debatable -> should be malformed quantum - testing.expect(t, err != nil) + testing.expect_value(t, err, .Malformed_Quantum) } @(test) diff --git a/tests/issues/test_issue_7763/test_issue_7763.odin b/tests/issues/test_issue_7763/test_issue_7763.odin index c55ed6941..92a10c155 100644 --- a/tests/issues/test_issue_7763/test_issue_7763.odin +++ b/tests/issues/test_issue_7763/test_issue_7763.odin @@ -47,7 +47,7 @@ test_issue_7763__3eq :: proc(t: ^testing.T) { test_issue_7763__Aeq :: proc(t: ^testing.T) { input := "A=" decoded, err := base64.decode(input) - testing.expect_value(t, err, .Invalid_Padding) + testing.expect_value(t, err, .Malformed_Quantum) testing.expect(t, decoded == nil) } @@ -111,7 +111,7 @@ test_issue_7763__strict_eq :: proc(t: ^testing.T) { test_issue_7763__strict_Aeq :: proc(t: ^testing.T) { input := "A=" decoded, err := base64.decode(input, options = { .Strict }) - testing.expect_value(t, err, .Invalid_Padding) + testing.expect_value(t, err, .Malformed_Quantum) testing.expect(t, decoded == nil) }