Feature/custom routes middleware and max_size fix#19
Merged
isala404 merged 3 commits intoisala404:mainfrom Apr 18, 2026
Merged
Conversation
Owner
|
@GoatedChopin can you rebase this on top of current main and fix up the commit tree? |
Moves the custom_routes merge point from the top-level router (outside /_api, no middleware) into GatewayServer::router() so custom routes receive the full middleware stack: auth, CORS, tracing, concurrency limits, and timeouts. Adds custom_routes_with(|pool| ...) so handlers can access Forge's managed PgPool, which isn't available at builder time. Updates the custom-handlers documentation to reflect the new behavior and recommended usage patterns.
The hardcoded DEFAULT_MAX_FILE_SIZE (10 MiB) silently clamped individual files below the configured per-mutation max_size or global max_body_size. A mutation with max_size = "200mb" would still reject any single file over 10 MiB. Use max_total as the per-file limit so the configured limits are respected end-to-end.
4a3d026 to
3726b09
Compare
Contributor
Author
I rebased to remove ~8 commits. Hopefully this looks better |
Dropped the Router-taking overload of custom_routes; the pool factory is the only form now, consistent with the pre-1.0 "no old way / new way" policy. Route paths resolve under /_api because they share the gateway router with built-ins, and the docs now say so explicitly and list the full conflict set. Upload limits split cleanly: gateway.max_body_size caps the total request body, gateway.max_file_size caps any single file when a mutation has no max_size declared (default 10mb, same safety bound the hardcoded constant used to provide). A mutation's max_size stays the explicit opt-in for large single files and overrides both. Config validation rejects max_file_size > max_body_size. Also removed the .unwrap() on auth.user_id() in the example (workspace denies clippy::unwrap_used), added unit tests for the limit resolution and config validation, and updated the skill references per the documentation policy.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preface
I sometimes need routes that aren't fully supported by the forge response shapes. I talked with Supiri about an original change I made called "authenticated_routes" that used a similar builder pattern. They helpfully suggested that we move the place that
custom_routesmerge into the main_router, which is a much better solution.The second change is motivated by my need to support large uploads in certain endpoints but not others. If this needs to be a separate PR we can split it up.
Core
Summary
Two changes in this PR:
Custom routes middleware — Moves the
custom_routesmerge point into the gateway's middleware stack so custom routes automatically receive auth, CORS, tracing, concurrency limits, and timeouts. Also addscustom_routes_with(|pool| ...)for pool access.Fix per-file upload cap ignoring
max_size— Removes a hardcoded 10 MiB per-file limit that silently clamped individual file uploads below the configuredmax_sizeormax_body_size.1. Apply middleware stack to custom routes
Changes
crates/forge-runtime/src/gateway/server.rs— Addcustom_routesfield toGatewayServer,with_custom_routes()setter, and merge intomain_routerbefore theServiceBuildermiddleware layers are applied.crates/forge/src/runtime.rs— Addcustom_routes_factoryfield andcustom_routes_with()builder method. Resolve the factory after the pool is connected inrun(), pass merged routes togateway.with_custom_routes(). Remove the old root-levelrouter.merge(custom)that bypassed middleware.Motivation
Custom routes merged at the top level don't get any of the gateway middleware (auth, CORS, tracing, etc.), which makes them unusable for real API endpoints that need authentication. Rather than adding a second concept (
authenticated_routes), this moves the existing merge point so the middleware applies automatically.The
_with(|pool| ...)variant is necessary because the database pool isn't available when the builder is constructed — it's created duringrun().This keeps a single
custom_routesconcept instead of requiring a separateauthenticated_routesAPI — one merge point, one set of docs, one mental model.2. Fix per-file upload cap ignoring max_size configuration
Changes
crates/forge-runtime/src/gateway/multipart.rs— RemoveDEFAULT_MAX_FILE_SIZE(hardcoded 10 MiB) and setmax_file = max_totalso the per-file limit equals the resolved total limit.Motivation
The multipart handler had a hardcoded
DEFAULT_MAX_FILE_SIZEof 10 MiB that was applied viamax_file = max_total.min(DEFAULT_MAX_FILE_SIZE). This meant that even when a mutation declaredmax_size = "200mb"orforge.tomlsetmax_body_size = "100mb", no single file could exceed 10 MiB. The total payload limit was respected, but the per-file limit silently clamped to 10 MiB.Since a single file can legitimately be the entire payload, the per-file limit should equal the total limit. The total limit (
max_total) already provides the safety bound.Test plan
cargo checkpasses across the workspacecargo test --workspace— all existing tests passcustom_routes_with, confirm it receives auth headers and rejects unauthenticated requestsmax_size = "200mb", confirm it succeeds