Skip to content

Conversation

@Greisby
Copy link
Contributor

@Greisby Greisby commented Jan 28, 2026

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:

  • HttpRequest::isCorsRequest() — convenience detection of CORS requests.
  • HttpRequest::isCorsPreflightRequest() — convenience detection of CORS preflight requests.
  • HttpResponse::newOptionsResponse(...) — a helper that generates either:
    • a generic OPTIONS response with 204 No Content and Allow, or
    • a CORS preflight response with 204 No Content and the appropriate Access-Control-Allow-* headers, with optional policy validation.
  • HttpResponse::addCorsHeaders(...) — a helper that adds CORS headers to answers to non-preflight CORS requests.
  • A few lightweight std::string_view utilities in drogon::utils used by the implementation (ci_equals, trim, split/join helpers).
  • Unit tests covering the helpers behavior.

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:

  • consistent (same headers, same status, same cache semantics),
  • browser friendly (preflight expectations),
  • optionally strict (reject disallowed method/headers/origin with explicit 40x/405).

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-Error diagnostic 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:

  • it is lighter-weight than std::string, and avoids unnecessary allocations/copies,
  • it fits well with HTTP header parsing, which is typically “view over a buffer” logic,
  • it keeps the helpers usable in hot paths without accidental heap churn.

API / Behavior

HttpRequest::isCorsRequest()
Returns true when the request contains an Origin header (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:

  • Origin
  • Access-Control-Request-Method

(Used as a cheap, explicit gate when implementing custom handlers.)

HttpResponse::newOptionsResponse(...)

  • Returns nullptr if the request is not OPTIONS.
  • Always responds with 204 No Content and:
    • Cache-Control: no-store
    • Vary: Origin (to protect against misbehaving caches/proxies)
  • Generic OPTIONS
    • Adds the header Allow: ... (method list comes from the request attributes obtained from Drogon’s route handler.
  • CORS preflight OPTIONS
    • Reflects Origin back in Access-Control-Allow-Origin (with optional originValidator and allowNullOrigin policy).
    • Sets the header Access-Control-Allow-Methods using the allowed method list (and validates Access-Control-Request-Method, returning 405 + Allow if not permitted).
    • Handles Access-Control-Request-Headers as follows:
      • If allowedHeaders is provided: validates requested headers against it (case-insensitive compare), returns 403 on mismatch and reports the allowed set.
      • If allowedHeaders is not provided: behaves permissively and echoes back the requested headers.
    • Optional knobs:
      • allowCredentials → adds the header Access-Control-Allow-Credentials: true to the response
      • allowPNA → if request has Access-Control-Request-Private-Network: true, adds the header Access-Control-Allow-Private-Network: true to the response
      • maxAgeSeconds → adds the header Access-Control-Max-Age: <n> on success

On 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 manages Access-Control-Allow-Credentials, and merges exposed headers into Access-Control-Expose-Headers.


Unit Tests

Added unit tests to cover:

  • Non-OPTIONS returns null.
  • Generic OPTIONS response has expected headers.
  • Preflight happy path: allow-origin/methods/headers/max-age.
  • Method mismatch → 405 + Allowed methods (Allow:).
  • Header restriction mismatch → 403 (+ allowed headers).
  • Origin empty / null-origin policy handling.
  • checking the headers added by addCorsHeaders()

@Greisby
Copy link
Contributor Author

Greisby commented Jan 28, 2026

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.
I added them to keep the helpers lightweight and allocation-free where possible, but I can rework this if it’s not aligned with the project's direction.
More generally, I tend to prefer views when doing simple string manipulation (trimming, splitting, joining), as it avoids incidental allocations and keeps the code “view-over-buffer” friendly.

@Greisby Greisby requested a review from Copilot January 28, 2026 14:18
@Greisby Greisby self-assigned this Jan 28, 2026
Copy link

Copilot AI left a 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() and isCorsPreflightRequest() 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.

@Greisby Greisby requested a review from an-tao January 28, 2026 14:28
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.

1 participant