Skip to content

Commit

Permalink
Split out a Settings struct from Config (#208)
Browse files Browse the repository at this point in the history
* Split out a `Settings` struct from `Config`

The `Settings` struct is an abstract representation of
`frame::Settings`.

Basically:

- When talking about our own connection, we should use `Config`.
  - e.g. in `ConnectionInner`'s `config` field

- When talking about a peer's settings, we should use `Settings.
  - e.g. in `SharedState`'s `peer_config` field

This separation allows us to define some conversions:

- `Config` -> `frame::Settings`
  - used in ConnectionInner::new

- `&frame::Settings` -> `Settings`
  - used in ConnectionInner::poll_control

* Remove Deref and DerefMut impls for Config

`Config` is not a smart pointer
  • Loading branch information
dongcarl authored Jul 21, 2023
1 parent 3991dca commit a57ed22
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 85 deletions.
6 changes: 3 additions & 3 deletions h3-webtransport/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ where
//
// However, it is still advantageous to show a log on the server as (attempting) to
// establish a WebTransportSession without the proper h3 config is usually a mistake.
if !conn.inner.config.enable_webtransport() {
if !conn.inner.config.settings.enable_webtransport() {
tracing::warn!("Server does not support webtransport");
}

if !conn.inner.config.enable_datagram() {
if !conn.inner.config.settings.enable_datagram() {
tracing::warn!("Server does not support datagrams");
}

if !conn.inner.config.enable_extended_connect() {
if !conn.inner.config.settings.enable_extended_connect() {
tracing::warn!("Server does not support CONNECT");
}

Expand Down
4 changes: 2 additions & 2 deletions h3/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ impl Builder {
///
/// [header size constraints]: https://www.rfc-editor.org/rfc/rfc9114.html#name-header-size-constraints
pub fn max_field_section_size(&mut self, value: u64) -> &mut Self {
self.config.max_field_section_size = value;
self.config.settings.max_field_section_size = value;
self
}

Expand Down Expand Up @@ -544,7 +544,7 @@ impl Builder {
open,
conn_state,
conn_waker,
max_field_section_size: self.config.max_field_section_size,
max_field_section_size: self.config.settings.max_field_section_size,
sender_count: Arc::new(AtomicUsize::new(1)),
send_grease_frame: self.config.send_grease,
_buf: PhantomData,
Expand Down
119 changes: 112 additions & 7 deletions h3/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::proto::varint::VarInt;
use std::convert::TryFrom;

use crate::proto::{frame, varint::VarInt};

/// Configures the HTTP/3 connection
#[derive(Debug, Clone, Copy)]
Expand All @@ -13,6 +15,13 @@ pub struct Config {
#[cfg(test)]
pub(crate) send_settings: bool,

/// HTTP/3 Settings
pub settings: Settings,
}

/// HTTP/3 Settings
#[derive(Debug, Clone, Copy)]
pub struct Settings {
/// The MAX_FIELD_SECTION_SIZE in HTTP/3 refers to the maximum size of the dynamic table used in HPACK compression.
/// HPACK is the compression algorithm used in HTTP/3 to reduce the size of the header fields in HTTP requests and responses.

Expand All @@ -34,7 +43,107 @@ pub struct Config {
pub(crate) max_webtransport_sessions: u64,
}

impl Config {
impl From<&frame::Settings> for Settings {
fn from(settings: &frame::Settings) -> Self {
let defaults: Self = Default::default();
Self {
max_field_section_size: settings
.get(frame::SettingId::MAX_HEADER_LIST_SIZE)
.unwrap_or(defaults.max_field_section_size),
enable_webtransport: settings
.get(frame::SettingId::ENABLE_WEBTRANSPORT)
.map(|value| value != 0)
.unwrap_or(defaults.enable_webtransport),
max_webtransport_sessions: settings
.get(frame::SettingId::WEBTRANSPORT_MAX_SESSIONS)
.unwrap_or(defaults.max_webtransport_sessions),
enable_datagram: settings
.get(frame::SettingId::H3_DATAGRAM)
.map(|value| value != 0)
.unwrap_or(defaults.enable_datagram),
enable_extended_connect: settings
.get(frame::SettingId::ENABLE_CONNECT_PROTOCOL)
.map(|value| value != 0)
.unwrap_or(defaults.enable_extended_connect),
}
}
}

impl TryFrom<Config> for frame::Settings {
type Error = frame::SettingsError;
fn try_from(value: Config) -> Result<Self, Self::Error> {
let mut settings = frame::Settings::default();

let Config {
send_grease,
#[cfg(test)]
send_settings: _,
settings:
Settings {
max_field_section_size,
enable_webtransport,
enable_extended_connect,
enable_datagram,
max_webtransport_sessions,
},
} = value;

if send_grease {
// Grease Settings (https://www.rfc-editor.org/rfc/rfc9114.html#name-defined-settings-parameters)
//= https://www.rfc-editor.org/rfc/rfc9114#section-7.2.4.1
//# Setting identifiers of the format 0x1f * N + 0x21 for non-negative
//# integer values of N are reserved to exercise the requirement that
//# unknown identifiers be ignored. Such settings have no defined
//# meaning. Endpoints SHOULD include at least one such setting in their
//# SETTINGS frame.

//= https://www.rfc-editor.org/rfc/rfc9114#section-7.2.4.1
//# Setting identifiers that were defined in [HTTP/2] where there is no
//# corresponding HTTP/3 setting have also been reserved
//# (Section 11.2.2). These reserved settings MUST NOT be sent, and
//# their receipt MUST be treated as a connection error of type
//# H3_SETTINGS_ERROR.
match settings.insert(frame::SettingId::grease(), 0) {
Ok(_) => (),
Err(err) => tracing::warn!("Error when adding the grease Setting. Reason {}", err),
}
}

settings.insert(
frame::SettingId::MAX_HEADER_LIST_SIZE,
max_field_section_size,
)?;
settings.insert(
frame::SettingId::ENABLE_CONNECT_PROTOCOL,
enable_extended_connect as u64,
)?;
settings.insert(
frame::SettingId::ENABLE_WEBTRANSPORT,
enable_webtransport as u64,
)?;
settings.insert(frame::SettingId::H3_DATAGRAM, enable_datagram as u64)?;
settings.insert(
frame::SettingId::WEBTRANSPORT_MAX_SESSIONS,
max_webtransport_sessions,
)?;

Ok(settings)
}
}

impl Default for Settings {
fn default() -> Self {
Self {
max_field_section_size: VarInt::MAX.0,
enable_webtransport: false,
enable_extended_connect: false,
enable_datagram: false,
max_webtransport_sessions: 0,
}
}
}

impl Settings {
/// https://datatracker.ietf.org/doc/html/draft-ietf-webtrans-http3/#section-3.1
/// Sets `SETTINGS_ENABLE_WEBTRANSPORT` if enabled
pub fn enable_webtransport(&self) -> bool {
Expand All @@ -58,14 +167,10 @@ impl Config {
impl Default for Config {
fn default() -> Self {
Self {
max_field_section_size: VarInt::MAX.0,
send_grease: true,
#[cfg(test)]
send_settings: true,
enable_webtransport: false,
enable_extended_connect: false,
enable_datagram: false,
max_webtransport_sessions: 0,
settings: Default::default(),
}
}
}
76 changes: 9 additions & 67 deletions h3/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ use stream::WriteBuf;
use tracing::{trace, warn};

use crate::{
config::Config,
config::{Config, Settings},
error::{Code, Error},
frame::FrameStream,
proto::{
frame::{Frame, PayloadLen, SettingId, Settings},
frame::{self, Frame, PayloadLen},
headers::Header,
stream::StreamType,
varint::VarInt,
Expand All @@ -30,7 +30,7 @@ use crate::{
#[non_exhaustive]
pub struct SharedState {
// Peer settings
pub peer_config: Config,
pub peer_config: Settings,
// connection-wide error, concerns all RequestStreams and drivers
pub error: Option<Error>,
// Has a GOAWAY frame been sent or received?
Expand All @@ -54,7 +54,7 @@ impl SharedStateRef {
impl Default for SharedStateRef {
fn default() -> Self {
Self(Arc::new(RwLock::new(SharedState {
peer_config: Config::default(),
peer_config: Default::default(),
error: None,
closing: false,
})))
Expand Down Expand Up @@ -148,54 +148,11 @@ where
return Ok(());
}

let mut settings = Settings::default();

settings
.insert(
SettingId::MAX_HEADER_LIST_SIZE,
self.config.max_field_section_size,
)
.map_err(|e| Code::H3_INTERNAL_ERROR.with_cause(e))?;

settings
.insert(
SettingId::ENABLE_CONNECT_PROTOCOL,
self.config.enable_extended_connect as u64,
)
.map_err(|e| Code::H3_INTERNAL_ERROR.with_cause(e))?;
settings
.insert(
SettingId::ENABLE_WEBTRANSPORT,
self.config.enable_webtransport as u64,
)
.map_err(|e| Code::H3_INTERNAL_ERROR.with_cause(e))?;
settings
.insert(SettingId::H3_DATAGRAM, self.config.enable_datagram as u64)
let settings = frame::Settings::try_from(self.config)
.map_err(|e| Code::H3_INTERNAL_ERROR.with_cause(e))?;

tracing::debug!("Sending server settings: {:#x?}", settings);

if self.config.send_grease {
// Grease Settings (https://www.rfc-editor.org/rfc/rfc9114.html#name-defined-settings-parameters)
//= https://www.rfc-editor.org/rfc/rfc9114#section-7.2.4.1
//# Setting identifiers of the format 0x1f * N + 0x21 for non-negative
//# integer values of N are reserved to exercise the requirement that
//# unknown identifiers be ignored. Such settings have no defined
//# meaning. Endpoints SHOULD include at least one such setting in their
//# SETTINGS frame.

//= https://www.rfc-editor.org/rfc/rfc9114#section-7.2.4.1
//# Setting identifiers that were defined in [HTTP/2] where there is no
//# corresponding HTTP/3 setting have also been reserved
//# (Section 11.2.2). These reserved settings MUST NOT be sent, and
//# their receipt MUST be treated as a connection error of type
//# H3_SETTINGS_ERROR.
match settings.insert(SettingId::grease(), 0) {
Ok(_) => (),
Err(err) => warn!("Error when adding the grease Setting. Reason {}", err),
}
}

//= https://www.rfc-editor.org/rfc/rfc9114#section-3.2
//# After the QUIC connection is
//# established, a SETTINGS frame MUST be sent by each endpoint as the
Expand Down Expand Up @@ -395,7 +352,9 @@ where
);
}
}
AcceptedRecvStream::WebTransportUni(id, s) if self.config.enable_webtransport => {
AcceptedRecvStream::WebTransportUni(id, s)
if self.config.settings.enable_webtransport =>
{
// Store until someone else picks it up, like a webtransport session which is
// not yet established.
self.accepted_streams.wt_uni_streams.push((id, s))
Expand Down Expand Up @@ -465,24 +424,7 @@ where
//# Endpoints MUST NOT consider such settings to have
//# any meaning upon receipt.
let mut shared = self.shared.write("connection settings write");
shared.peer_config.max_field_section_size = settings
.get(SettingId::MAX_HEADER_LIST_SIZE)
.unwrap_or(VarInt::MAX.0);

shared.peer_config.enable_webtransport =
settings.get(SettingId::ENABLE_WEBTRANSPORT).unwrap_or(0) != 0;

shared.peer_config.max_webtransport_sessions = settings
.get(SettingId::WEBTRANSPORT_MAX_SESSIONS)
.unwrap_or(0);

shared.peer_config.enable_datagram =
settings.get(SettingId::H3_DATAGRAM).unwrap_or(0) != 0;

shared.peer_config.enable_extended_connect = settings
.get(SettingId::ENABLE_CONNECT_PROTOCOL)
.unwrap_or(0)
!= 0;
shared.peer_config = (&settings).into();

Ok(Frame::Settings(settings))
}
Expand Down
12 changes: 6 additions & 6 deletions h3/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ impl Builder {
///
/// [header size constraints]: https://www.rfc-editor.org/rfc/rfc9114.html#name-header-size-constraints
pub fn max_field_section_size(&mut self, value: u64) -> &mut Self {
self.config.max_field_section_size = value;
self.config.settings.max_field_section_size = value;
self
}

Expand All @@ -590,27 +590,27 @@ impl Builder {
/// and `max_webtransport_sessions`.
#[inline]
pub fn enable_webtransport(&mut self, value: bool) -> &mut Self {
self.config.enable_webtransport = value;
self.config.settings.enable_webtransport = value;
self
}

/// Enables the CONNECT protocol
pub fn enable_connect(&mut self, value: bool) -> &mut Self {
self.config.enable_extended_connect = value;
self.config.settings.enable_extended_connect = value;
self
}

/// Limits the maximum number of WebTransport sessions
pub fn max_webtransport_sessions(&mut self, value: u64) -> &mut Self {
self.config.max_webtransport_sessions = value;
self.config.settings.max_webtransport_sessions = value;
self
}

/// Indicates that the client or server supports HTTP/3 datagrams
///
/// See: <https://www.rfc-editor.org/rfc/rfc9297#section-2.1.1>
pub fn enable_datagram(&mut self, value: bool) -> &mut Self {
self.config.enable_datagram = value;
self.config.settings.enable_datagram = value;
self
}
}
Expand All @@ -627,7 +627,7 @@ impl Builder {
let (sender, receiver) = mpsc::unbounded_channel();
Ok(Connection {
inner: ConnectionInner::new(conn, SharedStateRef::default(), self.config).await?,
max_field_section_size: self.config.max_field_section_size,
max_field_section_size: self.config.settings.max_field_section_size,
request_end_send: sender,
request_end_recv: receiver,
ongoing_streams: HashSet::new(),
Expand Down

0 comments on commit a57ed22

Please sign in to comment.