Skip to content

test: add gin body_limit coverage (gin#247) - #154

Merged
hwbrzzl merged 5 commits into
masterfrom
test-gin-247-body-limit
Sep 26, 2026
Merged

hwbrzzl merged 5 commits into
masterfrom
test-gin-247-body-limit

Conversation

@goravel-agent

@goravel-agent goravel-agent commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Move the body-limit tests into HttpClientTestSuite and drive the bounded over/under cases — JSON, unmatched route, form, multipart, at-the-limit, and chunked — through the app's real facades.Http() client against the live server, so gin and fiber both answer a real 413 Request Entity Too Large with no driver branching.
  • Drive the body_limit: 0 fallback through the same real client to confirm normal requests stay accepted, and share assertBodyTooLarge with the bounded cases, dropping the separate in-process helper and its fasthttp/testing-http imports.
  • Run HttpClientTestSuite under fiber as well, via TestFiberDriver, so both drivers exercise the same suite.

Why

This PR adds integration coverage for the request body limit fixed in goravel/gin#247. An earlier revision drove the over-limit cases through the in-process httptest harness, where fiber surfaced fasthttp.ErrBodyTooLarge with no response and the assertions had to branch on the driver. The cases now go through the app's real HTTP client, so the contract is verified on the wire: an over-limit body is rejected with 413 Request Entity Too Large before any handler runs, while a body exactly at the limit is still accepted.

// config/http.go
"gin": map[string]any{
	"body_limit":   4096, // KiB; 0 or less falls back to 4096 KiB
	"header_limit": 4096,
	"route": func() (route.Route, error) {
		return ginfacades.Route("gin"), nil
	},
},

The body_limit: 0 case now asserts only that normal requests keep being served: asserting the 4 MiB boundary would race the server's Content-Length rejection against the client's upload, so that boundary is left unasserted rather than made flaky. The gin dependency is pinned to 467f03c, the upstream merge commit of goravel/gin#247, so the fix is consumed straight from upstream with no fork replace.

@goravel-agent
goravel-agent requested a review from a team as a code owner September 25, 2026 13:15
Comment thread tests/feature/http_client_test.go Outdated
Comment on lines +173 to +178

// One byte over the 4 MiB fallback is rejected.
over := `{"test":"` + strings.Repeat("a", defaultBodyLimitByte+1) + `"}`
resp, err = facades.Http().WithHeader("Content-Type", "application/json").
Post("/input-map", strings.NewReader(over))
s.assertBodyTooLarge(resp, err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these lines directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed those lines in 1679b10, along with the now-unused defaultBodyLimitByte const and the stale doc comment that still described the ~4 MiB boundary. TestBodyLimitDefaultFallback now only checks that body_limit: 0 keeps serving normal requests.

@hwbrzzl
hwbrzzl merged commit b8aa3d9 into master Sep 26, 2026
9 checks passed
@hwbrzzl
hwbrzzl deleted the test-gin-247-body-limit branch September 26, 2026 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants