-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Enhancement/options cutom handling #2447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Enhancement/options cutom handling #2447
Conversation
|
This is an initial draft; please let me know if you’d prefer different policy choices or if you’d rather avoid adding the extra std::string_view utilities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive API for building consistent OPTIONS/CORS preflight responses in application code, allowing developers to handle CORS manually when opting out of Drogon's internal handling.
Changes:
- Adds convenience methods
isCorsRequest()andisCorsPreflightRequest()to HttpRequest for detecting CORS requests - Introduces
newOptionsResponse()helper to generate generic OPTIONS or CORS preflight responses with validation - Adds
addCorsHeaders()helper for adding CORS headers to non-preflight CORS responses - Implements lightweight string_view utilities (ci_equals, trim, split/join) for efficient header parsing
- Includes comprehensive unit tests covering various CORS scenarios and edge cases
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| trantor | Updates trantor submodule reference |
| lib/tests/unittests/HttpHeaderTest.cc | Adds unit tests for OPTIONS response generation, CORS preflight validation, and CORS header management |
| lib/src/HttpResponseImpl.cc | Implements newOptionsResponse() and addCorsHeaders() methods with CORS validation logic |
| lib/inc/drogon/utils/Utilities.h | Adds string_view utility functions for case-insensitive comparison, trimming, and splitting/joining |
| lib/inc/drogon/HttpResponse.h | Declares newOptionsResponse() and addCorsHeaders() public API methods with detailed documentation |
| lib/inc/drogon/HttpRequest.h | Adds isCorsRequest() and isCorsPreflightRequest() convenience methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This is a follow up to the PR #2444: now that Drogon can opt out of its internal OPTIONS/CORS short-circuit and let applications handle preflight in route handlers, this PR provides small, self-contained helpers API to build consistent OPTIONS / CORS preflight responses inside application code.
Summary
Concretely, it introduces:
Allow, orAccess-Control-Allow-*headers, with optional policy validation.No default behavior changes: Drogon’s existing internal OPTIONS handling remains untouched unless the application opts out (as introduced in #2444).
Motivation
When an application opts out of the internal preflight shortcut (per #2444), it typically still needs to generate responses that are:
This PR provides that utility without altering the HttpServer default CORS behavior or policy surface.
Policy choice: explicit errors vs silent success
For preflight validation (origin / requested method / requested headers), this implementation returns explicit errors (400/403/405) when the CORS preflight request is malformed or not allowed.
This is a deliberate policy choice:
The Fetch Standard (CORS protocol) does not mandate any particular error status for a denied preflight.
In practice, browsers decide on their side whether the preflight is acceptable based on the presence/values of Access-Control-Allow-* headers. A 204 without the expected allow headers is effectively a “deny” from the browser’s point of view as well.
I chose explicit 403/405 responses, with an
X-Cors-Errordiagnostic header, because it is more logical and much easier to debug for web developers (especially when testing from tools like curl, Postman, or browser devtools).If maintainers would prefer a “silent deny” approach instead (always 204 and omit/limit allow headers), I can adapt the helper accordingly — both approaches are valid, as browsers enforce the decision client-side.
Why the added string_view utilities
The helper needs a few small string operations (case-insensitive compare, trimming, splitting/joining header lists). I implemented them using std::string_view because:
API / Behavior
HttpRequest::isCorsRequest()
Returns true when the request contains an
Originheader (i.e. a CORS request), regardless of the HTTP method.HttpRequest::isCorsPreflightRequest()
Returns true when the request is an OPTIONS request and contains the preflight-defining headers:
OriginAccess-Control-Request-Method(Used as a cheap, explicit gate when implementing custom handlers.)
HttpResponse::newOptionsResponse(...)
Allow:... (method list comes from the request attributes obtained from Drogon’s route handler.Originback inAccess-Control-Allow-Origin(with optional originValidator and allowNullOrigin policy).Access-Control-Allow-Methodsusing the allowed method list (and validatesAccess-Control-Request-Method, returning 405 +Allowif not permitted).Access-Control-Request-Headersas follows:Access-Control-Allow-Credentials: trueto the responseAccess-Control-Request-Private-Network: true, adds the headerAccess-Control-Allow-Private-Network: trueto the responseAccess-Control-Max-Age: <n>on successOn validation failures, the helper returns explicit HTTP status codes and adds a diagnostic X-Cors-Error header to ease debugging.
HttpResponse::addCorsHeaders(…)
Helper for non-preflight CORS requests: reflects the request Origin (unless Access-Control-Allow-Origin is already set), ensures
Vary: Origin, optionally managesAccess-Control-Allow-Credentials, and merges exposed headers intoAccess-Control-Expose-Headers.Unit Tests
Added unit tests to cover:
Allow:).