fix(gateway): add body size limit and text length validation to POST /api/ai/summarize#259
Conversation
…/api/ai/summarize Without a request body size limit, a client could send megabytes of text to the summarize endpoint. This forwards directly to OpenRouter or Ollama and causes: token cost explosion (~.50+ per oversized request vs 0.001 USDC payment), timeout degradation for all concurrent users, and GC pressure from buffering large bodies in Go. Changes: - gateway/internal/middleware/body_limit.go: New BodySizeLimit(maxBytes) Gin middleware using http.MaxBytesReader. Rejects oversized bodies before any handler logic runs. Exports MaxSummarizeBodyBytes = 32KB constant. - gateway/handlers/summarize.go: Add application-layer text length check after JSON parsing. Reads MAX_SUMMARIZE_TEXT_LENGTH from env (default 8000). Returns structured JSON errors for body_too_large, text_too_long, text_empty. getMaxTextLength() helper allows operator tuning without code changes. - gateway/ (router): Apply BodySizeLimit middleware to /api/ai/summarize route. - .env.example: Document MAX_SUMMARIZE_TEXT_LENGTH with explanation of why the limit exists and how to tune it for different AI provider token budgets. Error responses: 413 body_too_large — raw body exceeds 32KB before JSON parsing 413 text_too_long — text field exceeds 8000 chars after JSON parsing 400 text_empty — text field is present but empty Closes AnkanMisra#256
|
@divyanshim27 is attempting to deploy a commit to the ankanmisra's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @divyanshim27, thanks for opening this PR. Every contribution helps MicroAI-Paygate grow. If you find the project useful, consider starring the repository — it helps others discover it. Star MicroAI-Paygate on GitHub Looking forward to reviewing this PR. |
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughThe summarize endpoint now limits request bodies to 32 KiB, validates required and non-empty text, enforces a configurable 8,000-character default, and returns specific 400 or 413 errors. The limit middleware is applied regardless of cache configuration. ChangesSummarize request limits
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
gateway/handlers/summarise.go (1)
19-27: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
errors.Asinstead of string comparison forMaxBytesError.Checking
err.Error() == "http: request body too large"is fragile — the error message could change in future Go versions. Useerrors.As(err, &maxBytesErr)for type-safe detection, consistent with the pattern already used inmain.go(line 474-475).♻️ Proposed fix
import ( "fmt" "net/http" "os" "strconv" "github.com/gin-gonic/gin" ) @@ -19,11 +19,14 @@ if err := c.ShouldBindJSON(&req); err != nil { - if err.Error() == "http: request body too large" { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { c.JSON(http.StatusRequestEntityTooLarge, gin.H{ "error": "body_too_large", "message": "Request body exceeds the maximum allowed size.", "max_size_kb": 32, }) return } + // also need to import "errors"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gateway/handlers/summarise.go` around lines 19 - 27, Replace the fragile err.Error() string comparison in the summarise handler’s JSON binding error path with errors.As targeting *http.MaxBytesError, declaring the typed error variable and preserving the existing 413 response for that case. Add the required errors import and follow the existing main.go pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gateway/handlers/summarise.go`:
- Around line 58-69: Remove the duplicate getMaxTextLength implementation from
gateway/handlers/summarise.go and main.go, extract the shared
environment-parsing logic into an appropriate shared package function, and
update both callers to use that function while preserving the existing default
and validation behavior.
- Around line 3-10: Run gofmt on the Go changes, particularly the imports and
formatting in summarise.go, then validate the repository with go vet ./...
before committing.
In `@gateway/main.go`:
- Around line 518-528: Update handleSummarize’s non-cache body-read error
handling so it reports the effective 32 KB BodySizeLimit rather than the
misleading 10 MB value; either remove the redundant MaxBytesReader wrapping or
use maxBytesErr.Limit when constructing the failure response.
- Around line 62-64: Fix the malformed struct tag on SummarizeRequest by
removing the space after the binding colon so the required validator is
recognized by reflect.StructTag.Get and Gin validation.
In `@gateway/routes.go`:
- Around line 51-53: Update the documentation and coverage for the new 413
behavior in the summarize route registrations in routes.go: document
body_too_large and text_too_long responses and the text field’s maximum length
in gateway/openapi.yaml, update the relevant README and web client contract if
applicable, and add tests covering oversized request bodies, overlong text, and
both route configurations with and without CacheMiddleware.
- Around line 51-53: Middleware ordering in the summarize routes allows
CacheMiddleware to read up to 10 MB before BodySizeLimit applies. In both
aiGroup.POST("/summarize") registrations, place middleware.BodySizeLimit(...)
before CacheMiddleware(), and update CacheMiddleware’s hardcoded max_size error
message to report the effective 32 KB limit when that reader fails.
---
Nitpick comments:
In `@gateway/handlers/summarise.go`:
- Around line 19-27: Replace the fragile err.Error() string comparison in the
summarise handler’s JSON binding error path with errors.As targeting
*http.MaxBytesError, declaring the typed error variable and preserving the
existing 413 response for that case. Add the required errors import and follow
the existing main.go pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2081916-0b8d-4262-b5f7-227b0de82af2
📒 Files selected for processing (5)
.env.examplegateway/handlers/summarise.gogateway/internal/ai/middleware/body_limit.gogateway/main.gogateway/routes.go
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| "os" | ||
| "strconv" | ||
|
|
||
| "github.com/gin-gonic/gin" | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Run gofmt -w . before committing.
This file uses space indentation instead of tabs, which violates the project's gofmt requirement. As per coding guidelines, use gofmt -w . before committing Go changes and validate with go vet ./....
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/handlers/summarise.go` around lines 3 - 10, Run gofmt on the Go
changes, particularly the imports and formatting in summarise.go, then validate
the repository with go vet ./... before committing.
Source: Coding guidelines
| func getMaxTextLength() int { | ||
| const defaultMaxLen = 8000 | ||
| valStr := os.Getenv("MAX_SUMMARIZE_TEXT_LENGTH") | ||
| if valStr == "" { | ||
| return defaultMaxLen | ||
| } | ||
| val, err := strconv.Atoi(valStr) | ||
| if err != nil || val <= 0 { | ||
| return defaultMaxLen | ||
| } | ||
| return val | ||
| } No newline at end of file |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
getMaxTextLength is duplicated in main.go (lines 557-570).
Both files define identical getMaxTextLength() functions with the same env-parsing logic. Extract this to a shared package to avoid divergence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/handlers/summarise.go` around lines 58 - 69, Remove the duplicate
getMaxTextLength implementation from gateway/handlers/summarise.go and main.go,
extract the shared environment-parsing logic into an appropriate shared package
function, and update both callers to use that function while preserving the
existing default and validation behavior.
| type SummarizeRequest struct { | ||
| Text string `json:"text"` | ||
| Text string `json:"text" binding: "required"` | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Struct tag binding: "required" has a space that breaks parsing.
Go's reflect.StructTag.Get requires the character immediately after the colon to be a double-quote. With binding: "required" (space after colon), the parser stops and Get("binding") returns "". The required validator is never applied. This has no current runtime impact because handleSummarize uses json.Unmarshal (not ShouldBindJSON), but the stated objective of adding Gin validation is not met, and any future switch to ShouldBindJSON would silently skip the required check.
🐛 Proposed fix
type SummarizeRequest struct {
- Text string `json:"text" binding: "required"`
+ Text string `json:"text" binding:"required"`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type SummarizeRequest struct { | |
| Text string `json:"text"` | |
| Text string `json:"text" binding: "required"` | |
| } | |
| type SummarizeRequest struct { | |
| Text string `json:"text" binding:"required"` | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/main.go` around lines 62 - 64, Fix the malformed struct tag on
SummarizeRequest by removing the space after the binding colon so the required
validator is recognized by reflect.StructTag.Get and Gin validation.
| // Validate text length | ||
| maxLen := getMaxTextLength() | ||
| if len(req.Text) > maxLen { | ||
| c.JSON(413, gin.H{ | ||
| "error": "text_too_long", | ||
| "message": fmt.Sprintf("Text must be %d characters or fewer.", maxLen), | ||
| "max_length": maxLen, | ||
| "received": len(req.Text), | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Text-length validation logic is correct, but error message in the non-cache body-read path is now misleading.
The new BodySizeLimit middleware wraps the body at 32 KB. In the non-cache branch, handleSummarize re-wraps with a 10 MB MaxBytesReader (line 471) and returns "max_size": "10MB" on failure (line 476). The effective limit is now 32 KB (the inner reader triggers first), so the error message is incorrect. Consider using maxBytesErr.Limit to report the actual limit, or removing the redundant 10 MB re-wrap when BodySizeLimit is already applied.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/main.go` around lines 518 - 528, Update handleSummarize’s non-cache
body-read error handling so it reports the effective 32 KB BodySizeLimit rather
than the misleading 10 MB value; either remove the redundant MaxBytesReader
wrapping or use maxBytesErr.Limit when constructing the failure response.
| aiGroup.POST("/summarize", CacheMiddleware(), middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), handleSummarize) | ||
| } else { | ||
| aiGroup.POST("/summarize", handleSummarize) | ||
| aiGroup.POST("/summarize", middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), handleSummarize) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Missing OpenAPI spec, README, and test updates for new 413 responses.
This PR adds HTTP 413 responses (body_too_large, text_too_long) and changes route behavior. As per coding guidelines, changes to gateway routes, headers, status codes, or response bodies require matching tests and updates to gateway/openapi.yaml, README sections, and the web client if applicable. The PR objectives also explicitly require documenting the text field's maximum length in the OpenAPI specification. No test files or openapi.yaml changes are included in the provided files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/routes.go` around lines 51 - 53, Update the documentation and
coverage for the new 413 behavior in the summarize route registrations in
routes.go: document body_too_large and text_too_long responses and the text
field’s maximum length in gateway/openapi.yaml, update the relevant README and
web client contract if applicable, and add tests covering oversized request
bodies, overlong text, and both route configurations with and without
CacheMiddleware.
Source: Coding guidelines
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Middleware ordering bypasses the 32 KB body limit when caching is enabled.
CacheMiddleware() runs before BodySizeLimit(...). CacheMiddleware reads the entire body with its own 10 MB MaxBytesReader, stores it in the Gin context (request_body), and restores c.Request.Body. BodySizeLimit then wraps the restored body at 32 KB, but handleSummarize retrieves the body from context (line 462-464 in main.go), never reading the 32 KB-wrapped body. The 32 KB limit is effectively bypassed — bodies up to 10 MB are read into memory.
Fix by reordering: place BodySizeLimit before CacheMiddleware so the 32 KB wrapper is active when CacheMiddleware reads. Note that CacheMiddleware's error message currently hardcodes "max_size": "10MB", so it will report the wrong limit when the 32 KB inner reader triggers; that message should be updated as well.
🔒️ Proposed fix for middleware ordering
if getCacheEnabled() {
- aiGroup.POST("/summarize", CacheMiddleware(), middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), handleSummarize)
+ aiGroup.POST("/summarize", middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), CacheMiddleware(), handleSummarize)
} else {
aiGroup.POST("/summarize", middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), handleSummarize)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| aiGroup.POST("/summarize", CacheMiddleware(), middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), handleSummarize) | |
| } else { | |
| aiGroup.POST("/summarize", handleSummarize) | |
| aiGroup.POST("/summarize", middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), handleSummarize) | |
| aiGroup.POST("/summarize", middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), CacheMiddleware(), handleSummarize) | |
| } else { | |
| aiGroup.POST("/summarize", middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), handleSummarize) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/routes.go` around lines 51 - 53, Middleware ordering in the summarize
routes allows CacheMiddleware to read up to 10 MB before BodySizeLimit applies.
In both aiGroup.POST("/summarize") registrations, place
middleware.BodySizeLimit(...) before CacheMiddleware(), and update
CacheMiddleware’s hardcoded max_size error message to report the effective 32 KB
limit when that reader fails.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
AnkanMisra
added
the
invalid
ChatGPT Codex Connector
|
Thanks for the contribution. I’m closing this PR because the linked issue #256 is invalid as written: the gateway already has a request-body limit, while configurability of that existing limit is tracked in #116 and #253. The current patch also duplicates summarize handling, adds a handler that is not used by the route, measures Go string bytes while describing them as characters, does not update the OpenAPI contract, lacks regression tests, and currently fails the Go lint and test checks. If you want to continue with a separate summarize-input policy, please first submit the focused enhancement suggested on #256 and wait for the scope and default limit to be agreed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aad37d95a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| import ( | ||
| "github.com/gin-gonic/gin" | ||
| "github.com/AnkanMisra/MicroAI-Paygate/gateway/internal/middleware" |
There was a problem hiding this comment.
The gateway module is declared as module gateway, and the new file was added under gateway/internal/ai/middleware, so importing github.com/AnkanMisra/MicroAI-Paygate/gateway/internal/middleware does not resolve in this module. The same external import was also added unused in main.go, and body_limit.go imports unused fmt, so the gateway will not build until the package path/location and unused imports are corrected.
Useful? React with 👍 / 👎.
| // if the body exceeds maxBytes — without reading the entire body first. | ||
| c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxBytes) | ||
|
|
||
| c.Next() |
There was a problem hiding this comment.
This middleware only wraps the body and then lets the rest of the chain run; handleSummarize does not read the body until after verifyPayment, and with CACHE_ENABLED=true the preceding CacheMiddleware reads up to 10 MB before this wrapper runs. A signed oversized request can therefore still hit Redis/verifier and consume work before returning 413, so the 32 KB protection is bypassed in those contexts; abort based on Content-Length/limited read before c.Next() or place/enforce the limit before cache and payment verification.
Useful? React with 👍 / 👎.
| // Validate text is not empty (also validated in cache middleware, but needed here for non-cached requests) | ||
| // Validate text length | ||
| maxLen := getMaxTextLength() | ||
| if len(req.Text) > maxLen { |
There was a problem hiding this comment.
The new limit is documented and reported as characters, but len(req.Text) counts UTF-8 bytes. Non-English or emoji-heavy input can be under 8,000 user-visible characters while exceeding 8,000 bytes, causing a false 413 and an inflated received value; use utf8.RuneCountInString for the comparison/response or document this as a byte limit.
Useful? React with 👍 / 👎.
| maxLen := getMaxTextLength() | ||
| if len(req.Text) > maxLen { |
There was a problem hiding this comment.
This validation runs only after verifyPayment succeeds, so a signed request whose JSON body is under 32 KB but whose text exceeds MAX_SUMMARIZE_TEXT_LENGTH still calls the verifier and burns the nonce before returning 413. That makes users sign an invalid request and prevents retrying the same payment context; parse and reject over-limit text before verifier/nonce claiming, or ensure the nonce is not consumed for validation failures.
Useful? React with 👍 / 👎.
| "error": "text_too_long", | ||
| "message": fmt.Sprintf("Text must be %d characters or fewer.", maxLen), | ||
| "max_length": maxLen, | ||
| "received": len(req.Text), |
There was a problem hiding this comment.
This introduces a new text_too_long 413 body with max_length and received, but gateway/openapi.yaml still documents the summarize 413 response as only Payload too large with max_size: 10MB. API consumers and generated clients will not match the gateway contract for over-limit text unless the OpenAPI spec is updated with the new 413 variants.
Useful? React with 👍 / 👎.
| aiGroup.Use(RequestTimeoutMiddleware(getAITimeout())) | ||
| if getCacheEnabled() { | ||
| aiGroup.POST("/summarize", CacheMiddleware(), handleSummarize) | ||
| aiGroup.POST("/summarize", CacheMiddleware(), middleware.BodySizeLimit(middleware.MaxSummarizeBodyBytes), handleSummarize) |
There was a problem hiding this comment.
When CACHE_ENABLED=true, this route runs CacheMiddleware() before either the new body-limit wrapper or the handler's MAX_SUMMARIZE_TEXT_LENGTH check. If Redis already contains a summary for text now over the configured limit, or the limit is lowered while cache entries are still live, the cache-hit path aborts before validation and returns 200 for a request the new contract should reject; validate body/text size before the cache lookup as well.
Useful? React with 👍 / 👎.
| // MaxSummarizeBodyBytes is the default body size limit for the summarize endpoint. | ||
| // 32KB is sufficient for ~8,000 words of English text — generous for summarization | ||
| // while preventing token cost explosions on AI provider APIs. | ||
| const MaxSummarizeBodyBytes = 32 * 1024 // 32 KB No newline at end of file |
There was a problem hiding this comment.
MAX_SUMMARIZE_TEXT_LENGTH is documented as operator-tunable, but the route still uses a fixed 32 KB body cap. Any deployment that raises the text limit above what fits in 32 KB, or sends 8,000 characters that require larger JSON escaping/UTF-8 encoding, will be rejected by the body limit before the configured text limit can take effect; derive the body cap from the configured text limit or reject incompatible settings at startup.
Useful? React with 👍 / 👎.
| # NEXT_PUBLIC_POSTHOG_PROJECT_TOKEN=phc_your_project_token | ||
| # NEXT_PUBLIC_POSTHOG_HOST=https://us.i.posthog.com | ||
|
|
||
| # Maximum text length (in characters) accepted by POST /api/ai/summarize. |
There was a problem hiding this comment.
This new gateway setting was added under the Frontend (Next.js / web/) — all read at build time, prefixed NEXT_PUBLIC_ section even though the gateway reads MAX_SUMMARIZE_TEXT_LENGTH at request time. Deployers following .env.example can easily apply it to the web service where it has no effect; place it with the gateway AI/request configuration instead.
Useful? React with 👍 / 👎.
|
The changes weren't merged into the main branch . Please review it once. |
|
2 participants
Summary
Fixes #256. Adds two defensive layers to prevent oversized requests from
reaching the AI provider on
POST /api/ai/summarize.Problem
A client sending 250,000 characters costs ~$1.50/request at GPT-4 class rates
vs 0.001 USDC payment → token cost explosion. Also blocks the gateway for all
concurrent users during the long AI inference.
Solution: Two-Layer Defence
http.MaxBytesReader(32KB)→ HTTP 413len(text) > 8000→ HTTP 413 with helpful messageError Responses
Configurability
Operators can tune the limit via
MAX_SUMMARIZE_TEXT_LENGTHenv var.Documented in
.env.example. No code change required for different deployments.Testing
Checklist
fix/gateway-body-size-limitgo build ./...passes.env.exampledocumentedCloses #256
Summary by CodeRabbit
MAX_SUMMARIZE_TEXT_LENGTH, defaulting to 8,000 characters.