diff --git a/rdma/rcroutes/errors.go b/rdma/rcroutes/errors.go index 5f3d0627..007c048e 100644 --- a/rdma/rcroutes/errors.go +++ b/rdma/rcroutes/errors.go @@ -51,9 +51,6 @@ func WriteRouteError(ctx fiber.Ctx, err error) error { requestID, hostID := utils.EnsureRequestIDs(ctx) apiErr := routeError(err) - if apiErr.HTTPStatusCode == fiber.StatusInternalServerError { - logInternalRouteError(ctx, err) - } if isRouteNotImplemented(err) { apiErr = s3err.APIError{ Code: "NotImplemented", @@ -61,10 +58,23 @@ func WriteRouteError(ctx fiber.Ctx, err error) error { HTTPStatusCode: fiber.StatusNotImplemented, } } + if apiErr.HTTPStatusCode == fiber.StatusInternalServerError { + logInternalRouteError(ctx, err) + } + + // A full S3 error keeps its own richer XML body - per-type + // diagnostics such as signature details would be lost if + // only the base error were serialized. + var body []byte + var s3Err s3err.S3Error + if errors.As(err, &s3Err) { + body = s3Err.XMLBody(requestID, hostID) + } else { + body = apiErr.XMLBody(requestID, hostID) + } ctx.Response().Header.SetContentType(fiber.MIMEApplicationXML) - return ctx.Status(apiErr.HTTPStatusCode). - Send(apiErr.XMLBody(requestID, hostID)) + return ctx.Status(apiErr.HTTPStatusCode).Send(body) } // routeError resolves err to its S3-style response. Errors that diff --git a/s3api/rc_route_error_test.go b/s3api/rc_route_error_test.go index 680677b5..3cfe7c46 100644 --- a/s3api/rc_route_error_test.go +++ b/s3api/rc_route_error_test.go @@ -14,8 +14,11 @@ package s3api import ( + "bytes" "encoding/xml" "errors" + "fmt" + "io" "net/http" "net/http/httptest" "testing" @@ -71,11 +74,62 @@ func TestRCRouteErrorPreservesS3Error(t *testing.T) { if er.RequestID == "" || er.HostID == "" { t.Fatal("response missing RequestId or HostId") } + if got := resp.Header.Get("x-amz-request-id"); got != er.RequestID { + t.Fatalf("body RequestId %q != header %q", er.RequestID, got) + } + if got := resp.Header.Get("x-amz-id-2"); got != er.HostID { + t.Fatalf("body HostId %q != header %q", er.HostID, got) + } if ct := resp.Header.Get("Content-Type"); ct != fiber.MIMEApplicationXML { t.Fatalf("content-type = %q, want %q", ct, fiber.MIMEApplicationXML) } } +func TestRCRouteErrorKeepsSubtypeDiagnostics(t *testing.T) { + // A wrapped per-type S3 error must keep both its status and + // its subtype-only XML fields; serializing only the base + // error would drop the diagnostics. + server, err := newTestS3ApiServer( + WithRoute(http.MethodPost, "/.hipobj-rc/op", func(ctx fiber.Ctx) error { + inner := s3err.GetAPIError(s3err.ErrSignatureDoesNotMatch) + wrapped := s3err.SignatureDoesNotMatchError{ + AWSAccessKeyId: "AKIAEXAMPLE", + // The remaining diagnostic fields flow from the + // base through the subtype constructor in + // production; the wire contract under test is + // that the subtype body is used verbatim. + APIError: inner, + StringToSign: "EXAMPLE-STRING-TO-SIGN", + } + return rcroutes.WriteRouteError(ctx, + fmt.Errorf("auth: %w", wrapped)) + }), + ) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + resp, err := server.app.Test(httptest.NewRequest(http.MethodPost, "/.hipobj-rc/op", nil)) + if err != nil { + t.Fatalf("app.Test() error = %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusForbidden) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + for _, want := range []string{"SignatureDoesNotMatch", "AKIAEXAMPLE", + "EXAMPLE-STRING-TO-SIGN"} { + if !bytes.Contains(body, []byte(want)) { + t.Fatalf("body missing %q: %s", want, body) + } + } +} + func TestRCRouteErrorRawFiberIs500(t *testing.T) { // Control case: an ordinary fiber error collapses into the // generic 500 of the production error handler. This is the