Skip to content

Commit 08622d5

Browse files
committed
feat(error): preserve InvalidUri details
This branch changes the representation of the `Parse::Uri` error kind to preserve information provided by the `http` crate's `uri::InvalidUri` and `uri::InvalidUriParts` errors. A new `InvalidUri` enum is added that holds one of those two error types, or a custom message (since `hyper` currently returns `Parse::Uri` errors that didn't come from an inner `http` error in some cases). The new enum has a custom `Display` and `Debug` implementation to reduce repetition of the string "invalid URI" in its formatted output. This is _not_ stored as the error's `cause` currently in order to avoid exposing the `http` crate's error types in the public API. However, when `http` 1.0 is released, we can simplify this code significantly by storing the error as a cause and exposing it in the source chain. Closes #3043
1 parent 75aac9f commit 08622d5

File tree

2 files changed

+41
-8
lines changed

2 files changed

+41
-8
lines changed

src/error.rs

+39-6
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ pub(super) enum Parse {
6161
Version,
6262
#[cfg(feature = "http1")]
6363
VersionH2,
64-
Uri,
64+
Uri(InvalidUri),
6565
#[cfg_attr(not(all(feature = "http1", feature = "server")), allow(unused))]
6666
UriTooLong,
6767
Header(Header),
@@ -123,6 +123,12 @@ pub(super) enum User {
123123
#[derive(Debug)]
124124
pub(super) struct TimedOut;
125125

126+
pub(super) enum InvalidUri {
127+
Uri(http::uri::InvalidUri),
128+
Parts(http::uri::InvalidUriParts),
129+
Other(&'static str),
130+
}
131+
126132
impl Error {
127133
/// Returns true if this was an HTTP parse error.
128134
pub fn is_parse(&self) -> bool {
@@ -343,7 +349,7 @@ impl Error {
343349
Kind::Parse(Parse::Version) => "invalid HTTP version parsed",
344350
#[cfg(feature = "http1")]
345351
Kind::Parse(Parse::VersionH2) => "invalid HTTP version parsed (found HTTP2 preface)",
346-
Kind::Parse(Parse::Uri) => "invalid URI",
352+
Kind::Parse(Parse::Uri(_)) => "invalid URI",
347353
Kind::Parse(Parse::UriTooLong) => "URI too long",
348354
Kind::Parse(Parse::Header(Header::Token)) => "invalid HTTP header parsed",
349355
#[cfg(feature = "http1")]
@@ -420,6 +426,11 @@ impl fmt::Debug for Error {
420426

421427
impl fmt::Display for Error {
422428
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
429+
// handle "invalid URI" parse errors separately, since the inner error
430+
// is not stored as a cause.
431+
if let Kind::Parse(Parse::Uri(ref error)) = self.inner.kind {
432+
return fmt::Display::fmt(error, f);
433+
}
423434
if let Some(ref cause) = self.inner.cause {
424435
write!(f, "{}: {}", self.description(), cause)
425436
} else {
@@ -487,14 +498,14 @@ impl From<http::status::InvalidStatusCode> for Parse {
487498
}
488499

489500
impl From<http::uri::InvalidUri> for Parse {
490-
fn from(_: http::uri::InvalidUri) -> Parse {
491-
Parse::Uri
501+
fn from(inner: http::uri::InvalidUri) -> Parse {
502+
Parse::Uri(InvalidUri::Uri(inner))
492503
}
493504
}
494505

495506
impl From<http::uri::InvalidUriParts> for Parse {
496-
fn from(_: http::uri::InvalidUriParts) -> Parse {
497-
Parse::Uri
507+
fn from(inner: http::uri::InvalidUriParts) -> Parse {
508+
Parse::Uri(InvalidUri::Parts(inner))
498509
}
499510
}
500511

@@ -513,6 +524,28 @@ impl fmt::Display for TimedOut {
513524

514525
impl StdError for TimedOut {}
515526

527+
// === impl InvalidUri ===
528+
529+
impl fmt::Display for InvalidUri {
530+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
531+
match self {
532+
Self::Other(msg) => write!(f, "invalid URI: {}", msg),
533+
Self::Uri(e) => fmt::Display::fmt(e, f),
534+
Self::Parts(e) => fmt::Display::fmt(e, f),
535+
}
536+
}
537+
}
538+
539+
impl fmt::Debug for InvalidUri {
540+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
541+
match self {
542+
Self::Other(msg) => f.debug_tuple("InvalidUri").field(msg).finish(),
543+
Self::Uri(e) => fmt::Debug::fmt(e, f),
544+
Self::Parts(e) => fmt::Debug::fmt(e, f),
545+
}
546+
}
547+
}
548+
516549
#[cfg(test)]
517550
mod tests {
518551
use super::*;

src/proto/h1/role.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ impl Http1Transaction for Server {
182182
Parse::Method
183183
} else {
184184
debug_assert!(req.path.is_none());
185-
Parse::Uri
185+
Parse::Uri(error::InvalidUri::Other("invalid token"))
186186
}
187187
}
188188
other => other.into(),
@@ -445,7 +445,7 @@ impl Http1Transaction for Server {
445445
let status = match *err.kind() {
446446
Kind::Parse(Parse::Method)
447447
| Kind::Parse(Parse::Header(_))
448-
| Kind::Parse(Parse::Uri)
448+
| Kind::Parse(Parse::Uri(_))
449449
| Kind::Parse(Parse::Version) => StatusCode::BAD_REQUEST,
450450
Kind::Parse(Parse::TooLarge) => StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE,
451451
Kind::Parse(Parse::UriTooLong) => StatusCode::URI_TOO_LONG,

0 commit comments

Comments
 (0)