Reject non-terminal WebSocket wildcards - #490
Conversation
444fa78 to
62e0672
Compare
|
@copilot please re-review only; do not make direct code modifications. Branch was rebased onto the latest main after #486 merged. Local validation after the rebase: |
Re-review after rebase The implementation is correct and well-aligned with the HTTP router. Here is the detailed analysis:
|
| Pattern | Expected | Actual |
|---|---|---|
/chat/* |
valid | ✓ |
/chat/*/admin |
throw | ✓ |
*/tail |
throw | ✓ |
/chat/*/ |
throw | ✓ |
Edge cases not in the tests but confirmed correct:
- Bare
*(whole pattern): accepted (wildcard at end-of-string). /*: accepted (wildcard at end-of-string)./**: accepted as a literal**segment (harmless).
test_websocket.cpp — test coverage
The three match assertions after REQUIRE_NOTHROW correctly characterise the wildcard semantics of ws_route::match():
/chat/room→ three components["", "chat", "room"]; wildcard fires atsi=2,ci=2 < 3→ match. ✓/chat/→ three components["", "chat", ""]; wildcard fires atsi=2,ci=2 < 3→ match. ✓/chat→ two components["", "chat"]; wildcard fires atsi=2,ci=2 < 2→ no match (returnsfalse). ✓
Minor observations (no action needed)
param_namesnot pushed for the WS param path — already a pre-existing characteristic; the new code doesn't touch that path.- Thread-safety of
ws_routes_— pre-existing; thewebsocket()call is a registration-time operation and not within scope of this fix. - Exception message wording — "WebSocket route wildcard must be the final path segment" is clear and consistent with the HTTP router's "HTTP route wildcard must be the final path segment".
Summary
The fix is minimal, correct, and exactly mirrors the HTTP router validation. No issues found. LGTM.
Description
Rejects WebSocket route patterns that place the
*wildcard before the final path segment. The WebSocket router documentation says its grammar matches the HTTP router, and the HTTP router already rejects non-terminal wildcards. Without this validation,ws_route::match()exits as soon as it sees*, silently ignoring later compiled segments.Type of Change
Related Issues
Closes #489
Related to #
Changes Made
Core Changes
ws_router::websocket()that the HTTP router already uses.<stdexcept>where the WebSocket router now throwsstd::invalid_argumentdirectly.API Changes (if applicable)
Invalid WebSocket route patterns such as
/chat/*/adminnow throwstd::invalid_argumentduring route registration instead of being accepted and matched with suffix truncation.Migration Guide (if breaking change)
Applications with WebSocket route patterns that put
*before later path segments should move*to the end of the route or replace it with explicit:paramsegments.Testing
Unit Tests
Integration Tests
Sanitizer Testing
Test Results
Checklist
Code Quality
Documentation
Testing
Compatibility
Performance (if applicable)
Screenshots / Diagrams
N/A.
Additional Notes
Sanitizer runs were not performed; this is a route registration validation fix covered by unit tests.
Reviewer Guidance
Areas requiring special attention:
Questions for reviewers: