Skip to content

URI parse errors should preserve information from the HTTP crate #3043

Open
@hawkw

Description

@hawkw

Is your feature request related to a problem? Please describe.

When an invalid URI is parsed by the http crate, it returns an InvalidUri error. When hyper encounters one of these errors, it converts it into a hyper::Error type with an error kind that indicates that an invalid URI was encountered. The http::uri::InvalidUri type contains additional information describing what part of the URI was invalid, which is included in the error's fmt::Display output: https://github.com/hyperium/http/blob/c1f309e0c3695f8e14e1b54c738681783d4c7b9e/src/uri/mod.rs#L1063-L1079

Unfrortunately, when hyper converts these errors into its hyper::Error type, the additional information is discarded:

hyper/src/error.rs

Lines 489 to 499 in 75aac9f

impl From<http::uri::InvalidUri> for Parse {
fn from(_: http::uri::InvalidUri) -> Parse {
Parse::Uri
}
}
impl From<http::uri::InvalidUriParts> for Parse {
fn from(_: http::uri::InvalidUriParts) -> Parse {
Parse::Uri
}
}

The error that's returned to the application will format as just "invalid URI", which is not particularly descriptive. Being able to surface more information about why a URI was invalid would be useful, particularly in applications where URIs are input by the user.

Describe the solution you'd like

It would be nice if hyper preserved the description of what part of the URI was invalid. I think we should probably change the Parse::Uri variant in hyper::error to contain an http::uri::InvalidUri, so that we can format the additional error information in the fmt::Display impl for hyper::Error.

Since we don't want to expose types from other crates in hyper's public API, for stability reasons, we probably shouldn't allow the user to access the http::uri::InvalidUri error in the hyper::Error source chain, or allow downcasting the error to it.

Describe alternatives you've considered

  • We could include the http::uri::InvalidUri as a source of hyper::Error. This might be conceptually the "right thing", but it would expose a type from an external crate in hyper's public API, which is undesirable as http is not yet 1.0.
  • We could leave things the way they are now. Unfortunately, that would mean that hyper would completely swallow information describing what part of the URI was invalid, and this would never be exposed to the user.

Metadata

Metadata

Assignees

Labels

C-featureCategory: feature. This is adding a new feature.E-easyEffort: easy. A task that would be a great starting point for a new contributor.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions