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.
feat(server/sse): Add support for dynamic base paths #214
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: main
Are you sure you want to change the base?
feat(server/sse): Add support for dynamic base paths #214
Changes from all commits
ebb4e67
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
💡 Verification agent
🧩 Analysis chain
Verify that path concatenation doesn't create double slashes
While the sanitization in
WithDynamicBasePath
should prevent issues with dynamic paths, the direct concatenation withbasePath + s.messageEndpoint
could still result in malformed paths ifs.messageEndpoint
starts with a slash.🏁 Script executed:
Length of output: 901
The previous grep didn’t surface any matches—let’s locate where
messageEndpoint
is declared and how it’s initialized/sanitized:🏁 Script executed:
Length of output: 1121
Let’s verify how
basePath
is initialized and whetherWithBasePath
orWithDynamicBasePath
normalize slashes:Let’s verify how
basePath
is initialized and whether theWithBasePath
/WithDynamicBasePath
options normalize trailing or leading slashes:Avoid double slashes when building the message endpoint
Right now,
GetMessageEndpointForClient
does:If
basePath
ends with a slash ors.messageEndpoint
begins with one, you’ll end up with//
in the URL. NeitherWithBasePath
norWithMessageEndpoint
strip extra slashes, so this can easily slip through.Locations to update:
GetMessageEndpointForClient
(around line 360)WithBasePath
setter (line 137)WithMessageEndpoint
setter (line 141)Suggested fix:
path.Join
when concatenating:"path"
)This will ensure you never accidentally generate
//
in your URLs.