From 15965310036a1cbb76df1dc910859410ca071a1a Mon Sep 17 00:00:00 2001 From: Jihyeon Gim Date: Fri, 4 Sep 2026 17:30:46 +0900 Subject: [PATCH] rdma: keep RC route XML fidelity Classify the platform-stub answer before the internal-error logging decision, so the expected 501 no longer logs as an internal 500 while debugging production servers. Serialize the full S3 error XML body instead of the base error alone: per-type diagnostics such as the access key and the string-to-sign survive the route boundary. A regression test wraps a signature failure with both diagnostic fields and asserts the response keeps the status, the code, and both fields. Assert the response body identifiers equal the request-ID headers, pinning the two views of the same response. --- rdma/rcroutes/errors.go | 20 +++++++++---- s3api/rc_route_error_test.go | 54 ++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) 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