Skip to content

Reject non-terminal WebSocket wildcards - #490

Merged
Coldwings merged 1 commit into
mainfrom
fix/ws-route-wildcard-validation
Jul 11, 2026
Merged

Reject non-terminal WebSocket wildcards#490
Coldwings merged 1 commit into
mainfrom
fix/ws-route-wildcard-validation

Conversation

@Coldwings

Copy link
Copy Markdown
Owner

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

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Performance improvement (optimization that improves speed/memory usage)
  • Documentation (changes to documentation, comments, or examples)
  • Refactoring (code changes that neither fix bugs nor add features)
  • Tests (adding or modifying tests)
  • Build/CI (changes to build system, CI configuration, or dependencies)

Related Issues

Closes #489
Related to #

Changes Made

Core Changes

  • Added the same final-segment wildcard validation to ws_router::websocket() that the HTTP router already uses.
  • Included <stdexcept> where the WebSocket router now throws std::invalid_argument directly.
  • Added WebSocket router tests for valid trailing wildcard behavior and invalid non-terminal wildcard patterns.

API Changes (if applicable)

Invalid WebSocket route patterns such as /chat/*/admin now throw std::invalid_argument during 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 :param segments.

Testing

Unit Tests

  • Added new tests for the changes
  • Updated existing tests if needed
  • All tests pass locally

Integration Tests

  • Tested with existing examples
  • Tested in real-world scenarios (if applicable)

Sanitizer Testing

  • Tested with ASAN (AddressSanitizer)
  • Tested with TSAN (ThreadSanitizer)
  • No new warnings or errors

Test Results

HTTP_PROXY=http://192.168.31.164:7890 HTTPS_PROXY=http://192.168.31.164:7890 ALL_PROXY=http://192.168.31.164:7890 cmake -S /tmp/elio-fix-489 -B /tmp/elio-build-489 -DELIO_BUILD_TESTS=ON -DELIO_BUILD_EXAMPLES=OFF -DELIO_ENABLE_TLS=ON -DELIO_ENABLE_HTTP=ON -DELIO_ENABLE_HTTP2=OFF -DELIO_ENABLE_RDMA=OFF -DELIO_ENABLE_RDMA_CM=OFF -DELIO_ENABLE_RDMA_IBVERBS=OFF -DELIO_ENABLE_RDMA_CUDA=OFF -DELIO_WARNINGS_AS_ERRORS=OFF
cmake --build /tmp/elio-build-489 --target elio_tests --parallel 2
/tmp/elio-build-489/tests/elio_tests "[websocket][router]"
# All tests passed (7 assertions in 1 test case)
/tmp/elio-build-489/tests/elio_tests "[websocket]"
# All tests passed (312 assertions in 26 test cases)

Checklist

Code Quality

  • My code follows the project's coding standards
  • I have added/updated comments for complex logic
  • I have removed any debug code, TODOs, or commented-out code
  • My changes generate no new warnings

Documentation

  • I have updated documentation (wiki, README, code comments)
  • I have added examples for new features (if applicable)
  • I have updated API documentation (if applicable)

Testing

  • I have added tests that prove my fix is effective or my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested with ASAN and TSAN

Compatibility

  • My changes are backward compatible (or I've documented breaking changes)
  • I have considered the impact on existing users
  • I have updated CHANGELOG.md (if applicable)

Performance (if applicable)

  • I have considered the performance impact
  • I have added benchmarks for performance-critical changes

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:

  • Whether the WebSocket router should remain exactly aligned with the HTTP router wildcard grammar.
  • The exception type/message for invalid WebSocket route patterns.

Questions for reviewers:

  • None.

Copilot AI review requested due to automatic review settings July 11, 2026 05:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Coldwings
Coldwings force-pushed the fix/ws-route-wildcard-validation branch from 444fa78 to 62e0672 Compare July 11, 2026 05:10
@Coldwings

Copy link
Copy Markdown
Owner Author

@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:

cmake --build /tmp/elio-build-489 --target elio_tests --parallel 2
/tmp/elio-build-489/tests/elio_tests "[websocket][router]"
# All tests passed (7 assertions in 1 test case)
/tmp/elio-build-489/tests/elio_tests "[websocket]"
# All tests passed (318 assertions in 26 test cases)

Copilot AI commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

@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:


websocket_server.hpp — validation logic

The added guard (if (i != pattern.size())) is structurally identical to the existing check in http_server.hpp (add_route). The loop invariant is: the body executes when i == pattern.size() (end-of-string, last component) or when pattern[i] == '/' (delimiter reached). So:

  • * at end-of-string (i == pattern.size()) → condition is false → accepted as wildcard. ✓
  • * followed by / in the middle (i != pattern.size()) → condition is true → throws. ✓
  • * followed by / at trailing slash (/chat/*/) → i points at the trailing /, i != 8 is true → throws. ✓

Correctness of all tested patterns:

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 at si=2, ci=2 < 3 → match. ✓
  • /chat/ → three components ["", "chat", ""]; wildcard fires at si=2, ci=2 < 3 → match. ✓
  • /chat → two components ["", "chat"]; wildcard fires at si=2, ci=2 < 2 → no match (returns false). ✓

Minor observations (no action needed)

  1. param_names not pushed for the WS param path — already a pre-existing characteristic; the new code doesn't touch that path.
  2. Thread-safety of ws_routes_ — pre-existing; the websocket() call is a registration-time operation and not within scope of this fix.
  3. 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.

@Coldwings
Coldwings merged commit aae50cb into main Jul 11, 2026
10 checks passed
@Coldwings
Coldwings deleted the fix/ws-route-wildcard-validation branch July 11, 2026 05: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.

[Bug] WebSocket routes accept non-terminal wildcard segments

3 participants