diff --git a/internal/server/app_test.go b/internal/server/app_test.go index 0eabfbe2..285eb867 100644 --- a/internal/server/app_test.go +++ b/internal/server/app_test.go @@ -78,7 +78,8 @@ func TestHttpToLambdaRequest_XForwardedFor(t *testing.T) { req.Header.Set("X-Forwarded-For", "1.2.3.4, 5.6.7.8") req.Header.Set("User-Agent", "TestAgent/1.0") - lambdaReq := httpToLambdaRequest(req) + lambdaReq, err := httpToLambdaRequest(httptest.NewRecorder(), req) + testutil.AssertNoError(t, err) testutil.AssertEqual(t, "5.6.7.8", lambdaReq.RequestContext.HTTP.SourceIP) testutil.AssertEqual(t, "TestAgent/1.0", lambdaReq.RequestContext.HTTP.UserAgent) @@ -88,7 +89,8 @@ func TestHttpToLambdaRequest_NilBody(t *testing.T) { req := httptest.NewRequestWithContext(context.Background(), "GET", "/api/test", nil) req.Body = nil - lambdaReq := httpToLambdaRequest(req) + lambdaReq, err := httpToLambdaRequest(httptest.NewRecorder(), req) + testutil.AssertNoError(t, err) testutil.AssertEqual(t, "", lambdaReq.Body) } diff --git a/internal/server/handler.go b/internal/server/handler.go index 1d0d3ee5..a24dc176 100644 --- a/internal/server/handler.go +++ b/internal/server/handler.go @@ -287,8 +287,9 @@ func (app *Application) handleSendNotifications(ctx context.Context) (*purchase. func (app *Application) handleCleanupExpiredRecords(ctx context.Context) (map[string]int64, error) { log.Println("Cleaning up expired records...") + // No sessions_deleted counter: CleanupExpiredSessions reports no count, + // so it could only ever be a fabricated 0 (#326). result := map[string]int64{ - "sessions_deleted": 0, "executions_deleted": 0, } @@ -316,7 +317,7 @@ func (app *Application) handleCleanupExpiredRecords(ctx context.Context) (map[st } } - log.Printf("Cleanup complete: %d sessions, %d executions deleted", result["sessions_deleted"], result["executions_deleted"]) + log.Printf("Cleanup complete: %d executions deleted", result["executions_deleted"]) return result, nil } @@ -385,11 +386,11 @@ func (app *Application) handleFinalizeRevocations(ctx context.Context) (*purchas func (app *Application) handleRefreshAnalytics(ctx context.Context) (map[string]any, error) { log.Println("Refreshing analytics...") + // No partition counters: this task never creates or drops partitions + // (handleCollectAnalytics does), so they could only be fabricated zeros. result := map[string]any{ - "status": "success", - "views_refreshed": 0, - "partitions_created": 0, - "partitions_dropped": 0, + "status": "success", + "views_refreshed": 0, } // Refresh materialized views if analytics store is available. @@ -406,6 +407,9 @@ func (app *Application) handleRefreshAnalytics(ctx context.Context) (map[string] log.Println("Materialized views refreshed successfully") } } else { + // Nothing ran, so this is not a success; handleCollectAnalytics + // reports the same condition as "skipped" (#326). + result["status"] = "skipped" log.Println("Analytics store not available, skipping materialized view refresh") } diff --git a/internal/server/handler_coverage_test.go b/internal/server/handler_coverage_test.go index 7e900c1d..8bf1446b 100644 --- a/internal/server/handler_coverage_test.go +++ b/internal/server/handler_coverage_test.go @@ -31,8 +31,12 @@ func TestHandleCleanupExpiredRecords_WithAuthAndConfig(t *testing.T) { result, err := app.handleCleanupExpiredRecords(ctx) testutil.AssertNoError(t, err) testutil.AssertTrue(t, result != nil, "expected non-nil result map") + // CleanupExpiredSessions reports no count, so the result must not carry a + // sessions_deleted counter that is always 0 (#326, A06-018). _, hasSessionsKey := result["sessions_deleted"] - testutil.AssertTrue(t, hasSessionsKey, "expected sessions_deleted key") + testutil.AssertTrue(t, !hasSessionsKey, "sessions_deleted is never populated and must not be reported") + _, hasExecutionsKey := result["executions_deleted"] + testutil.AssertTrue(t, hasExecutionsKey, "expected executions_deleted key") } func TestHandleCleanupExpiredRecords_NilAuthAndConfig(t *testing.T) { @@ -97,6 +101,19 @@ func TestHandleRefreshAnalytics_Success(t *testing.T) { result, err := app.handleRefreshAnalytics(ctx) testutil.AssertNoError(t, err) testutil.AssertEqual(t, "success", result["status"]) + testutil.AssertEqual(t, 1, result["views_refreshed"]) + assertNoPartitionCounters(t, result) +} + +// assertNoPartitionCounters checks that handleRefreshAnalytics does not report +// partition counters: it never creates or drops partitions, so they could only +// ever be fabricated zeros (#326, A06-018). +func assertNoPartitionCounters(t *testing.T, result map[string]any) { + t.Helper() + for _, key := range []string{"partitions_created", "partitions_dropped"} { + _, ok := result[key] + testutil.AssertTrue(t, !ok, key+" is never populated and must not be reported") + } } func TestHandleRefreshAnalytics_RefreshError(t *testing.T) { @@ -117,6 +134,11 @@ func TestHandleRefreshAnalytics_NilAnalytics(t *testing.T) { result, err := app.handleRefreshAnalytics(ctx) testutil.AssertNoError(t, err) testutil.AssertTrue(t, result != nil, "expected non-nil result") + // Nothing was refreshed, so the run must not report success (#326, + // A06-018); handleCollectAnalytics reports "skipped" for the same case. + testutil.AssertEqual(t, "skipped", result["status"]) + testutil.AssertEqual(t, 0, result["views_refreshed"]) + assertNoPartitionCounters(t, result) } // ----- configExchangeStoreAdapter — CompleteRIExchange / FailRIExchange ----- diff --git a/internal/server/handler_ladder.go b/internal/server/handler_ladder.go index 16338133..6cc1897c 100644 --- a/internal/server/handler_ladder.go +++ b/internal/server/handler_ladder.go @@ -142,9 +142,8 @@ func (app *Application) resolveLadderAccount(ctx context.Context) (accountID, re } // defaultLadderAccountResolver resolves the Lambda's own AWS account ID via STS -// STRICTLY: unlike resolveAccountID (which returns the "unknown" sentinel for -// audit-only use in the RI-exchange path), this returns an error when the -// account cannot be determined. The ladder path uses the account ID to SCOPE +// STRICTLY, like resolveAccountID in the RI-exchange path: this returns an +// error when the account cannot be determined. The ladder path uses the account ID to SCOPE // which configs run, so a fabricated/"unknown" value is unsafe: it would skip // every config as multi-account and report a false success. func defaultLadderAccountResolver(ctx context.Context) (accountID, region string, err error) { diff --git a/internal/server/handler_ri_exchange.go b/internal/server/handler_ri_exchange.go index e6b2c0b8..a911dda3 100644 --- a/internal/server/handler_ri_exchange.go +++ b/internal/server/handler_ri_exchange.go @@ -2,11 +2,11 @@ package server import ( "context" + "errors" "fmt" "log" "time" - "github.com/aws/aws-sdk-go-v2/aws" awsconfig "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/sts" @@ -52,6 +52,10 @@ func (app *Application) handleRIExchangeReshape(ctx context.Context) (*exchange. if err != nil { return nil, fmt.Errorf("failed to load AWS config: %w", err) } + accountID, err := resolveAccountID(ctx, sts.NewFromConfig(awsCfg)) + if err != nil { + return nil, fmt.Errorf("RI exchange reshape aborted: %w", err) + } ec2Client := awsprovider.NewEC2ClientDirect(awsCfg) clients := riExchangeClients{ @@ -64,7 +68,7 @@ func (app *Application) handleRIExchangeReshape(ctx context.Context) (*exchange. }, exchangeClient: exchange.NewExchangeClient(awsCfg), lookupOffering: newConvertibleOfferingLookup(ec2Client), - accountID: resolveAccountID(ctx, awsCfg), + accountID: accountID, region: awsCfg.Region, } @@ -127,6 +131,11 @@ func newConvertibleOfferingLookup(finder convertibleOfferingFinder) exchange.Loo // executeRIExchangeReshape contains the core exchange logic, separated from AWS // client construction for testability. func (app *Application) executeRIExchangeReshape(ctx context.Context, cfg *config.GlobalConfig, clients riExchangeClients) (*exchange.AutoExchangeResult, error) { + // Every record the run creates carries this account ID for the audit + // trail, so a run without one stops before touching AWS or the store. + if clients.accountID == "" { + return nil, errors.New("no AWS account ID resolved for the RI exchange audit trail") + } instances, err := clients.listConvertibleRIs(ctx) if err != nil { return nil, fmt.Errorf("failed to list convertible RIs: %w", err) @@ -174,19 +183,26 @@ func (app *Application) executeRIExchangeReshape(ctx context.Context, cfg *confi return result, nil } -// resolveAccountID fetches the AWS account ID via STS. Returns "unknown" on failure -// since account_id is stored for audit trail only and is not used to scope queries. -func resolveAccountID(ctx context.Context, awsCfg aws.Config) string { - stsClient := sts.NewFromConfig(awsCfg) - identity, err := stsClient.GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{}) +// callerIdentityGetter is the slice of the STS client resolveAccountID needs. +// Satisfied by *sts.Client. +type callerIdentityGetter interface { + GetCallerIdentity(ctx context.Context, params *sts.GetCallerIdentityInput, optFns ...func(*sts.Options)) (*sts.GetCallerIdentityOutput, error) +} + +// resolveAccountID fetches the caller's AWS account ID via STS and fails when +// it cannot be determined. The ID is written onto every RI exchange record the +// run creates, so a placeholder such as "unknown" would put a fabricated +// account on the audit trail of a money action (#326); the run is aborted +// instead, as defaultLadderAccountResolver does. +func resolveAccountID(ctx context.Context, client callerIdentityGetter) (string, error) { + identity, err := client.GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{}) if err != nil { - log.Printf("Warning: failed to get AWS account ID via STS: %v (using 'unknown')", err) - return "unknown" + return "", fmt.Errorf("resolve caller AWS account via STS: %w", err) } - if identity.Account != nil { - return *identity.Account + if identity == nil || identity.Account == nil || *identity.Account == "" { + return "", errors.New("STS GetCallerIdentity returned no account ID") } - return "unknown" + return *identity.Account, nil } // sendExchangeNotification sends email notifications based on the exchange result. diff --git a/internal/server/handler_ri_exchange_account_test.go b/internal/server/handler_ri_exchange_account_test.go new file mode 100644 index 00000000..22d35441 --- /dev/null +++ b/internal/server/handler_ri_exchange_account_test.go @@ -0,0 +1,101 @@ +package server + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/sts" + + "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" + ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/testutil" +) + +// fakeCallerIdentity is a canned STS GetCallerIdentity response. +type fakeCallerIdentity struct { + out *sts.GetCallerIdentityOutput + err error +} + +func (f fakeCallerIdentity) GetCallerIdentity(context.Context, *sts.GetCallerIdentityInput, ...func(*sts.Options)) (*sts.GetCallerIdentityOutput, error) { + return f.out, f.err +} + +// resolveAccountID must fail instead of inventing an "unknown" account ID, +// which used to be written onto every RI exchange audit row the run created +// (#326, A06-025). +func TestResolveAccountID_RefusesToFabricateAnAccount(t *testing.T) { + cases := map[string]fakeCallerIdentity{ + "STS error": {err: errors.New("ExpiredToken: the security token included in the request is expired")}, + "nil output": {}, + "nil account": {out: &sts.GetCallerIdentityOutput{}}, + "empty account": {out: &sts.GetCallerIdentityOutput{Account: aws.String("")}}, + } + for name, fake := range cases { + t.Run(name, func(t *testing.T) { + accountID, err := resolveAccountID(context.Background(), fake) + + testutil.AssertError(t, err) + testutil.AssertEqual(t, "", accountID) + }) + } +} + +func TestResolveAccountID_ReturnsTheCallerAccount(t *testing.T) { + fake := fakeCallerIdentity{out: &sts.GetCallerIdentityOutput{Account: aws.String("123456789012")}} + + accountID, err := resolveAccountID(context.Background(), fake) + + testutil.AssertNoError(t, err) + testutil.AssertEqual(t, "123456789012", accountID) +} + +// A reshape run without a resolved account ID must stop before it lists RIs +// or writes any exchange record, so no audit row carries a blank account. +func TestExecuteRIExchangeReshape_RefusesEmptyAccountID(t *testing.T) { + ctx := testutil.TestContext(t) + writes := 0 + store := &mockConfigStoreForExchange{ + saveRIExchangeRecordFunc: func(context.Context, *config.RIExchangeRecord) error { + writes++ + return nil + }, + reserveRIExchangeFunc: func(context.Context, *config.RIExchangeRecord, string, string) (string, error) { + writes++ + return "", errors.New("unexpected reservation") + }, + } + app := &Application{Config: store} + cfg := &config.GlobalConfig{ + RIExchangeEnabled: true, + RIExchangeMode: "auto", + RIExchangeUtilizationThreshold: 95.0, + RIExchangeLookbackDays: 30, + } + + listed := false + clients := riExchangeClients{ + listConvertibleRIs: func(context.Context) ([]ec2svc.ConvertibleRI, error) { + listed = true + return nil, nil + }, + getRIUtilization: func(context.Context, int) ([]recommendations.RIUtilization, error) { + return nil, nil + }, + exchangeClient: &mockExchangeClient{}, + accountID: "", + region: "us-east-1", + } + + result, err := app.executeRIExchangeReshape(ctx, cfg, clients) + + testutil.AssertError(t, err) + testutil.AssertTrue(t, strings.Contains(err.Error(), "account ID"), "expected an account ID error, got: "+err.Error()) + testutil.AssertTrue(t, result == nil, "expected no result") + testutil.AssertTrue(t, !listed, "convertible RIs must not be listed without an account ID") + testutil.AssertEqual(t, 0, writes) +} diff --git a/internal/server/handler_ri_exchange_test.go b/internal/server/handler_ri_exchange_test.go index 6d4d998b..55f1085f 100644 --- a/internal/server/handler_ri_exchange_test.go +++ b/internal/server/handler_ri_exchange_test.go @@ -489,6 +489,7 @@ func TestExecuteRIExchangeReshape_ListRIsFailure(t *testing.T) { listConvertibleRIs: func(ctx context.Context) ([]ec2svc.ConvertibleRI, error) { return nil, errors.New("EC2 API throttled") }, + accountID: "123456789", } _, err := app.executeRIExchangeReshape(ctx, cfg, clients) @@ -517,6 +518,7 @@ func TestExecuteRIExchangeReshape_UtilizationFailure(t *testing.T) { getRIUtilization: func(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) { return nil, errors.New("Cost Explorer unavailable") }, + accountID: "123456789", } _, err := app.executeRIExchangeReshape(ctx, cfg, clients) diff --git a/internal/server/http.go b/internal/server/http.go index 8bbf2261..c562e5ce 100644 --- a/internal/server/http.go +++ b/internal/server/http.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "log" "net/http" "os/signal" @@ -231,7 +230,11 @@ func (app *Application) handleOIDCHTTP(w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) defer cancel() - lambdaReq := httpToLambdaRequest(r) + lambdaReq, err := httpToLambdaRequest(w, r) + if err != nil { + writeBodyReadError(w, err) + return + } resp, handled := app.API.HandleOIDC(ctx, lambdaReq) if !handled { // Path matched /oidc/ prefix but is not a recognized OIDC endpoint. @@ -279,7 +282,11 @@ func (app *Application) handleHTTPRequest(w http.ResponseWriter, r *http.Request } // Convert HTTP request to Lambda Function URL request format - lambdaReq := httpToLambdaRequest(r) + lambdaReq, err := httpToLambdaRequest(w, r) + if err != nil { + writeBodyReadError(w, err) + return + } // Call the API handler lambdaResp, err := app.API.HandleRequest(ctx, lambdaReq) @@ -303,7 +310,12 @@ func (app *Application) handleVersion(w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second) defer cancel() - lambdaResp, err := app.API.HandleRequest(ctx, httpToLambdaRequest(r)) + lambdaReq, err := httpToLambdaRequest(w, r) + if err != nil { + writeBodyReadError(w, err) + return + } + lambdaResp, err := app.API.HandleRequest(ctx, lambdaReq) if err != nil { log.Printf("Version handler error: %v", err) http.Error(w, "Internal server error", http.StatusInternalServerError) @@ -393,18 +405,13 @@ func (app *Application) handleScheduledHTTP(w http.ResponseWriter, r *http.Reque } } -// httpToLambdaRequest converts a standard HTTP request to Lambda Function URL request format. -func httpToLambdaRequest(r *http.Request) *events.LambdaFunctionURLRequest { - // Read body with size limit to prevent memory exhaustion - body := "" - if r.Body != nil { - defer r.Body.Close() - const maxBodySize = 10 << 20 // 10 MB - limited := io.LimitReader(r.Body, maxBodySize) - bodyBytes, err := io.ReadAll(limited) - if err == nil && len(bodyBytes) > 0 { - body = string(bodyBytes) - } +// httpToLambdaRequest converts a standard HTTP request to Lambda Function URL +// request format. It fails when the body cannot be read whole; answer that +// with writeBodyReadError. +func httpToLambdaRequest(w http.ResponseWriter, r *http.Request) (*events.LambdaFunctionURLRequest, error) { + body, err := readRequestBody(w, r) + if err != nil { + return nil, err } // Convert headers - lowercase all keys to match Lambda Function URL behavior. @@ -451,7 +458,7 @@ func httpToLambdaRequest(r *http.Request) *events.LambdaFunctionURLRequest { QueryStringParameters: queryParams, Body: body, IsBase64Encoded: false, - } + }, nil } // safeHeaderNames is a whitelist of headers that are safe to pass through from Lambda responses. diff --git a/internal/server/http_body.go b/internal/server/http_body.go new file mode 100644 index 00000000..db19fba4 --- /dev/null +++ b/internal/server/http_body.go @@ -0,0 +1,47 @@ +package server + +import ( + "encoding/json" + "errors" + "io" + "log" + "net/http" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/api" +) + +// readRequestBody reads the whole request body, capped at the API router's +// own limit (api.MaxRequestBodySize) so the transport never buffers a body +// the router would reject anyway. http.MaxBytesReader makes an oversize body +// an error rather than truncating it, and a read error (a client that +// disconnects mid-upload) is returned rather than read as an empty body +// (#326). +func readRequestBody(w http.ResponseWriter, r *http.Request) (string, error) { + if r.Body == nil { + return "", nil + } + defer r.Body.Close() + bodyBytes, err := io.ReadAll(http.MaxBytesReader(w, r.Body, api.MaxRequestBodySize)) + if err != nil { + return "", err + } + return string(bodyBytes), nil +} + +// writeBodyReadError answers a failed body read: 413 for a body over the cap, +// with the same JSON error the API router returns for one, and 400 for any +// other read failure. +func writeBodyReadError(w http.ResponseWriter, err error) { + status, msg := http.StatusBadRequest, "Failed to read request body" + var tooLarge *http.MaxBytesError + if errors.As(err, &tooLarge) { + status, msg = http.StatusRequestEntityTooLarge, "Request body too large" + } else { + log.Printf("Failed to read request body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if encErr := json.NewEncoder(w).Encode(map[string]string{"error": msg}); encErr != nil { + log.Printf("Failed to encode body-read error response: %v", encErr) + } +} diff --git a/internal/server/http_body_test.go b/internal/server/http_body_test.go new file mode 100644 index 00000000..25aee615 --- /dev/null +++ b/internal/server/http_body_test.go @@ -0,0 +1,121 @@ +package server + +import ( + "bytes" + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/api" + "github.com/LeanerCloud/cloud-commitments-platform/internal/testutil" +) + +// requestBodyLimit is the cap the HTTP transport applies to request bodies: +// the API router's own limit, so the two cannot disagree. +const requestBodyLimit = api.MaxRequestBodySize + +// failingBody yields some bytes and then a read error, like a client that +// disconnects mid-upload. +type failingBody struct { + sent bool +} + +func (b *failingBody) Read(p []byte) (int, error) { + if !b.sent { + b.sent = true + return copy(p, `{"partial":`), nil + } + return 0, errors.New("connection reset by peer") +} + +func (b *failingBody) Close() error { return nil } + +// bodyHandlers lists every HTTP handler that converts the request through +// httpToLambdaRequest, so each one is checked for the same body handling. +func bodyHandlers(app *Application) map[string]http.HandlerFunc { + return map[string]http.HandlerFunc{ + "handleHTTPRequest": app.handleHTTPRequest, + "handleVersion": app.handleVersion, + "handleOIDCHTTP": app.handleOIDCHTTP, + } +} + +// An oversize body must be rejected with 413, not truncated at the cap and +// handed to the API router cut mid-JSON (#326, A06-024). +func TestHTTPHandlers_RejectOversizeBodyWith413(t *testing.T) { + app := &Application{API: api.NewHandler(api.HandlerConfig{})} + for name, h := range bodyHandlers(app) { + t.Run(name, func(t *testing.T) { + body := bytes.Repeat([]byte("a"), requestBodyLimit+1) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/api/recommendations", bytes.NewReader(body)) + rec := httptest.NewRecorder() + + h(rec, req) + + testutil.AssertEqual(t, http.StatusRequestEntityTooLarge, rec.Code) + testutil.AssertEqual(t, "application/json", rec.Header().Get("Content-Type")) + testutil.AssertTrue(t, strings.Contains(rec.Body.String(), `"error":"Request body too large"`), + "expected the router's JSON 413 body, got: "+rec.Body.String()) + }) + } +} + +// A body read error must be answered with 400, not read as an empty body and +// routed (#326, A06-024). +func TestHTTPHandlers_RejectBodyReadErrorWith400(t *testing.T) { + app := &Application{API: api.NewHandler(api.HandlerConfig{})} + for name, h := range bodyHandlers(app) { + t.Run(name, func(t *testing.T) { + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/api/recommendations", nil) + req.Body = &failingBody{} + rec := httptest.NewRecorder() + + h(rec, req) + + testutil.AssertEqual(t, http.StatusBadRequest, rec.Code) + testutil.AssertTrue(t, strings.Contains(rec.Body.String(), "Failed to read request body"), + "expected the body-read error message, got: "+rec.Body.String()) + }) + } +} + +// A body of exactly the cap is still accepted whole. +func TestHttpToLambdaRequest_BodyAtLimitIsKeptWhole(t *testing.T) { + body := bytes.Repeat([]byte("a"), requestBodyLimit) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/api/test", bytes.NewReader(body)) + + lambdaReq, err := httpToLambdaRequest(httptest.NewRecorder(), req) + + testutil.AssertNoError(t, err) + testutil.AssertEqual(t, requestBodyLimit, len(lambdaReq.Body)) +} + +// One byte over the cap is an error carrying *http.MaxBytesError, not a body +// silently truncated to the cap. +func TestHttpToLambdaRequest_OversizeBodyIsAnError(t *testing.T) { + body := bytes.Repeat([]byte("a"), requestBodyLimit+1) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/api/test", bytes.NewReader(body)) + + lambdaReq, err := httpToLambdaRequest(httptest.NewRecorder(), req) + + testutil.AssertError(t, err) + var tooLarge *http.MaxBytesError + testutil.AssertTrue(t, errors.As(err, &tooLarge), "expected *http.MaxBytesError, got: "+err.Error()) + testutil.AssertTrue(t, lambdaReq == nil, "expected no request on an oversize body") +} + +// A read error is returned, not swallowed into an empty body. +func TestHttpToLambdaRequest_ReadErrorIsReturned(t *testing.T) { + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/api/test", nil) + req.Body = &failingBody{} + + lambdaReq, err := httpToLambdaRequest(httptest.NewRecorder(), req) + + testutil.AssertError(t, err) + testutil.AssertTrue(t, !errors.Is(err, io.EOF), "a read error must not look like a clean EOF") + testutil.AssertTrue(t, lambdaReq == nil, "expected no request on a read error") +} diff --git a/internal/server/http_test.go b/internal/server/http_test.go index b24d32e2..2a2dafe2 100644 --- a/internal/server/http_test.go +++ b/internal/server/http_test.go @@ -85,7 +85,8 @@ func TestHttpToLambdaRequest(t *testing.T) { } // Convert to Lambda request - lambdaReq := httpToLambdaRequest(req) + lambdaReq, err := httpToLambdaRequest(httptest.NewRecorder(), req) + testutil.AssertNoError(t, err) // Verify conversion testutil.AssertEqual(t, tt.expectedMethod, lambdaReq.RequestContext.HTTP.Method)