Skip to content

Commit 296c504

Browse files
committed
Return ChecksumMismatch error instead of logging warning
Extract checksum verification from doRequest into a testable verifyResponseChecksum function that returns error.ChecksumMismatch on mismatch instead of silently logging a warning.
1 parent e16456b commit 296c504

1 file changed

Lines changed: 53 additions & 25 deletions

File tree

src/http.zig

Lines changed: 53 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -613,31 +613,7 @@ pub const HttpClient = struct {
613613
return if (err == error.StreamTooLong) error.ResponseTooLarge else error.RequestFailed;
614614
};
615615

616-
if (effective_checksum_alg) |alg| {
617-
const alg_name = algToString(alg);
618-
var key_buf: [64]u8 = undefined;
619-
const header_key = std.fmt.bufPrint(
620-
&key_buf,
621-
"x-amz-checksum-{s}",
622-
.{alg_name},
623-
) catch null;
624-
if (header_key) |k| {
625-
if (resp_headers.get(k)) |expected| {
626-
const ok = checksum_mod.verify(
627-
alg,
628-
body,
629-
expected,
630-
self.allocator,
631-
) catch false;
632-
if (!ok) {
633-
std.log.warn(
634-
"Response checksum mismatch for {s}",
635-
.{alg_name},
636-
);
637-
}
638-
}
639-
}
640-
}
616+
try verifyResponseChecksum(effective_checksum_alg, body, &resp_headers, self.allocator);
641617

642618
var final_response = Response{
643619
.status = @intFromEnum(response.head.status),
@@ -808,6 +784,25 @@ fn algToString(alg: checksum_mod.Algorithm) []const u8 {
808784
};
809785
}
810786

787+
fn verifyResponseChecksum(
788+
alg: ?checksum_mod.Algorithm,
789+
body: []const u8,
790+
resp_headers: *const std.StringHashMapUnmanaged([]const u8),
791+
allocator: std.mem.Allocator,
792+
) RequestError!void {
793+
const a = alg orelse return;
794+
var key_buf: [64]u8 = undefined;
795+
const header_key = std.fmt.bufPrint(
796+
&key_buf,
797+
"x-amz-checksum-{s}",
798+
.{algToString(a)},
799+
) catch return;
800+
if (resp_headers.get(header_key)) |expected| {
801+
const ok = checksum_mod.verify(a, body, expected, allocator) catch false;
802+
if (!ok) return error.ChecksumMismatch;
803+
}
804+
}
805+
811806
pub fn isClockSkewError(status: u16, body: []const u8) bool {
812807
if (status != 403 and status != 400) return false;
813808
const clock_skew_codes = [_][]const u8{
@@ -1728,6 +1723,39 @@ test "checksum computeBase64 and verify round-trip" {
17281723
try std.testing.expect(!bad);
17291724
}
17301725

1726+
test "verifyResponseChecksum returns error on mismatch" {
1727+
var headers: std.StringHashMapUnmanaged([]const u8) = .{};
1728+
defer headers.deinit(std.testing.allocator);
1729+
try headers.put(std.testing.allocator, "x-amz-checksum-crc32", "AAAA");
1730+
1731+
try std.testing.expectError(
1732+
error.ChecksumMismatch,
1733+
verifyResponseChecksum(.crc32, "test body", &headers, std.testing.allocator),
1734+
);
1735+
}
1736+
1737+
test "verifyResponseChecksum passes on valid checksum" {
1738+
const body = "test body";
1739+
const valid_b64 = try checksum_mod.computeBase64(std.testing.allocator, .crc32, body);
1740+
defer std.testing.allocator.free(valid_b64);
1741+
1742+
var headers: std.StringHashMapUnmanaged([]const u8) = .{};
1743+
defer headers.deinit(std.testing.allocator);
1744+
try headers.put(std.testing.allocator, "x-amz-checksum-crc32", valid_b64);
1745+
1746+
try verifyResponseChecksum(.crc32, body, &headers, std.testing.allocator);
1747+
}
1748+
1749+
test "verifyResponseChecksum is no-op without algorithm" {
1750+
var headers: std.StringHashMapUnmanaged([]const u8) = .{};
1751+
try verifyResponseChecksum(null, "anything", &headers, std.testing.allocator);
1752+
}
1753+
1754+
test "verifyResponseChecksum is no-op without matching header" {
1755+
var headers: std.StringHashMapUnmanaged([]const u8) = .{};
1756+
try verifyResponseChecksum(.crc32, "anything", &headers, std.testing.allocator);
1757+
}
1758+
17311759
test "isClockSkewError detects clock skew codes" {
17321760
try std.testing.expect(
17331761
isClockSkewError(403, "RequestTimeTooSkewed: ..."),

0 commit comments

Comments
 (0)