Skip to content

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

Closed
wants to merge 16 commits into from

Conversation

dbp
Copy link
Contributor

@dbp dbp commented Aug 1, 2012

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).

@brson
Copy link
Contributor

brson commented Aug 2, 2012

Thanks! I'm very interested in this and will review tomorrow. FYI, I made url sendable so that servo can use it across tasks.

@brson
Copy link
Contributor

brson commented Aug 3, 2012

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!

@brson brson closed this Aug 3, 2012
@dbp
Copy link
Contributor Author

dbp commented Aug 3, 2012

Thanks. I'm pretty new to git; I'll try to do a better job next time...

bors pushed a commit to rust-lang-ci/rust that referenced this pull request May 15, 2021
Consider a multi-lined array as a block-like expression
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants