diff --git a/cmd/storage-rest-server-guard.go b/cmd/storage-rest-server-guard.go index 99efcb2fd..c3169b326 100644 --- a/cmd/storage-rest-server-guard.go +++ b/cmd/storage-rest-server-guard.go @@ -83,9 +83,9 @@ type guardedStorage struct { // short paths used here, so " " and "..." reach the syscall as an empty // component. On Unix they are ordinary filenames and legal object keys. // -// Note the second point is reasoned from documented Win32 behaviour, not from a +// Note the second point is reasoned from documented Win32 behavior, not from a // Windows test run: CI is Linux-only, so TestIsVolumeRootAliasIsPlatformCorrect -// pins both branches of the predicate rather than the syscall behaviour itself. +// pins both branches of the predicate rather than the syscall behavior itself. func isVolumeRootAlias(p string) bool { return isVolumeRootAliasOn(p, runtime.GOOS == globalWindowsOSName) } @@ -125,7 +125,7 @@ func isVolumeRootAliasOn(p string, windows bool) bool { // for them, so they say nothing about whether the metadata is sound. // // This is the boundary check; ShardFileSize's own zero-value guard stays as the -// last line of defence against a panic. +// last line of defense against a panic. func guardErasureParams(fi FileInfo) error { // Negative sizes share their rule with the storage layer, which also has to // cope with metadata already on disk; keep the two from drifting apart by diff --git a/cmd/storage-rest-server-guard_test.go b/cmd/storage-rest-server-guard_test.go index 152b2dcdf..0216a09d6 100644 --- a/cmd/storage-rest-server-guard_test.go +++ b/cmd/storage-rest-server-guard_test.go @@ -190,13 +190,22 @@ func TestIsVolumeRootAlias(t *testing.T) { want bool }{ // Collapse back to the volume directory. - {"", true}, {"/", true}, {"//", true}, + {"", true}, + {"/", true}, + {"//", true}, // Backslash is platform-dependent; see TestIsVolumeRootAliasIsPlatformCorrect. // Whitespace is NOT a separator. path.Clean leaves it alone, so these // name real directories and are legal S3 object keys. - {" ", false}, {" ", false}, {"\t", false}, {"\n", false}, - {" / ", false}, {"/ ", false}, - {"a", false}, {"/a", false}, {"..", false}, {" a ", false}, + {" ", false}, + {" ", false}, + {"\t", false}, + {"\n", false}, + {" / ", false}, + {"/ ", false}, + {"a", false}, + {"/a", false}, + {"..", false}, + {" a ", false}, {"obj/part.1", false}, } { if got := isVolumeRootAlias(tc.path); got != tc.want { @@ -239,12 +248,19 @@ func TestIsVolumeRootAliasIsPlatformCorrect(t *testing.T) { // Space and period: ordinary filename characters on Unix, but stripped // from a component by Win32 normalisation, so a component made only of // them vanishes and the path resolves to the volume root. - {" ", false, true}, {" ", false, true}, {" / ", false, true}, - {"...", false, true}, {". .", false, true}, + {" ", false, true}, + {" ", false, true}, + {" / ", false, true}, + {"...", false, true}, + {". .", false, true}, // Never an alias anywhere. - {"\t", false, false}, {"\n", false, false}, - {"a", false, false}, {"/a", false, false}, {"a ", false, false}, - {" a", false, false}, {"a.", false, false}, + {"\t", false, false}, + {"\n", false, false}, + {"a", false, false}, + {"/a", false, false}, + {"a ", false, false}, + {" a", false, false}, + {"a.", false, false}, } { if got := isVolumeRootAliasOn(tc.path, false); got != tc.unix { t.Errorf("isVolumeRootAliasOn(%q, unix) = %v, want %v", tc.path, got, tc.unix) diff --git a/cmd/storage-rest-server.go b/cmd/storage-rest-server.go index 2b0562b9b..0b65edd6b 100644 --- a/cmd/storage-rest-server.go +++ b/cmd/storage-rest-server.go @@ -597,11 +597,14 @@ func (s *storageRESTServer) ReadPartsHandler(w http.ResponseWriter, r *http.Requ done := keepHTTPResponseAlive(w) infos, err := s.getStorage().ReadParts(r.Context(), volume, preq.Paths...) - done(nil) if err != nil { - s.writeErrorResponse(w, err) + // The keep-alive stream owns the response body from here on, so the + // error has to travel through done(); writing a header afterwards is + // too late and leaves the client decoding the error text as msgpack. + done(err) return } + done(nil) presp := &ReadPartsResp{Infos: infos} storageLogIf(r.Context(), msgp.Encode(w, presp)) diff --git a/cmd/storage-rest-traversal_test.go b/cmd/storage-rest-traversal_test.go index cf34d65ef..d1bd4358f 100644 --- a/cmd/storage-rest-traversal_test.go +++ b/cmd/storage-rest-traversal_test.go @@ -469,10 +469,14 @@ func TestCheckPartsMalformedErasure(t *testing.T) { // Deleted short-circuits FileInfo.IsValid() to true, so an IsValid() // guard would not have caught this one. {Volume: "foo", Name: "obj", Deleted: true, Parts: []ObjectPartInfo{{Number: 1, Size: 4}}}, - {Volume: "foo", Name: "obj", Parts: []ObjectPartInfo{{Number: 1, Size: 4}}, - Erasure: ErasureInfo{DataBlocks: 4}}, // BlockSize still zero - {Volume: "foo", Name: "obj", Parts: []ObjectPartInfo{{Number: 1, Size: 4}}, - Erasure: ErasureInfo{BlockSize: blockSizeV2}}, // DataBlocks still zero + { + Volume: "foo", Name: "obj", Parts: []ObjectPartInfo{{Number: 1, Size: 4}}, + Erasure: ErasureInfo{DataBlocks: 4}, + }, // BlockSize still zero + { + Volume: "foo", Name: "obj", Parts: []ObjectPartInfo{{Number: 1, Size: 4}}, + Erasure: ErasureInfo{BlockSize: blockSizeV2}, + }, // DataBlocks still zero } { if _, err := restClient.CheckParts(ctx, "foo", "obj", fi); !errors.Is(err, errFileCorrupt) { t.Errorf("CheckParts(%+v): got %v, want %v", fi.Erasure, err, errFileCorrupt) @@ -584,7 +588,7 @@ func TestDeleteVersionsDeclaredCountIsNotTrusted(t *testing.T) { grew := after.TotalAlloc - before.TotalAlloc t.Logf("total-versions=%s -> err=%v, allocated %d bytes", total, err, grew) // 100000000 * sizeof(FileInfoVersions)(104) is ~9.7 GiB; anything in - // that neighbourhood means the declared count is still being trusted. + // that neighborhood means the declared count is still being trusted. if grew > 64<<20 { t.Errorf("total-versions=%s allocated %d bytes for an empty body - "+ "the declared count is sizing the allocation", total, grew)