Skip to content

Commit fc18b68

Browse files
seanmonstarseanmonstar
committed
feat(http2): check Error::source() for an HTTP2 error code to send in reset
1 parent d1501a0 commit fc18b68

File tree

4 files changed

+152
-18
lines changed

4 files changed

+152
-18
lines changed

‎src/error.rs

+63-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::io;
55

66
use httparse;
77
use http;
8+
use h2;
89

910
/// Result type often returned from methods that can have hyper `Error`s.
1011
pub type Result<T> = ::std::result::Result<T, Error>;
@@ -137,10 +138,8 @@ impl Error {
137138
self.inner.kind == Kind::Connect
138139
}
139140

140-
/// Returns the error's cause.
141-
///
142-
/// This is identical to `Error::cause` except that it provides extra
143-
/// bounds required to be able to downcast the error.
141+
#[doc(hidden)]
142+
#[cfg_attr(error_source, deprecated(note = "use Error::source instead"))]
144143
pub fn cause2(&self) -> Option<&(StdError + 'static + Sync + Send)> {
145144
self.inner.cause.as_ref().map(|e| &**e)
146145
}
@@ -163,6 +162,45 @@ impl Error {
163162
&self.inner.kind
164163
}
165164

165+
#[cfg(not(error_source))]
166+
pub(crate) fn h2_reason(&self) -> h2::Reason {
167+
// Since we don't have access to `Error::source`, we can only
168+
// look so far...
169+
let mut cause = self.cause2();
170+
while let Some(err) = cause {
171+
if let Some(h2_err) = err.downcast_ref::<h2::Error>() {
172+
return h2_err
173+
.reason()
174+
.unwrap_or(h2::Reason::INTERNAL_ERROR);
175+
}
176+
177+
cause = err
178+
.downcast_ref::<Error>()
179+
.and_then(Error::cause2);
180+
}
181+
182+
// else
183+
h2::Reason::INTERNAL_ERROR
184+
}
185+
186+
#[cfg(error_source)]
187+
pub(crate) fn h2_reason(&self) -> h2::Reason {
188+
// Find an h2::Reason somewhere in the cause stack, if it exists,
189+
// otherwise assume an INTERNAL_ERROR.
190+
let mut cause = self.source();
191+
while let Some(err) = cause {
192+
if let Some(h2_err) = err.downcast_ref::<h2::Error>() {
193+
return h2_err
194+
.reason()
195+
.unwrap_or(h2::Reason::INTERNAL_ERROR);
196+
}
197+
cause = err.source();
198+
}
199+
200+
// else
201+
h2::Reason::INTERNAL_ERROR
202+
}
203+
166204
pub(crate) fn new_canceled<E: Into<Cause>>(cause: Option<E>) -> Error {
167205
Error::new(Kind::Canceled, cause.map(Into::into))
168206
}
@@ -400,4 +438,25 @@ impl AssertSendSync for Error {}
400438

401439
#[cfg(test)]
402440
mod tests {
441+
use super::*;
442+
443+
#[test]
444+
fn h2_reason_unknown() {
445+
let closed = Error::new_closed();
446+
assert_eq!(closed.h2_reason(), h2::Reason::INTERNAL_ERROR);
447+
}
448+
449+
#[test]
450+
fn h2_reason_one_level() {
451+
let body_err = Error::new_user_body(h2::Error::from(h2::Reason::ENHANCE_YOUR_CALM));
452+
assert_eq!(body_err.h2_reason(), h2::Reason::ENHANCE_YOUR_CALM);
453+
}
454+
455+
#[test]
456+
fn h2_reason_nested() {
457+
let recvd = Error::new_h2(h2::Error::from(h2::Reason::HTTP_1_1_REQUIRED));
458+
// Suppose a user were proxying the received error
459+
let svc_err = Error::new_user_service(recvd);
460+
assert_eq!(svc_err.h2_reason(), h2::Reason::HTTP_1_1_REQUIRED);
461+
}
403462
}

‎src/proto/h2/mod.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use bytes::Buf;
22
use futures::{Async, Future, Poll};
3-
use h2::{Reason, SendStream};
3+
use h2::{SendStream};
44
use http::header::{
55
HeaderName, CONNECTION, PROXY_AUTHENTICATE, PROXY_AUTHORIZATION, TE, TRAILER,
66
TRANSFER_ENCODING, UPGRADE,
@@ -91,10 +91,10 @@ where
9191
}
9292
}
9393

94-
fn on_err(&mut self, err: S::Error) -> ::Error {
94+
fn on_user_err(&mut self, err: S::Error) -> ::Error {
9595
let err = ::Error::new_user_body(err);
96-
trace!("send body user stream error: {}", err);
97-
self.body_tx.send_reset(Reason::INTERNAL_ERROR);
96+
debug!("send body user stream error: {}", err);
97+
self.body_tx.send_reset(err.h2_reason());
9898
err
9999
}
100100

@@ -138,7 +138,7 @@ where
138138
}
139139
}
140140

141-
match try_ready!(self.stream.poll_data().map_err(|e| self.on_err(e))) {
141+
match try_ready!(self.stream.poll_data().map_err(|e| self.on_user_err(e))) {
142142
Some(chunk) => {
143143
let is_eos = self.stream.is_end_stream();
144144
trace!(
@@ -175,7 +175,7 @@ where
175175
return Err(::Error::new_body_write(::h2::Error::from(reason)));
176176
}
177177

178-
match try_ready!(self.stream.poll_trailers().map_err(|e| self.on_err(e))) {
178+
match try_ready!(self.stream.poll_trailers().map_err(|e| self.on_user_err(e))) {
179179
Some(trailers) => {
180180
self.body_tx
181181
.send_trailers(trailers)

‎src/proto/h2/server.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ where
211211
Err(e) => {
212212
let err = ::Error::new_user_service(e);
213213
warn!("http2 service errored: {}", err);
214-
self.reply.send_reset(Reason::INTERNAL_ERROR);
214+
self.reply.send_reset(err.h2_reason());
215215
return Err(err);
216216
},
217217
};
@@ -232,7 +232,7 @@ where
232232
match self.reply.send_response(res, $eos) {
233233
Ok(tx) => tx,
234234
Err(e) => {
235-
trace!("send response error: {}", e);
235+
debug!("send response error: {}", e);
236236
self.reply.send_reset(Reason::INTERNAL_ERROR);
237237
return Err(::Error::new_h2(e));
238238
}

‎tests/server.rs

+81-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#![deny(warnings)]
22
extern crate http;
33
extern crate hyper;
4+
extern crate h2;
45
#[macro_use]
56
extern crate futures;
67
extern crate futures_timer;
@@ -1562,6 +1563,73 @@ fn http1_only() {
15621563
})).unwrap_err();
15631564
}
15641565

1566+
#[test]
1567+
fn http2_service_error_sends_reset_reason() {
1568+
use std::error::Error;
1569+
let server = serve();
1570+
let addr_str = format!("http://{}", server.addr());
1571+
1572+
server
1573+
.reply()
1574+
.error(h2::Error::from(h2::Reason::INADEQUATE_SECURITY));
1575+
1576+
let mut rt = Runtime::new().expect("runtime new");
1577+
1578+
let err = rt.block_on(hyper::rt::lazy(move || {
1579+
let client = Client::builder()
1580+
.http2_only(true)
1581+
.build_http::<hyper::Body>();
1582+
let uri = addr_str.parse().expect("server addr should parse");
1583+
1584+
client.get(uri)
1585+
})).unwrap_err();
1586+
1587+
let h2_err = err
1588+
.source()
1589+
.unwrap()
1590+
.downcast_ref::<h2::Error>()
1591+
.unwrap();
1592+
1593+
assert_eq!(h2_err.reason(), Some(h2::Reason::INADEQUATE_SECURITY));
1594+
}
1595+
1596+
#[test]
1597+
fn http2_body_user_error_sends_reset_reason() {
1598+
use std::error::Error;
1599+
let server = serve();
1600+
let addr_str = format!("http://{}", server.addr());
1601+
1602+
let b = ::futures::stream::once::<String, h2::Error>(Err(
1603+
h2::Error::from(h2::Reason::INADEQUATE_SECURITY)
1604+
));
1605+
let b = hyper::Body::wrap_stream(b);
1606+
1607+
server
1608+
.reply()
1609+
.body_stream(b);
1610+
1611+
let mut rt = Runtime::new().expect("runtime new");
1612+
1613+
let err = rt.block_on(hyper::rt::lazy(move || {
1614+
let client = Client::builder()
1615+
.http2_only(true)
1616+
.build_http::<hyper::Body>();
1617+
let uri = addr_str.parse().expect("server addr should parse");
1618+
1619+
client
1620+
.get(uri)
1621+
.and_then(|res| res.into_body().concat2())
1622+
})).unwrap_err();
1623+
1624+
let h2_err = err
1625+
.source()
1626+
.unwrap()
1627+
.downcast_ref::<h2::Error>()
1628+
.unwrap();
1629+
1630+
assert_eq!(h2_err.reason(), Some(h2::Reason::INADEQUATE_SECURITY));
1631+
}
1632+
15651633
// -------------------------------------------------
15661634
// the Server that is used to run all the tests with
15671635
// -------------------------------------------------
@@ -1609,6 +1677,8 @@ impl Serve {
16091677
}
16101678
}
16111679

1680+
type BoxError = Box<::std::error::Error + Send + Sync>;
1681+
16121682
struct ReplyBuilder<'a> {
16131683
tx: &'a spmc::Sender<Reply>,
16141684
}
@@ -1635,10 +1705,13 @@ impl<'a> ReplyBuilder<'a> {
16351705
self.tx.send(Reply::Body(body.as_ref().to_vec().into())).unwrap();
16361706
}
16371707

1638-
fn body_stream(self, body: Body)
1639-
{
1708+
fn body_stream(self, body: Body) {
16401709
self.tx.send(Reply::Body(body)).unwrap();
16411710
}
1711+
1712+
fn error<E: Into<BoxError>>(self, err: E) {
1713+
self.tx.send(Reply::Error(err.into())).unwrap();
1714+
}
16421715
}
16431716

16441717
impl<'a> Drop for ReplyBuilder<'a> {
@@ -1666,6 +1739,7 @@ enum Reply {
16661739
Version(hyper::Version),
16671740
Header(HeaderName, HeaderValue),
16681741
Body(hyper::Body),
1742+
Error(BoxError),
16691743
End,
16701744
}
16711745

@@ -1677,7 +1751,7 @@ enum Msg {
16771751
}
16781752

16791753
impl TestService {
1680-
fn call(&self, req: Request<Body>) -> Box<Future<Item=Response<Body>, Error=hyper::Error> + Send> {
1754+
fn call(&self, req: Request<Body>) -> Box<Future<Item=Response<Body>, Error=BoxError> + Send> {
16811755
let tx1 = self.tx.clone();
16821756
let tx2 = self.tx.clone();
16831757
let replies = self.reply.clone();
@@ -1691,12 +1765,12 @@ impl TestService {
16911765
};
16921766
tx2.send(msg).unwrap();
16931767
Ok(())
1694-
}).map(move |_| {
1768+
}).and_then(move |_| {
16951769
TestService::build_reply(replies)
16961770
}))
16971771
}
16981772

1699-
fn build_reply(replies: spmc::Receiver<Reply>) -> Response<Body> {
1773+
fn build_reply(replies: spmc::Receiver<Reply>) -> Result<Response<Body>, BoxError> {
17001774
let mut res = Response::new(Body::empty());
17011775
while let Ok(reply) = replies.try_recv() {
17021776
match reply {
@@ -1712,10 +1786,11 @@ impl TestService {
17121786
Reply::Body(body) => {
17131787
*res.body_mut() = body;
17141788
},
1789+
Reply::Error(err) => return Err(err),
17151790
Reply::End => break,
17161791
}
17171792
}
1718-
res
1793+
Ok(res)
17191794
}
17201795
}
17211796

0 commit comments

Comments
 (0)