Merge remote-tracking branch 'origin/main' into redo-1662

.codecov.yml

@@ -1,6 +1,7 @@
 # neqo has no test coverage for its example client and server
 ignore:
   - "neqo-bin"
+  - "test-fixture"
 
 # Do not notify until at least three results have been uploaded from the CI pipeline.
 # (This corresponds to the three main platforms we support: Linux, macOS, and Windows.)

.dockerignore

@@ -5,5 +5,6 @@
 !**/*.rs
 !**/*.h
 !**/*.hpp
+!neqo-crypto/min_version.txt
 !qns
 !Cargo.lock

.github/actions/pr-comment/action.yml

@@ -5,6 +5,9 @@ inputs:
   name:
     description: 'Artifact name to import comment data from.'
     required: true
+  mode:
+    description: 'Mode of operation (upsert/recreate/delete).'
+    default: 'upsert'
   token:
     description: 'A Github PAT'
     required: true
@@ -29,5 +32,6 @@ runs:
     - uses: thollander/actions-comment-pull-request@v2
       with:
         filePath: contents
+        mode: ${{ inputs.mode }}
         pr_number: ${{ steps.pr-number.outputs.number }}
         comment_tag: ${{ inputs.name }}-comment

.github/actions/quic-interop-runner/action.yml

@@ -93,9 +93,13 @@ runs:
       run: |
         echo '[**QUIC Interop Runner**](https://github.com/quic-interop/quic-interop-runner)' >> comment
         echo '' >> comment
-        # Ignore all, but table, which starts with "|".
-        grep -E '^\|' quic-interop-runner/summary >> comment
+        # Ignore all, but table, which starts with "|". Also reformat it to GitHub Markdown.
+        grep -E '^\|' quic-interop-runner/summary |\
+          awk '(!/^\| *:-/ || (d++ && d < 3))' |\
+          sed -E -e 's/✓/:white_check_mark:/gi' -e 's/✕/:x:/gi' -e 's/\?/:grey_question:/gi' \
+          >> comment
         echo '' >> comment
+        echo "EXPORT_COMMENT=1" >> "$GITHUB_ENV"
       shell: bash
 
     - name: Export PR comment data

.github/workflows/qns-comment.yml

@@ -18,11 +18,11 @@ jobs:
       pull-requests: write
     runs-on: ubuntu-latest
     if: |
-      github.event.workflow_run.event == 'pull_request' &&
-      github.event.workflow_run.conclusion == 'failure'
+      github.event.workflow_run.event == 'pull_request'
     steps:
       - uses: actions/checkout@v4
       - uses: ./.github/actions/pr-comment
         with:
           name: qns
+          mode: ${{ github.event.workflow_run.conclusion == 'success' && 'delete' || 'upsert' }}
           token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/qns.yml

@@ -71,6 +71,6 @@ jobs:
           name: 'neqo-latest'
           image: ${{ steps.docker_build_and_push.outputs.imageID }}
           url: https://github.com/mozilla/neqo
-          test: handshake
+          test: handshake,ecn,keyupdate
           client: neqo-latest,quic-go,ngtcp2,neqo,msquic
           server: neqo-latest,quic-go,ngtcp2,neqo,msquic

Cargo.toml

@@ -14,14 +14,18 @@ resolver = "2"
 homepage = "https://github.com/mozilla/neqo/"
 repository = "https://github.com/mozilla/neqo/"
 authors = ["The Neqo Authors <[email protected]>"]
-version = "0.7.2"
+version = "0.7.5"
 # Keep in sync with `.rustfmt.toml` `edition`.
 edition = "2021"
 license = "MIT OR Apache-2.0"
 # Don't increase beyond what Firefox is currently using:
-# https://firefox-source-docs.mozilla.org/writing-rust-code/update-policy.html#schedule
+# https://searchfox.org/mozilla-central/search?q=MINIMUM_RUST_VERSION&path=python/mozboot/mozboot/util.py
 rust-version = "1.74.0"
 
+[workspace.dependencies]
+log = { version = "0.4", default-features = false }
+qlog = { version = "0.13", default-features = false }
+
 [workspace.lints.clippy]
 pedantic = { level = "warn", priority = -1 }
 

neqo-bin/Cargo.toml

@@ -28,13 +28,13 @@ clap = { version = "4.4", default-features = false, features = ["std", "color",
 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"] }
-log = { version = "0.4", default-features = false }
+log = { workspace = true }
 neqo-common = { path = "./../neqo-common" }
 neqo-crypto = { path = "./../neqo-crypto" }
 neqo-http3 = { path = "./../neqo-http3" }
 neqo-qpack = { path = "./../neqo-qpack" }
 neqo-transport = { path = "./../neqo-transport" }
-qlog = { version = "0.12", default-features = false }
+qlog = { workspace = true }
 quinn-udp = { git = "https://github.com/quinn-rs/quinn/", rev = "a947962131aba8a6521253d03cc948b20098a2d6" }
 regex = { version = "1.9", default-features = false, features = ["unicode-perl"] }
 tokio = { version = "1", default-features = false, features = ["net", "time", "macros", "rt", "rt-multi-thread"] }

neqo-bin/benches/main.rs

@@ -13,8 +13,6 @@ use tokio::runtime::Runtime;
 struct Benchmark {
     name: String,
     requests: Vec<u64>,
-    /// Download resources in series using separate connections.
-    download_in_series: bool,
     sample_size: Option<usize>,
 }
 
@@ -27,25 +25,21 @@ fn transfer(c: &mut Criterion) {
     for Benchmark {
         name,
         requests,
-        download_in_series,
         sample_size,
     } in [
         Benchmark {
             name: "1-conn/1-100mb-resp (aka. Download)".to_string(),
             requests: vec![100 * 1024 * 1024],
-            download_in_series: false,
             sample_size: Some(10),
         },
         Benchmark {
-            name: "1-conn/10_000-1b-seq-resp (aka. RPS)".to_string(),
+            name: "1-conn/10_000-parallel-1b-resp (aka. RPS)".to_string(),
             requests: vec![1; 10_000],
-            download_in_series: false,
             sample_size: None,
         },
         Benchmark {
-            name: "100-seq-conn/1-1b-resp (aka. HPS)".to_string(),
-            requests: vec![1; 100],
-            download_in_series: true,
+            name: "1-conn/1-1b-resp (aka. HPS)".to_string(),
+            requests: vec![1; 1],
             sample_size: None,
         },
     ] {
@@ -61,7 +55,7 @@ fn transfer(c: &mut Criterion) {
         }
         group.bench_function("client", |b| {
             b.to_async(Runtime::new().unwrap()).iter_batched(
-                || client::client(client::Args::new(&requests, download_in_series)),
+                || client::client(client::Args::new(&requests)),
                 |client| async move {
                     client.await.unwrap();
                 },
@@ -81,7 +75,7 @@ fn spawn_server() -> tokio::sync::oneshot::Sender<()> {
             let mut server = Box::pin(server::server(server::Args::default()));
             tokio::select! {
                 _ = &mut done_receiver => {}
-                _ = &mut server  => {}
+                res = &mut server  => panic!("expect server not to terminate: {res:?}"),
             }
         });
     });

neqo-bin/src/client/http09.rs

@@ -20,27 +20,39 @@ use std::{
 use neqo_common::{event::Provider, qdebug, qinfo, qwarn, Datagram};
 use neqo_crypto::{AuthenticationStatus, ResumptionToken};
 use neqo_transport::{
-    Connection, ConnectionEvent, EmptyConnectionIdGenerator, Error, Output, State, StreamId,
-    StreamType,
+    Connection, ConnectionError, ConnectionEvent, EmptyConnectionIdGenerator, Error, Output, State,
+    StreamId, StreamType,
 };
 use url::Url;
 
-use super::{get_output_file, qlog_new, Args, KeyUpdateState, Res};
+use super::{get_output_file, qlog_new, Args, Res};
 
 pub struct Handler<'a> {
     streams: HashMap<StreamId, Option<BufWriter<File>>>,
     url_queue: VecDeque<Url>,
     all_paths: Vec<PathBuf>,
     args: &'a Args,
     token: Option<ResumptionToken>,
-    key_update: KeyUpdateState,
+    needs_key_update: bool,
 }
 
 impl<'a> super::Handler for Handler<'a> {
     type Client = Connection;
 
     fn handle(&mut self, client: &mut Self::Client) -> Res<bool> {
         while let Some(event) = client.next_event() {
+            if self.needs_key_update {
+                match client.initiate_key_update() {
+                    Ok(()) => {
+                        qdebug!("Keys updated");
+                        self.needs_key_update = false;
+                        self.download_urls(client);
+                    }
+                    Err(neqo_transport::Error::KeyUpdateBlocked) => (),
+                    Err(e) => return Err(e.into()),
+                }
+            }
+
             match event {
                 ConnectionEvent::AuthenticationNeeded => {
                     client.authenticated(AuthenticationStatus::Ok, Instant::now());
@@ -66,9 +78,6 @@ impl<'a> super::Handler for Handler<'a> {
                     qdebug!("{event:?}");
                     self.download_urls(client);
                 }
-                ConnectionEvent::StateChange(State::Confirmed) => {
-                    self.maybe_key_update(client)?;
-                }
                 ConnectionEvent::ResumptionToken(token) => {
                     self.token = Some(token);
                 }
@@ -86,12 +95,6 @@ impl<'a> super::Handler for Handler<'a> {
         Ok(false)
     }
 
-    fn maybe_key_update(&mut self, c: &mut Self::Client) -> Res<()> {
-        self.key_update.maybe_update(|| c.initiate_key_update())?;
-        self.download_urls(c);
-        Ok(())
-    }
-
     fn take_token(&mut self) -> Option<ResumptionToken> {
         self.token.take()
     }
@@ -138,35 +141,51 @@ pub(crate) fn create_client(
 }
 
 impl super::Client for Connection {
-    fn process(&mut self, dgram: Option<&Datagram>, now: Instant) -> Output {
-        self.process(dgram, now)
+    fn process_output(&mut self, now: Instant) -> Output {
+        self.process_output(now)
+    }
+
+    fn process_multiple_input<'a, I>(&mut self, dgrams: I, now: Instant)
+    where
+        I: IntoIterator<Item = &'a Datagram>,
+    {
+        self.process_multiple_input(dgrams, now);
     }
 
     fn close<S>(&mut self, now: Instant, app_error: neqo_transport::AppError, msg: S)
     where
         S: AsRef<str> + std::fmt::Display,
     {
-        self.close(now, app_error, msg);
+        if !self.state().closed() {
+            self.close(now, app_error, msg);
+        }
     }
 
-    fn is_closed(&self) -> bool {
-        matches!(self.state(), State::Closed(..))
+    fn is_closed(&self) -> Option<ConnectionError> {
+        if let State::Closed(err) = self.state() {
+            return Some(err.clone());
+        }
+        None
     }
 
     fn stats(&self) -> neqo_transport::Stats {
         self.stats()
     }
+
+    fn has_events(&self) -> bool {
+        neqo_common::event::Provider::has_events(self)
+    }
 }
 
 impl<'b> Handler<'b> {
-    pub fn new(url_queue: VecDeque<Url>, args: &'b Args, key_update: KeyUpdateState) -> Self {
+    pub fn new(url_queue: VecDeque<Url>, args: &'b Args) -> Self {
         Self {
             streams: HashMap::new(),
             url_queue,
             all_paths: Vec::new(),
             args,
             token: None,
-            key_update,
+            needs_key_update: args.key_update,
         }
     }
 
@@ -185,7 +204,7 @@ impl<'b> Handler<'b> {
     }
 
     fn download_next(&mut self, client: &mut Connection) -> bool {
-        if self.key_update.needed() {
+        if self.needs_key_update {
             qdebug!("Deferring requests until after first key update");
             return false;
         }