Skip to content

Commit

Permalink
refactor: enable mozilla-central http3server to use neqo-bin (#1878)
Browse files Browse the repository at this point in the history
* refactor(bin): introduce server/http3.rs and server/http09.rs

The QUIC Interop Runner requires an http3 and http09 implementation for both
client and server. The client code is already structured into an http3 and an
http09 implementation since #1727.

This commit does the same for the server side, i.e. splits the http3 and http09
implementation into separate Rust modules.

* refactor: merge mozilla-central http3 server into neqo-bin

There are two server implementations based on neqo:

1. https://github.com/mozilla/neqo/tree/main/neqo-bin/src/server
  - http3 and http09 implementation
  - used for manual testing and QUIC Interop

2. https://searchfox.org/mozilla-central/source/netwerk/test/http3server/src/main.rs
  - used to test Firefox

I assume one was once an exact copy of the other. Both implement their own I/O,
event loop, ... Since then, the two implementations diverged significantly.
Especially (1) saw a lot of improvements in recent months:

- #1564
- #1569
- #1578
- #1581
- #1604
- #1612
- #1676
- #1692
- #1707
- #1708
- #1727
- #1753
- #1756
- #1766
- #1772
- #1786
- #1787
- #1788
- #1794
- #1806
- #1808
- #1848
- #1866

At this point, bugs in (2) are hard to fix, see e.g.
#1801.

This commit merges (2) into (1), thus removing all duplicate logic and
having (2) benefit from all the recent improvements to (1).

* Move firefox.rs to mozilla-central

* Reduce HttpServer trait functions

* Extract constructor

* Remove unused deps

* Remove clap color feature

Nice to have. Adds multiple dependencies. Hard to justify for mozilla-central.
  • Loading branch information
mxinden authored May 8, 2024
1 parent 35c157b commit b8ae981
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 145 deletions.
2 changes: 1 addition & 1 deletion neqo-bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ workspace = true

[dependencies]
# neqo-bin is not used in Firefox, so we can be liberal with dependency versions
clap = { version = "4.4", default-features = false, features = ["std", "color", "help", "usage", "error-context", "suggestions", "derive"] }
clap = { version = "4.4", default-features = false, features = ["std", "help", "usage", "error-context", "suggestions", "derive"] }
clap-verbosity-flag = { version = "2.2", default-features = false }
futures = { version = "0.3", default-features = false, features = ["alloc"] }
hex = { version = "0.4", default-features = false, features = ["std"] }
Expand Down
2 changes: 1 addition & 1 deletion neqo-bin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use neqo_transport::{

pub mod client;
pub mod server;
mod udp;
pub mod udp;

#[derive(Debug, Parser)]
pub struct SharedArgs {
Expand Down
84 changes: 36 additions & 48 deletions neqo-bin/src/server/http09.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::{
cell::RefCell, collections::HashMap, fmt::Display, path::PathBuf, rc::Rc, time::Instant,
};
use std::{cell::RefCell, collections::HashMap, fmt::Display, rc::Rc, time::Instant};

use neqo_common::{event::Provider, hex, qdebug, qerror, qinfo, qwarn, Datagram};
use neqo_crypto::{generate_ech_keys, random, AllowZeroRtt, AntiReplay, Cipher};
use neqo_crypto::{generate_ech_keys, random, AllowZeroRtt, AntiReplay};
use neqo_http3::Error;
use neqo_transport::{
server::{ActiveConnectionRef, Server, ValidateAddress},
ConnectionEvent, ConnectionIdGenerator, ConnectionParameters, Output, State, StreamId,
ConnectionEvent, ConnectionIdGenerator, Output, State, StreamId,
};
use regex::Regex;

Expand All @@ -29,29 +27,44 @@ pub struct HttpServer {
server: Server,
write_state: HashMap<StreamId, HttpStreamState>,
read_state: HashMap<StreamId, Vec<u8>>,
is_qns_test: bool,
}

impl HttpServer {
pub fn new(
now: Instant,
certs: &[impl AsRef<str>],
protocols: &[impl AsRef<str>],
args: &Args,
anti_replay: AntiReplay,
cid_manager: Rc<RefCell<dyn ConnectionIdGenerator>>,
conn_params: ConnectionParameters,
) -> Result<Self, Error> {
let mut server = Server::new(
args.now(),
&[args.key.clone()],
&[args.shared.alpn.clone()],
anti_replay,
Box::new(AllowZeroRtt {}),
cid_manager,
args.shared.quic_parameters.get(&args.shared.alpn),
)?;

server.set_ciphers(&args.get_ciphers());
server.set_qlog_dir(args.shared.qlog_dir.clone());
if args.retry {
server.set_validation(ValidateAddress::Always);
}
if args.ech {
let (sk, pk) = generate_ech_keys().expect("generate ECH keys");
server
.enable_ech(random::<1>()[0], "public.example", &sk, &pk)
.expect("enable ECH");
let cfg = server.ech_config();
qinfo!("ECHConfigList: {}", hex(cfg));
}

Ok(Self {
server: Server::new(
now,
certs,
protocols,
anti_replay,
Box::new(AllowZeroRtt {}),
cid_manager,
conn_params,
)?,
server,
write_state: HashMap::new(),
read_state: HashMap::new(),
is_qns_test: args.shared.qns_test.is_some(),
})
}

Expand Down Expand Up @@ -100,12 +113,7 @@ impl HttpServer {
}
}

fn stream_readable(
&mut self,
stream_id: StreamId,
conn: &mut ActiveConnectionRef,
args: &Args,
) {
fn stream_readable(&mut self, stream_id: StreamId, conn: &mut ActiveConnectionRef) {
if !stream_id.is_client_initiated() || !stream_id.is_bidi() {
qdebug!("Stream {} not client-initiated bidi, ignoring", stream_id);
return;
Expand Down Expand Up @@ -136,7 +144,7 @@ impl HttpServer {
return;
};

let re = if args.shared.qns_test.is_some() {
let re = if self.is_qns_test {
Regex::new(r"GET +/(\S+)(?:\r)?\n").unwrap()
} else {
Regex::new(r"GET +/(\d+)(?:\r)?\n").unwrap()
Expand All @@ -150,7 +158,7 @@ impl HttpServer {
let resp = {
let path = path.as_str();
qdebug!("Path = '{path}'");
if args.shared.qns_test.is_some() {
if self.is_qns_test {
match qns_read_response(path) {
Ok(data) => Some(data),
Err(e) => {
Expand Down Expand Up @@ -199,7 +207,7 @@ impl super::HttpServer for HttpServer {
self.server.process(dgram, now)
}

fn process_events(&mut self, args: &Args, now: Instant) {
fn process_events(&mut self, now: Instant) {
let active_conns = self.server.active_connections();
for mut acr in active_conns {
loop {
Expand All @@ -213,7 +221,7 @@ impl super::HttpServer for HttpServer {
.insert(stream_id, HttpStreamState::default());
}
ConnectionEvent::RecvStreamReadable { stream_id } => {
self.stream_readable(stream_id, &mut acr, args);
self.stream_readable(stream_id, &mut acr);
}
ConnectionEvent::SendStreamWritable { stream_id } => {
self.stream_writable(stream_id, &mut acr);
Expand All @@ -233,26 +241,6 @@ impl super::HttpServer for HttpServer {
}
}

fn set_qlog_dir(&mut self, dir: Option<PathBuf>) {
self.server.set_qlog_dir(dir);
}

fn validate_address(&mut self, v: ValidateAddress) {
self.server.set_validation(v);
}

fn set_ciphers(&mut self, ciphers: &[Cipher]) {
self.server.set_ciphers(ciphers);
}

fn enable_ech(&mut self) -> &[u8] {
let (sk, pk) = generate_ech_keys().expect("generate ECH keys");
self.server
.enable_ech(random::<1>()[0], "public.example", &sk, &pk)
.expect("enable ECH");
self.server.ech_config()
}

fn has_events(&self) -> bool {
self.server.has_active_connections()
}
Expand Down
47 changes: 21 additions & 26 deletions neqo-bin/src/server/http3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ use std::{
cmp::min,
collections::HashMap,
fmt::{self, Display},
path::PathBuf,
rc::Rc,
time::Instant,
};

use neqo_common::{qdebug, qerror, qwarn, Datagram, Header};
use neqo_crypto::{generate_ech_keys, random, AntiReplay, Cipher};
use neqo_common::{hex, qdebug, qerror, qinfo, qwarn, Datagram, Header};
use neqo_crypto::{generate_ech_keys, random, AntiReplay};
use neqo_http3::{
Http3OrWebTransportStream, Http3Parameters, Http3Server, Http3ServerEvent, StreamId,
};
Expand All @@ -29,6 +28,7 @@ pub struct HttpServer {
/// Progress writing to each stream.
remaining_data: HashMap<StreamId, ResponseData>,
posts: HashMap<Http3OrWebTransportStream, usize>,
is_qns_test: bool,
}

impl HttpServer {
Expand All @@ -39,7 +39,7 @@ impl HttpServer {
anti_replay: AntiReplay,
cid_mgr: Rc<RefCell<dyn ConnectionIdGenerator>>,
) -> Self {
let server = Http3Server::new(
let mut server = Http3Server::new(
args.now(),
&[args.key.clone()],
&[args.shared.alpn.clone()],
Expand All @@ -53,10 +53,25 @@ impl HttpServer {
None,
)
.expect("We cannot make a server!");

server.set_ciphers(&args.get_ciphers());
server.set_qlog_dir(args.shared.qlog_dir.clone());
if args.retry {
server.set_validation(ValidateAddress::Always);
}
if args.ech {
let (sk, pk) = generate_ech_keys().expect("should create ECH keys");
server
.enable_ech(random::<1>()[0], "public.example", &sk, &pk)
.unwrap();
let cfg = server.ech_config();
qinfo!("ECHConfigList: {}", hex(cfg));
}
Self {
server,
remaining_data: HashMap::new(),
posts: HashMap::new(),
is_qns_test: args.shared.qns_test.is_some(),
}
}
}
Expand All @@ -72,7 +87,7 @@ impl super::HttpServer for HttpServer {
self.server.process(dgram, now)
}

fn process_events(&mut self, args: &Args, _now: Instant) {
fn process_events(&mut self, _now: Instant) {
while let Some(event) = self.server.next_event() {
match event {
Http3ServerEvent::Headers {
Expand All @@ -97,7 +112,7 @@ impl super::HttpServer for HttpServer {
continue;
};

let mut response = if args.shared.qns_test.is_some() {
let mut response = if self.is_qns_test {
match qns_read_response(path.value()) {
Ok(data) => ResponseData::from(data),
Err(e) => {
Expand Down Expand Up @@ -166,26 +181,6 @@ impl super::HttpServer for HttpServer {
}
}

fn set_qlog_dir(&mut self, dir: Option<PathBuf>) {
self.server.set_qlog_dir(dir);
}

fn validate_address(&mut self, v: ValidateAddress) {
self.server.set_validation(v);
}

fn set_ciphers(&mut self, ciphers: &[Cipher]) {
self.server.set_ciphers(ciphers);
}

fn enable_ech(&mut self) -> &[u8] {
let (sk, pk) = generate_ech_keys().expect("should create ECH keys");
self.server
.enable_ech(random::<1>()[0], "public.example", &sk, &pk)
.unwrap();
self.server.ech_config()
}

fn has_events(&self) -> bool {
self.server.has_events()
}
Expand Down
Loading

0 comments on commit b8ae981

Please sign in to comment.