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

Use query string from WebSocket URI-based instantiation #5095

Merged
merged 1 commit into from Oct 13, 2017

Conversation

jackturnbull
Copy link
Contributor

When instantiating a WebSocket using separate parameters for host and path, it is possible to pass in a query string to the connection by appending it to the end of the path parameter. This is effectively stripped out when is passed through as a URI since the path call on the URL does not return the query string.

Fixed by calling full_path instead of path on the given URI.

See: #3234 for further and working example

@jackturnbull jackturnbull changed the title Use query string from URI-based instantiation Use query string from WebSocket URI-based instantiation Oct 10, 2017
@bew
Copy link
Contributor

bew commented Oct 10, 2017

Thanks!
Could you add a spec for this change?

@jackturnbull
Copy link
Contributor Author

Yeah not a problem! I'm tethered to my phone at the minute so will get one together when I can run it locally

@RX14
Copy link
Contributor

RX14 commented Oct 10, 2017

I just pushed a commit which adds specs to this PR for you.

When instantiating a WebSocket using separate parameters for host and path, it is possible to pass in a query string to the connection by appending it to the end of the path parameter. This is effectively stripped out when is passed through as a URI since the `path` call on the URL does not return the query string.

Fixed by calling `full_path` instead of `path` on the given URI.
@jackturnbull
Copy link
Contributor Author

Nice one, thanks! Was just about to look at it but saved me a job :)

@RX14 RX14 added this to the Next milestone Oct 13, 2017
@RX14 RX14 merged commit f0ec21e into crystal-lang:master Oct 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants