Skip to content

Commit

Permalink
Merge pull request mozilla#1409 from dlrobertson/issue-1364
Browse files Browse the repository at this point in the history 
Add headers to session_start and session_end
  • Loading branch information
@KershawChang
KershawChang authored Feb 6, 2023
2 parents 07c2019 + bc8375c commit c9a19ac
Show file tree
Hide file tree
Showing 15 changed files with 141 additions and 36 deletions.
5 changes: 3 additions & 2 deletions neqo-common/src/timer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<T> Timer<T> {
for i in &self.items {
debug_assert!(i.is_empty());
}
self.now = time - short_span;
self.now = time.checked_sub(short_span).unwrap();
self.cursor = 0;
}

Expand Down Expand Up @@ -287,7 +287,8 @@ mod test {
t.add(near_future, v);
assert_eq!(near_future, t.next_time().expect("should return a value"));
assert_eq!(
t.take_until(near_future - Duration::from_millis(1)).count(),
t.take_until(near_future.checked_sub(Duration::from_millis(1)).unwrap())
.count(),
0
);
assert!(t
Expand Down
18 changes: 16 additions & 2 deletions neqo-http3/src/client_events.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ pub enum WebTransportEvent {
Session {
stream_id: StreamId,
status: u16,
headers: Vec<Header>,
},
SessionClosed {
stream_id: StreamId,
reason: SessionCloseReason,
headers: Option<Vec<Header>>,
},
NewStream {
stream_id: StreamId,
Expand Down Expand Up @@ -181,11 +183,18 @@ impl SendStreamEvents for Http3ClientEvents {
}

impl ExtendedConnectEvents for Http3ClientEvents {
fn session_start(&self, connect_type: ExtendedConnectType, stream_id: StreamId, status: u16) {
fn session_start(
&self,
connect_type: ExtendedConnectType,
stream_id: StreamId,
status: u16,
headers: Vec<Header>,
) {
if connect_type == ExtendedConnectType::WebTransport {
self.insert(Http3ClientEvent::WebTransport(WebTransportEvent::Session {
stream_id,
status,
headers,
}));
} else {
unreachable!("There is only ExtendedConnectType::WebTransport.");
Expand All @@ -197,10 +206,15 @@ impl ExtendedConnectEvents for Http3ClientEvents {
connect_type: ExtendedConnectType,
stream_id: StreamId,
reason: SessionCloseReason,
headers: Option<Vec<Header>>,
) {
if connect_type == ExtendedConnectType::WebTransport {
self.insert(Http3ClientEvent::WebTransport(
WebTransportEvent::SessionClosed { stream_id, reason },
WebTransportEvent::SessionClosed {
stream_id,
reason,
headers,
},
));
} else {
unreachable!("There are no other types.");
Expand Down
3 changes: 2 additions & 1 deletion neqo-http3/src/connection_client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ fn alpn_from_quic_version(version: Version) -> &'static str {
/// match event {
/// Http3ClientEvent::WebTransport(WebTransportEvent::Session{
/// stream_id,
/// status
/// status,
/// ..
/// }) => {
/// println!("The response from the server: WebTransport session ID {:?} status={:?}",
/// stream_id,
Expand Down
10 changes: 9 additions & 1 deletion neqo-http3/src/features/extended_connect/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::client_events::Http3ClientEvents;
use crate::features::NegotiationState;
use crate::settings::{HSettingType, HSettings};
use crate::{CloseType, Http3StreamInfo, Http3StreamType};
use neqo_common::Header;
use neqo_transport::{AppError, StreamId};
use std::fmt::Debug;
pub(crate) use webtransport_session::WebTransportSession;
Expand All @@ -39,12 +40,19 @@ impl From<CloseType> for SessionCloseReason {
}

pub(crate) trait ExtendedConnectEvents: Debug {
fn session_start(&self, connect_type: ExtendedConnectType, stream_id: StreamId, status: u16);
fn session_start(
&self,
connect_type: ExtendedConnectType,
stream_id: StreamId,
status: u16,
headers: Vec<Header>,
);
fn session_end(
&self,
connect_type: ExtendedConnectType,
stream_id: StreamId,
reason: SessionCloseReason,
headers: Option<Vec<Header>>,
);
fn extended_connect_new_stream(&self, stream_info: Http3StreamInfo);
fn new_datagram(&self, session_id: StreamId, datagram: Vec<u8>);
Expand Down
30 changes: 24 additions & 6 deletions neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mod streams;
use neqo_common::event::Provider;

use crate::{
features::extended_connect::SessionCloseReason, Error, Http3Client, Http3ClientEvent,
features::extended_connect::SessionCloseReason, Error, Header, Http3Client, Http3ClientEvent,
Http3OrWebTransportStream, Http3Parameters, Http3Server, Http3ServerEvent, Http3State,
WebTransportEvent, WebTransportRequest, WebTransportServerEvent,
WebTransportSessionAcceptAction,
Expand Down Expand Up @@ -177,8 +177,13 @@ impl WtTest {
e,
Http3ClientEvent::WebTransport(WebTransportEvent::Session{
stream_id,
status
}) if stream_id == wt_session_id && status == 200
status,
headers,
}) if (
stream_id == wt_session_id &&
status == 200 &&
headers.contains(&Header::new(":status", "200"))
)
)
};
assert!(self.client.events().any(wt_session_negotiated_event));
Expand Down Expand Up @@ -210,13 +215,15 @@ impl WtTest {
e: &Http3ClientEvent,
id: StreamId,
expected_reason: &SessionCloseReason,
expected_headers: &Option<Vec<Header>>,
) -> bool {
if let Http3ClientEvent::WebTransport(WebTransportEvent::SessionClosed {
stream_id,
reason,
headers,
}) = e
{
*stream_id == id && reason == expected_reason
*stream_id == id && reason == expected_reason && headers == expected_headers
} else {
false
}
Expand All @@ -226,11 +233,17 @@ impl WtTest {
&mut self,
wt_session_id: StreamId,
expected_reason: &SessionCloseReason,
expected_headers: &Option<Vec<Header>>,
) {
let mut event_found = false;

while let Some(event) = self.client.next_event() {
event_found = WtTest::session_closed_client(&event, wt_session_id, expected_reason);
event_found = WtTest::session_closed_client(
&event,
wt_session_id,
expected_reason,
expected_headers,
);
if event_found {
break;
}
Expand All @@ -251,9 +264,10 @@ impl WtTest {
if let Http3ServerEvent::WebTransport(WebTransportServerEvent::SessionClosed {
session,
reason,
headers,
}) = e
{
session.stream_id() == id && reason == expected_reason
session.stream_id() == id && reason == expected_reason && headers.is_none()
} else {
false
}
Expand Down Expand Up @@ -401,10 +415,12 @@ impl WtTest {
Http3ClientEvent::WebTransport(WebTransportEvent::SessionClosed {
stream_id,
reason,
headers,
}) => {
close_event = true;
assert_eq!(stream_id, expected_session_close.as_ref().unwrap().0);
assert_eq!(expected_session_close.as_ref().unwrap().1, reason);
assert!(headers.is_none());
}
_ => {}
}
Expand Down Expand Up @@ -536,13 +552,15 @@ impl WtTest {
Http3ServerEvent::WebTransport(WebTransportServerEvent::SessionClosed {
session,
reason,
headers,
}) => {
close_event = true;
assert_eq!(
session.stream_id(),
expected_session_close.as_ref().unwrap().0
);
assert_eq!(expected_session_close.as_ref().unwrap().1, reason);
assert!(headers.is_none());
}
_ => {}
}
Expand Down
46 changes: 35 additions & 11 deletions neqo-http3/src/features/extended_connect/tests/webtransport/sessions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ fn wt_session() {
#[test]
fn wt_session_reject() {
let mut wt = WtTest::new();
let accept_res =
WebTransportSessionAcceptAction::Reject([Header::new(":status", "404")].to_vec());
let headers = vec![Header::new(":status", "404")];
let accept_res = WebTransportSessionAcceptAction::Reject(headers.clone());
let (wt_session_id, _wt_session) = wt.negotiate_wt_session(&accept_res);

wt.check_session_closed_event_client(wt_session_id, &SessionCloseReason::Status(404));
wt.check_session_closed_event_client(
wt_session_id,
&SessionCloseReason::Status(404),
&Some(headers),
);
}

#[test]
Expand All @@ -54,6 +58,7 @@ fn wt_session_close_server() {
wt.check_session_closed_event_client(
wt_session.stream_id(),
&SessionCloseReason::Error(Error::HttpNoError.code()),
&None,
);
}

Expand All @@ -70,6 +75,7 @@ fn wt_session_close_server_close_send() {
error: 0,
message: String::new(),
},
&None,
);
}

Expand All @@ -85,6 +91,7 @@ fn wt_session_close_server_stop_sending() {
wt.check_session_closed_event_client(
wt_session.stream_id(),
&SessionCloseReason::Error(Error::HttpNoError.code()),
&None,
);
}

Expand All @@ -100,6 +107,7 @@ fn wt_session_close_server_reset() {
wt.check_session_closed_event_client(
wt_session.stream_id(),
&SessionCloseReason::Error(Error::HttpNoError.code()),
&None,
);
}

Expand Down Expand Up @@ -149,8 +157,13 @@ fn wt_session_response_with_1xx() {
e,
Http3ClientEvent::WebTransport(WebTransportEvent::Session{
stream_id,
status
}) if stream_id == wt_session_id && status == 200
status,
headers,
}) if (
stream_id == wt_session_id &&
status == 200 &&
headers.contains(&Header::new(":status", "200"))
)
)
};
assert!(wt.client.events().any(wt_session_negotiated_event));
Expand All @@ -160,15 +173,18 @@ fn wt_session_response_with_1xx() {

#[test]
fn wt_session_response_with_redirect() {
let headers = [Header::new(":status", "302"), Header::new("location", "/")].to_vec();
let mut wt = WtTest::new();

let accept_res = WebTransportSessionAcceptAction::Reject(
[Header::new(":status", "302"), Header::new("location", "/")].to_vec(),
);
let accept_res = WebTransportSessionAcceptAction::Reject(headers.clone());

let (wt_session_id, _wt_session) = wt.negotiate_wt_session(&accept_res);

wt.check_session_closed_event_client(wt_session_id, &SessionCloseReason::Status(302));
wt.check_session_closed_event_client(
wt_session_id,
&SessionCloseReason::Status(302),
&Some(headers),
);
}

#[test]
Expand Down Expand Up @@ -212,8 +228,14 @@ fn wt_session_respone_200_with_fin() {
e,
Http3ClientEvent::WebTransport(WebTransportEvent::SessionClosed{
stream_id,
reason
}) if stream_id == wt_session_id && reason == SessionCloseReason::Clean{ error: 0, message: String::new()}
reason,
headers,
..
}) if (
stream_id == wt_session_id &&
reason == SessionCloseReason::Clean{ error: 0, message: String::new()} &&
headers.is_none()
)
)
};
assert!(wt.client.events().any(wt_session_close_event));
Expand Down Expand Up @@ -256,6 +278,7 @@ fn wt_session_close_frame_server() {
error: ERROR_NUM,
message: ERROR_MESSAGE.to_string(),
},
&None,
);
}

Expand Down Expand Up @@ -331,6 +354,7 @@ fn wt_close_session_frame_broken_client() {
wt.check_session_closed_event_client(
wt_session.stream_id(),
&SessionCloseReason::Error(Error::HttpGeneralProtocolStream.code()),
&None,
);
wt.check_session_closed_event_server(
&mut wt_session,
Expand Down
7 changes: 7 additions & 0 deletions neqo-http3/src/features/extended_connect/webtransport_session.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ impl WebTransportSession {
ExtendedConnectType::WebTransport,
self.session_id,
SessionCloseReason::from(close_type),
None,
);
}
}
Expand Down Expand Up @@ -242,6 +243,7 @@ impl WebTransportSession {
error: 0,
message: String::new(),
},
Some(headers),
);
self.state = SessionState::Done;
}
Expand All @@ -266,13 +268,15 @@ impl WebTransportSession {
error: 0,
message: String::new(),
},
Some(headers),
);
SessionState::Done
} else {
self.events.session_start(
ExtendedConnectType::WebTransport,
self.session_id,
status,
headers,
);
SessionState::Active
}
Expand All @@ -281,6 +285,7 @@ impl WebTransportSession {
ExtendedConnectType::WebTransport,
self.session_id,
SessionCloseReason::Status(status),
Some(headers),
);
SessionState::Done
};
Expand Down Expand Up @@ -345,6 +350,7 @@ impl WebTransportSession {
ExtendedConnectType::WebTransport,
self.session_id,
SessionCloseReason::Clean { error, message },
None,
);
self.state = if fin {
SessionState::Done
Expand All @@ -359,6 +365,7 @@ impl WebTransportSession {
error: 0,
message: String::new(),
},
None,
);
self.state = SessionState::Done;
}
Expand Down
Loading

0 comments on commit c9a19ac

Please sign in to comment.