Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates the two previous forwarding routes into a single catch-all handler, centralizes path-splitting logic, and refines the client’s CLI prompts and status output.
- Replace separate
/ :client_idand/ :client_id/*pathroutes with one/*pathroute inmain.rs - Merge
forward_handler_no_pathandforward_handler_with_pathinto a singleforward_handlerwith path parsing - Enhance client UX by extracting a status printer, updating allowed-paths prompts, and cleaning up imports/formatting
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/src/main.rs | Swapped two specific routes for a single wildcard route |
| server/src/forwarding.rs | Combined two handlers into forward_handler, added path extraction and validation |
| client/src/websocket_handler.rs | Consolidated import formatting |
| client/src/main.rs | Added print_tunnel_status, moved HTTP client setup earlier |
| client/src/http_handler.rs | Tidied up header insertion formatting and base64 encoding call |
| client/src/config.rs | Overhauled allowed-paths input flow and improved error messages |
Comments suppressed due to low confidence (2)
server/src/forwarding.rs:122
- The new segment-splitting logic in
forward_handlershould have unit tests covering cases like empty path, missingclient_id, and multi-segment paths to verify correct behavior.
let mut segments = path.splitn(2, '/');
server/src/forwarding.rs:134
- The call to
handle_forwarding_requestreturns a future but isn’t awaited, which will cause a compilation error. Append.awaitto invoke the handler and return its response.
handle_forwarding_request(
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.
This PR solves #15 where forwarding to root with trailing slash would not work.
These changes remove some duplicate code, and move the logic of the path splitting into the forward path method instead.
It also refines the way users add paths a bit for better usability.