fix(ws client): use query part of URL. #429
Conversation
Fixes #428, we didn't take that query part of the URL into account and it wasn't sent in GET request.
ws-client/src/transport.rs
Outdated
| if let Some(query) = url.query() { | ||
| path.push('?'); | ||
| path.push_str(query); | ||
| } |
There was a problem hiding this comment.
This looks sane to me (without a wider context of how the path String that now contains the query part is used elsewhere)! I wonder whether it should be renamed path_and_query for clarity? Is there anywhere that will be surprised to now be getting a query string in here?
I was wondering whether percent-encoded URLs would end up being double percent-encoded when returned (because Url::query percent-encodes) but I did this:
use url::Url;
fn main() {
let foo = Url::parse("ws://localhost/foo?bar=hello%20world").unwrap();
println!("{:?}", foo.query());
}
in the rust playground and the output is "bar=hello%20world" as I'd hope for :)
There was a problem hiding this comment.
It's the same as soketto calls http resource but I like path_and_query better even if the query might be empty :P
There was a problem hiding this comment.
Further path_and_query is passed directly to soketto::Client::new(...) called resource there, it's used here
* fix(ws client): use query part of URL. Fixes #428, we didn't take that query part of the URL into account and it wasn't sent in GET request. * add log when connecting to a target * fix(grumbles): path -> path_and_query
Fixes #428, we didn't take that query part of the URL into account and it wasn't sent in GET request because of that.