http icon indicating copy to clipboard operation
http copied to clipboard

Uri's Debug implementation should show components

Open jsha opened this issue 5 years ago • 5 comments

Right now Uri's implementation of the Debug trait calls out to its Display implementation. This makes it hard to use the Debug trait to check what has been parsed into the various fields of the Uri, because those fields get concatenated together by the Display trait.

I think a #[derive(Debug)] would do the job nicely. Is there some history I'm missing about why the Debug method calls out to Display?

jsha avatar May 24 '20 06:05 jsha

It's not clear to me that the extra information is desirable for the default case. In many cases, a user understands a URI as basically a string. For instance, if a user prints a Request, I suspect they'd just want to see uri: "https://hyper.rs/guides", not it split into pieces.

Perhaps we could do so in the "alternate" format, which is triggered via {:#?}.

seanmonstar avatar May 24 '20 17:05 seanmonstar

Ah, this is a good point - I was thinking of this mainly in the context of printing the debug format of a URL directly, like println!("{}", url).

For the case of printing a Request, where the user may not want to see the details of the URL, would it make sense to specialize Request's Debug trait to stringify URLs before printing?

jsha avatar May 24 '20 18:05 jsha

I think this is just determine the default. Does a user more often just assume a Uri is a string-y thing, or do they assume it has a bunch of fields? Some other examples of string-y types that do what Uri does now are Url and std::path::Path.

seanmonstar avatar May 27 '20 00:05 seanmonstar

Users of Uri can't #[derive(Debug)] on it themselves if they want the split-out view, but they can use format("{}", uri) if they want the serialized view, so offering a split-out view as the Debug trait makes both options available, depending on what the user wants.

If someone's specifically logging the Debug view of a Uri, it's probably because something unexpected is happening with the parsing. Since the semantics that Uri add relative to a plain string are specifically around the parsing, I think the most informative thing is to show the parsed components. As a concrete example, when I was investigating https://github.com/seanmonstar/reqwest/issues/919, one of the first things I tried was to print out the Debug of a Uri to see if the scheme was getting set to "http".

Here's another concrete example: parsing the URL javascript:alert(1). url::Url parses this with a scheme of "javascript" and a path of "alert(1)". http::Uri parses this with a scheme of None, and authority of javascript:alert(1), and a path_and_query of /. But if you print both of these parsed objects with {:?}, they look exactly the same.

Note that the commit in rust-url that switched from a #[derive(Debug)] doesn't specifically mention wanting to print the serialized view vs the split-out view: https://github.com/servo/rust-url/commit/9e759f18. That commit was actually part of a big "rewrite all the things refactor" to use indexes into the string. That rewrite also would have made the derived debug output much less readable, which could explain why the switch to a manually-implemented Debug function. I'll file an issue over on that repo and ask what people think there too.

jsha avatar May 27 '20 16:05 jsha

One way would be to deal with it similar with how Path[Buf]'s Display formatting works: as a separate method. So you could have a .display_parts() on it that specifically is for printing out the individual parts.

CryZe avatar May 27 '20 16:05 CryZe