mirror of
https://github.com/pgsty/minio.git
synced 2026-09-05 18:16:16 +03:00
fix: return 500 for unreadable objects instead of 206
ErrObjectTampered was mapped to http.StatusPartialContent since upstream
ca6b4773e (2017), so a GET or HEAD of an object the server cannot decode
(invalid encrypted size, malformed actual-size, bad multipart ETag shape)
answered with a success status and an XML error document that SDKs handed
back as object content; boto3 returned the XML as Body and a zero-length
HEAD as success. Every origin of errObjectTampered is a stored-state
defect, not caller input, so map the entry to 500 Internal Server Error
and keep the XMinioObjectTampered code and message. The comment records
the deliberate divergence from upstream.
Tests: TestObjectTamperedGETHEADStatus (signed GET and HEAD, ErasureSD and
Erasure) fails with 206 on main and passes with 500; TestAPIErrCode,
TestAPIErrCodeDefinition, TestAPIHeadObjectHandler,
TestAPIHeadObjectHandlerWithEncryption and TestAPIGetObjectHandler stay
green. Compatibility: only the status line of one MinIO-specific error
changes; clients now retry damaged-object reads per their 5xx policy.
Fixes pgsty/silo#110
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe
Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
+5
-1
@@ -1523,10 +1523,14 @@ var errorCodes = errorCodeMap{
|
|||||||
Description: "Your Host header is malformed.",
|
Description: "Your Host header is malformed.",
|
||||||
HTTPStatusCode: http.StatusBadRequest,
|
HTTPStatusCode: http.StatusBadRequest,
|
||||||
},
|
},
|
||||||
|
// The stored object cannot be served: a server-side data condition, not a
|
||||||
|
// successful partial read. Upstream maps it to http.StatusPartialContent
|
||||||
|
// (since ca6b4773e, 2017), which lets SDKs accept the XML error document
|
||||||
|
// as object content; SILO deliberately diverges and returns 500.
|
||||||
ErrObjectTampered: {
|
ErrObjectTampered: {
|
||||||
Code: "XMinioObjectTampered",
|
Code: "XMinioObjectTampered",
|
||||||
Description: errObjectTampered.Error(),
|
Description: errObjectTampered.Error(),
|
||||||
HTTPStatusCode: http.StatusPartialContent,
|
HTTPStatusCode: http.StatusInternalServerError,
|
||||||
},
|
},
|
||||||
|
|
||||||
ErrSiteReplicationInvalidRequest: {
|
ErrSiteReplicationInvalidRequest: {
|
||||||
|
|||||||
@@ -0,0 +1,64 @@
|
|||||||
|
package cmd
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/minio/minio/internal/auth"
|
||||||
|
"github.com/minio/minio/internal/crypto"
|
||||||
|
)
|
||||||
|
|
||||||
|
// tamperedObjectLayer returns a fixed error from the read paths so the handler
|
||||||
|
// response can be tested without reproducing the storage defect that produces
|
||||||
|
// an unreadable object. No object bytes are sent to the caller.
|
||||||
|
type tamperedObjectLayer struct {
|
||||||
|
ObjectLayer
|
||||||
|
err error
|
||||||
|
}
|
||||||
|
|
||||||
|
func (o *tamperedObjectLayer) GetObjectNInfo(context.Context, string, string, *HTTPRangeSpec, http.Header, ObjectOptions) (*GetObjectReader, error) {
|
||||||
|
return nil, o.err
|
||||||
|
}
|
||||||
|
|
||||||
|
func (o *tamperedObjectLayer) GetObjectInfo(context.Context, string, string, ObjectOptions) (ObjectInfo, error) {
|
||||||
|
return ObjectInfo{}, o.err
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestObjectTamperedGETHEADStatus asserts that an object the server cannot
|
||||||
|
// decode is reported with a 5xx status, not a success status. Returning
|
||||||
|
// http.StatusPartialContent here let SDKs accept the XML error document as
|
||||||
|
// object content. See pgsty/silo#110.
|
||||||
|
func TestObjectTamperedGETHEADStatus(t *testing.T) {
|
||||||
|
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{t: t, objAPITest: func(obj ObjectLayer, instanceType, bucket string, router http.Handler, credentials auth.Credentials, t *testing.T) {
|
||||||
|
// An encrypted stream shorter than one complete encryption package is
|
||||||
|
// a real size-validation origin of errObjectTampered.
|
||||||
|
damaged := ObjectInfo{Size: 31, UserDefined: map[string]string{crypto.MetaAlgorithm: crypto.InsecureSealAlgorithm}}
|
||||||
|
_, err := damaged.DecryptedSize()
|
||||||
|
if err != errObjectTampered {
|
||||||
|
t.Fatalf("damaged-size error = %v, want errObjectTampered", err)
|
||||||
|
}
|
||||||
|
previous := newObjectLayerFn()
|
||||||
|
setObjectLayer(&tamperedObjectLayer{ObjectLayer: obj, err: err})
|
||||||
|
defer setObjectLayer(previous)
|
||||||
|
for _, method := range []string{http.MethodGet, http.MethodHead} {
|
||||||
|
req, err := newTestSignedRequestV4(method, getGetObjectURL("", bucket, "damaged-object"), 0, nil, credentials.AccessKey, credentials.SecretKey, nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
router.ServeHTTP(rec, req)
|
||||||
|
if method == http.MethodGet && !strings.Contains(rec.Body.String(), "<Code>XMinioObjectTampered</Code>") {
|
||||||
|
t.Fatalf("%s: GET did not reach the damaged-object response: %d %s", instanceType, rec.Code, rec.Body.String())
|
||||||
|
}
|
||||||
|
if method == http.MethodHead && rec.Header().Get(xMinIOErrCodeHeader) != "XMinioObjectTampered" {
|
||||||
|
t.Fatalf("%s: HEAD did not reach the damaged-object response: %d %v", instanceType, rec.Code, rec.Header())
|
||||||
|
}
|
||||||
|
if rec.Code != http.StatusInternalServerError {
|
||||||
|
t.Errorf("%s: %s of a damaged object returned HTTP %d, want %d", instanceType, method, rec.Code, http.StatusInternalServerError)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}})
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user