Skip to content
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

Add protection against missing trailing slashes when using websocket. #391

Merged
merged 1 commit into from
Oct 15, 2020
Merged

Conversation

nzdjb
Copy link

@nzdjb nzdjb commented Jul 23, 2020

Currently, if -webSocket is set and -url does not have a trailing slash, connection will fail with an UnresolvedAddressException. This can prove confusing to diagnose and resolve.

This PR adds a check for a trailing slash and adds it if missing.

Copy link
Contributor

@jeffret-b jeffret-b left a comment

Choose a reason for hiding this comment

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

This seems reasonable.

@jeffret-b jeffret-b requested a review from jglick July 23, 2020 22:10
@jeffret-b
Copy link
Contributor

Pinging @jglick since he did the WebSocket work to see if he has any concerns.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Seems safe.

@jeffret-b jeffret-b added bug For changelog: Fixes a bug. ready-to-merge labels Jul 24, 2020
@jeffret-b
Copy link
Contributor

I'll release this when I get a chance but it will be a while.

@nzdjb
Copy link
Author

nzdjb commented Jul 25, 2020

No worries, it's not blocking us. Thanks for including it.

@jglick
Copy link
Member

jglick commented Oct 12, 2020

ping @jeffret-b

@jeffret-b
Copy link
Contributor

I'm going to spend a bit of time on maintenance here this week, going through PRs, reviewing, merging where I can, and getting out a release soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Fixes a bug. ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants