Merge branch 'master' into pedantic-regressions

.circleci/config.yml

@@ -23,11 +23,11 @@ jobs:
 
       - run:
           name: Build
-          command: NSS_JOBS=3 cargo build -v --all-targets --all-features
+          command: NSS_JOBS=3 cargo build -v --all-targets --tests
 
       - run:
           name: Clippy
-          command: NSS_JOBS=3 cargo clippy -v --all-targets --all-features
+          command: NSS_JOBS=3 cargo clippy -v --all-targets --tests
 
       - run:
           name: Test

neqo-crypto/Cargo.toml

@@ -17,3 +17,6 @@ toml = "0.4"
 
 [dev-dependencies]
 test-fixture = { path = "../test-fixture" }
+
+[features]
+gecko = []

neqo-crypto/build.rs

@@ -145,7 +145,16 @@ fn build_nss(dir: PathBuf) {
     assert!(status.success(), "NSS build failed");
 }
 
-fn dynamic_link(extra_libs: &[&str]) {
+fn dynamic_link() {
+    let libs = if env::consts::OS == "windows" {
+        &["nssutil3.dll", "nss3.dll", "ssl3.dll"]
+    } else {
+        &["nssutil3", "nss3", "ssl3"]
+    };
+    dynamic_link_both(libs);
+}
+
+fn dynamic_link_both(extra_libs: &[&str]) {
     let nspr_libs = if env::consts::OS == "windows" {
         &["libplds4", "libplc4", "libnspr4"]
     } else {
@@ -195,7 +204,7 @@ fn static_link(nsstarget: &PathBuf) {
     if env::consts::OS == "macos" {
         other_libs.push("sqlite3");
     }
-    dynamic_link(&other_libs);
+    dynamic_link_both(&other_libs);
 }
 
 fn get_includes(nsstarget: &Path, nssdist: &Path) -> Vec<PathBuf> {
@@ -222,6 +231,18 @@ fn build_bindings(base: &str, bindings: &Bindings, includes: &[PathBuf]) {
     for i in includes {
         builder = builder.clang_arg(String::from("-I") + i.to_str().unwrap());
     }
+
+    builder = builder.clang_arg("-DNO_NSPR_10_SUPPORT");
+    if env::consts::OS == "windows" {
+        builder = builder.clang_arg("-DWIN");
+    } else if env::consts::OS == "macos" {
+        builder = builder.clang_arg("-DDARWIN");
+    } else if env::consts::OS == "linux" {
+        builder = builder.clang_arg("-DLINUX");
+    } else if env::consts::OS == "android" {
+        builder = builder.clang_arg("-DLINUX");
+        builder = builder.clang_arg("-DANDROID");
+    }
     if bindings.cplusplus {
         builder = builder.clang_args(&["-x", "c++", "-std=c++11"]);
     }
@@ -253,9 +274,7 @@ fn build_bindings(base: &str, bindings: &Bindings, includes: &[PathBuf]) {
         .expect("couldn't write bindings");
 }
 
-fn main() {
-    setup_clang();
-
+fn setup_standalone() -> Vec<PathBuf> {
     println!("cargo:rerun-if-env-changed=NSS_DIR");
     let nss = nss_dir();
     build_nss(nss.clone());
@@ -278,12 +297,52 @@ fn main() {
     if is_debug() {
         static_link(&nsstarget);
     } else {
-        let libs = if env::consts::OS == "windows" {
-            &["nssutil3.dll", "nss3.dll", "ssl3.dll"]
-        } else {
-            &["nssutil3", "nss3", "ssl3"]
-        };
-        dynamic_link(libs);
+        dynamic_link();
+    }
+    includes
+}
+
+fn setup_for_gecko() -> Vec<PathBuf> {
+    let mut includes: Vec<PathBuf> = Vec::new();
+    dynamic_link();
+
+    match env::var_os("MOZ_TOPOBJDIR").map(PathBuf::from) {
+        Some(path) => {
+            let nsprinclude = path.join("dist").join("include").join("nspr");
+            includes.push(nsprinclude);
+            let nssinclude = path.join("dist").join("include").join("nss");
+            includes.push(nssinclude);
+            for i in &includes {
+                println!("cargo:include={}", i.to_str().unwrap());
+            }
+        }
+        None => {
+            println!("cargo:warning={}", "MOZ_TOPOBJDIR should be set by default, otherwise the build is not guaranteed to finish.");
+        }
+    }
+    includes
+}
+
+fn main() {
+    setup_clang();
+
+    let includes = if cfg!(feature = "gecko") {
+        setup_for_gecko()
+    } else {
+        setup_standalone()
+    };
+
+    let mut flags: Vec<String> = Vec::new();
+    flags.push(String::from("-DNO_NSPR_10_SUPPORT"));
+    if env::consts::OS == "windows" {
+        flags.push(String::from("-DWIN"));
+    } else if env::consts::OS == "macos" {
+        flags.push(String::from("-DDARWIN"));
+    } else if env::consts::OS == "linux" {
+        flags.push(String::from("-DLINUX"));
+    } else if env::consts::OS == "android" {
+        flags.push(String::from("-DLINUX"));
+        flags.push(String::from("-DANDROID"));
     }
 
     let config_file = PathBuf::from(BINDINGS_DIR).join(BINDINGS_CONFIG);

neqo-crypto/src/agent.rs

@@ -421,13 +421,13 @@ impl SecretAgent {
 
         // NSS inherited an idiosyncratic API as a result of having implemented NPN
         // before ALPN.  For that reason, we need to put the "best" option last.
-        for v in protocols.iter().skip(1) {
-            add(v.as_ref())
+        let (first, rest) = protocols
+            .split_first()
+            .expect("at least one ALPN value needed");
+        for v in rest {
+            add(v.as_ref());
         }
-        add(protocols
-            .first()
-            .expect("at least one ALPN value needs to be provided")
-            .as_ref());
+        add(first.as_ref());
         assert_eq!(encoded_len, encoded.len());
 
         // Now give the result to NSS.

neqo-transport/src/connection.rs

@@ -8,7 +8,7 @@
 
 #![allow(dead_code)]
 use std::cell::RefCell;
-use std::cmp::{max, min, Ordering};
+use std::cmp::{max, Ordering};
 use std::collections::HashMap;
 use std::convert::TryInto;
 use std::fmt::{self, Debug};
@@ -54,7 +54,12 @@ const CID_LENGTH: usize = 8;
 
 pub const LOCAL_STREAM_LIMIT_BIDI: u64 = 16;
 pub const LOCAL_STREAM_LIMIT_UNI: u64 = 16;
+
+#[cfg(not(test))]
 const LOCAL_MAX_DATA: u64 = 0x3FFF_FFFF_FFFF_FFFE; // 2^62-1
+#[cfg(test)]
+const LOCAL_MAX_DATA: u64 = 0x3FFF; // 16,383
+
 const LOCAL_IDLE_TIMEOUT: Duration = Duration::from_secs(60); // 1 minute
 
 #[derive(Debug, PartialEq, Copy, Clone)]
@@ -1158,6 +1163,24 @@ impl Connection {
         Ok(())
     }
 
+    fn handle_max_data(&mut self, maximum_data: u64) {
+        let conn_was_blocked = self.flow_mgr.borrow().conn_credit_avail() == 0;
+        let conn_credit_increased = self
+            .flow_mgr
+            .borrow_mut()
+            .conn_increase_max_credit(maximum_data);
+
+        if conn_was_blocked && conn_credit_increased {
+            for (id, ss) in &mut self.send_streams {
+                if ss.avail() > 0 {
+                    // These may not actually all be writable if one
+                    // uses up all the conn credit. Not our fault.
+                    self.events.send_stream_writable(*id)
+                }
+            }
+        }
+    }
+
     fn input_frame(&mut self, epoch: Epoch, frame: Frame, now: Instant) -> Res<()> {
         match frame {
             Frame::Padding => {
@@ -1229,10 +1252,7 @@ impl Connection {
                     rs.inbound_stream_frame(fin, offset, data)?;
                 }
             }
-            Frame::MaxData { maximum_data } => self
-                .flow_mgr
-                .borrow_mut()
-                .conn_increase_max_credit(maximum_data),
+            Frame::MaxData { maximum_data } => self.handle_max_data(maximum_data),
             Frame::MaxStreamData {
                 stream_id,
                 maximum_stream_data,
@@ -1689,10 +1709,7 @@ impl Connection {
     /// i.e. that will not be blocked by flow credits or send buffer max
     /// capacity.
     pub fn stream_avail_send_space(&self, stream_id: u64) -> Res<u64> {
-        Ok(min(
-            self.send_streams.get(stream_id.into())?.avail(),
-            self.flow_mgr.borrow().conn_credit_avail(),
-        ))
+        Ok(self.send_streams.get(stream_id.into())?.avail())
     }
 
     /// Close the stream. Enqueued data will be sent.
@@ -1732,7 +1749,7 @@ impl Connection {
 
     /// Get events that indicate state changes on the connection.
     pub fn events(&mut self) -> impl Iterator<Item = ConnectionEvent> {
-        self.events.events().into_iter()
+        self.events.events()
     }
 
     fn check_loss_detection_timeout(&mut self, now: Instant) {
@@ -2560,4 +2577,41 @@ mod tests {
         client.process_timer(now + Duration::from_secs(80));
         assert!(matches!(client.state(), State::Closing{..}));
     }
+
+    #[test]
+    fn max_data() {
+        let mut client = default_client();
+        let mut server = default_server();
+        connect(&mut client, &mut server);
+
+        let stream_id = client.stream_create(StreamType::UniDi).unwrap();
+        assert_eq!(stream_id, 2);
+        assert_eq!(
+            client.stream_avail_send_space(stream_id).unwrap(),
+            LOCAL_MAX_DATA // 16383, when cfg(test)
+        );
+        assert_eq!(
+            client
+                .stream_send(stream_id, &[b'a'; RX_STREAM_DATA_WINDOW as usize])
+                .unwrap(),
+            LOCAL_MAX_DATA as usize
+        );
+        let evts = client.events().collect::<Vec<_>>();
+        assert_eq!(evts.len(), 2); // SendStreamWritable, StateChange(connected)
+        assert_eq!(client.stream_send(stream_id, b"hello").unwrap(), 0);
+        let ss = client.send_streams.get_mut(stream_id.into()).unwrap();
+        ss.mark_as_sent(0, 4096, false);
+        ss.mark_as_acked(0, 4096, false);
+
+        // no event because still limited by conn max data
+        let evts = client.events().collect::<Vec<_>>();
+        assert_eq!(evts.len(), 0);
+
+        // increase max data
+        client.handle_max_data(100_000);
+        assert_eq!(client.stream_avail_send_space(stream_id).unwrap(), 49152);
+        let evts = client.events().collect::<Vec<_>>();
+        assert_eq!(evts.len(), 1);
+        assert!(matches!(evts[0], ConnectionEvent::SendStreamWritable{..}));
+    }
 }

neqo-transport/src/events.rs

@@ -30,7 +30,7 @@ pub enum ConnectionEvent {
     RecvStreamReadable { stream_id: u64 },
     /// Peer reset the stream.
     RecvStreamReset { stream_id: u64, app_error: AppError },
-    /// Peer has sent STOP_SENDIconnectioNG
+    /// Peer has sent STOP_SENDING
     SendStreamStopSending { stream_id: u64, app_error: AppError },
     /// Peer has acked everything sent on the stream.
     SendStreamComplete { stream_id: u64 },
@@ -107,8 +107,8 @@ impl ConnectionEvents {
         self.insert(ConnectionEvent::ZeroRttRejected);
     }
 
-    pub fn events(&self) -> BTreeSet<ConnectionEvent> {
-        self.events.replace(BTreeSet::new())
+    pub fn events(&self) -> impl Iterator<Item = ConnectionEvent> {
+        self.events.replace(BTreeSet::new()).into_iter()
     }
 
     fn insert(&self, event: ConnectionEvent) {

neqo-transport/src/flow_mgr.rs

@@ -7,7 +7,6 @@
 // Tracks possibly-redundant flow control signals from other code and converts
 // into flow control frames needing to be sent to the remote.
 
-use std::cmp::max;
 use std::collections::HashMap;
 use std::mem;
 
@@ -52,8 +51,14 @@ impl FlowMgr {
         assert!(self.used_data <= self.max_data)
     }
 
-    pub fn conn_increase_max_credit(&mut self, new: u64) {
-        self.max_data = max(self.max_data, new)
+    /// Returns whether max credit was actually increased.
+    pub fn conn_increase_max_credit(&mut self, new: u64) -> bool {
+        if new > self.max_data {
+            self.max_data = new;
+            true
+        } else {
+            false
+        }
     }
 
     // -- frames scoped on connection --

neqo-transport/src/recovery.rs

@@ -400,20 +400,11 @@ impl LossRecovery {
     pub fn get_timer(&mut self, conn_state: &State) -> LossRecoveryState {
         qdebug!([self] "get_loss_detection_timer.");
 
-        let mut has_ack_eliciting_out = false;
-        for sp in self
+        let has_ack_eliciting_out = self
             .spaces
             .iter()
             .flat_map(|spc| self.spaces[*spc].sent_packets.values())
-        {
-            if sp.ack_eliciting {
-                has_ack_eliciting_out = true
-            }
-
-            if has_ack_eliciting_out {
-                break;
-            }
-        }
+            .any(|sp| sp.ack_eliciting);
 
         qdebug!(
             [self]