-
Notifications
You must be signed in to change notification settings - Fork 13.3k
std::net::url - adding encoding/decoding of components and forms (from erickt/rust-uri) and more compliant parsing #3069
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
Conversation
…t/rust-uri into std::net::url
…ate error messages, better split fn
Conflicts: src/libstd/net_url.rs
… encounters invalid stuff; support for ipv6, more tests.
… debugging import Conflicts: src/libstd/net_url.rs
…_chari instead of str_reader to make code cleaner
Conflicts: src/libstd/net_url.rs
Thanks! I'm very interested in this and will review tomorrow. FYI, I made url sendable so that servo can use it across tasks. |
This looks great. Merged to incoming. Right now I don't have a need to make the arguments to encode_form_url sendable. I did some significant rebasing to clean up the history. I realize that you had a difficult merge with several changes hitting this module at the same time, but I would prefer to see a cleaner history because they are easier to review and understand (and just for the sake of cleanliness). Generally I try to rebase a branch off of incoming before pushing or, if it doesn't rebase cleanly, then just a single merge at the end of the series. With frequent merges between branches it's difficult, when reviewing, to separate the work on the branch from the work on trunk. Thanks! |
Thanks. I'm pretty new to git; I'll try to do a better job next time... |
Consider a multi-lined array as a block-like expression
Automatic sync from rustc
This includes both a merge of @erickt's url / form encoding/decoding code into net::url and a bunch of work to make the parsing a lot more robust. It (obviously) builds off of the rough draft that I wrote a couple weeks ago. In particular, it now actually follows the RFC in terms of what characters are allowed where, as far as I am aware. It also produces error messages when invalid urls are encountered (they could probably use improvement).
In terms of reading the diff, the first couple hundred lines are @erickt's encoding/decoding code, with the only changes being some naming and type updates (mainly using @~strs instead of @Strs). The core of the new code is in the get_* functions - which do the parsing of various components; only get_authority has significant logic in it (which is primarily caused by the use of ':' in the spec - it can separate a user from password, parts of an ipv6 host, or a host from a port). And of course
from_str
was rewritten to use this new code; it serves as a guide of the order things happen in (and the names follow the convention in the RFC).Finally, there are a bunch of new tests - all the ones from @erickt's repo for encoding/decoding, and some to test individual component parsing and some more full url tests.
@brson made some changes to the type of query a couple days ago to make it sendable - presumably those changes should also be made to the encoding/decoding functions for forms, but I'm not sure how it would be best to handle it (straight vectors/tuples, or something else?), and I wanted to have erickt's code mostly intact for this pull request (to try to keep the new code limited).
(I have run the local tests and the lint check from make check - I'm running the full make check now, but it takes forever on my slow machine and since this code is not used anywhere as far as I know, I doubt there would be anything elsewhere that could cause a failure; if there is, I will obviously update this).