diff --git a/.cargo/config.in b/.cargo/config.in index 7a321d933326..70272d5604a7 100644 --- a/.cargo/config.in +++ b/.cargo/config.in @@ -10,7 +10,7 @@ replace-with = "vendored-sources" [source."https://github.com/mozilla/neqo"] git = "https://github.com/mozilla/neqo" replace-with = "vendored-sources" -tag = "v0.4.13" +tag = "v0.4.14" [source."https://github.com/mozilla/mp4parse-rust"] git = "https://github.com/mozilla/mp4parse-rust" diff --git a/Cargo.lock b/Cargo.lock index 5c030b1ef875..7527daa928ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3237,11 +3237,11 @@ dependencies = [ [[package]] name = "neqo-common" -version = "0.4.13" -source = "git+https://github.com/mozilla/neqo?tag=v0.4.13#5cc310cc6d86260e6125770a2a7592e6a6bffa79" +version = "0.4.14" +source = "git+https://github.com/mozilla/neqo?tag=v0.4.14#ec00592e3332b7a0eb4b88457a89c24c55bf3974" dependencies = [ "chrono", - "env_logger 0.6.2", + "env_logger 0.7.1", "lazy_static", "log", "qlog", @@ -3249,8 +3249,8 @@ dependencies = [ [[package]] name = "neqo-crypto" -version = "0.4.13" -source = "git+https://github.com/mozilla/neqo?tag=v0.4.13#5cc310cc6d86260e6125770a2a7592e6a6bffa79" +version = "0.4.14" +source = "git+https://github.com/mozilla/neqo?tag=v0.4.14#ec00592e3332b7a0eb4b88457a89c24c55bf3974" dependencies = [ "bindgen", "log", @@ -3262,8 +3262,8 @@ dependencies = [ [[package]] name = "neqo-http3" -version = "0.4.13" -source = "git+https://github.com/mozilla/neqo?tag=v0.4.13#5cc310cc6d86260e6125770a2a7592e6a6bffa79" +version = "0.4.14" +source = "git+https://github.com/mozilla/neqo?tag=v0.4.14#ec00592e3332b7a0eb4b88457a89c24c55bf3974" dependencies = [ "log", "neqo-common", @@ -3276,8 +3276,8 @@ dependencies = [ [[package]] name = "neqo-qpack" -version = "0.4.13" -source = "git+https://github.com/mozilla/neqo?tag=v0.4.13#5cc310cc6d86260e6125770a2a7592e6a6bffa79" +version = "0.4.14" +source = "git+https://github.com/mozilla/neqo?tag=v0.4.14#ec00592e3332b7a0eb4b88457a89c24c55bf3974" dependencies = [ "lazy_static", "log", @@ -3290,8 +3290,8 @@ dependencies = [ [[package]] name = "neqo-transport" -version = "0.4.13" -source = "git+https://github.com/mozilla/neqo?tag=v0.4.13#5cc310cc6d86260e6125770a2a7592e6a6bffa79" +version = "0.4.14" +source = "git+https://github.com/mozilla/neqo?tag=v0.4.14#ec00592e3332b7a0eb4b88457a89c24c55bf3974" dependencies = [ "indexmap", "lazy_static", diff --git a/netwerk/socket/neqo_glue/Cargo.toml b/netwerk/socket/neqo_glue/Cargo.toml index 76f3404bb983..5188259d41af 100644 --- a/netwerk/socket/neqo_glue/Cargo.toml +++ b/netwerk/socket/neqo_glue/Cargo.toml @@ -8,10 +8,10 @@ edition = "2018" name = "neqo_glue" [dependencies] -neqo-http3 = { tag = "v0.4.13", git = "https://github.com/mozilla/neqo" } -neqo-transport = { tag = "v0.4.13", git = "https://github.com/mozilla/neqo" } -neqo-common = { tag = "v0.4.13", git = "https://github.com/mozilla/neqo" } -neqo-qpack = { tag = "v0.4.13", git = "https://github.com/mozilla/neqo" } +neqo-http3 = { tag = "v0.4.14", git = "https://github.com/mozilla/neqo" } +neqo-transport = { tag = "v0.4.14", git = "https://github.com/mozilla/neqo" } +neqo-common = { tag = "v0.4.14", git = "https://github.com/mozilla/neqo" } +neqo-qpack = { tag = "v0.4.14", git = "https://github.com/mozilla/neqo" } nserror = { path = "../../../xpcom/rust/nserror" } nsstring = { path = "../../../xpcom/rust/nsstring" } xpcom = { path = "../../../xpcom/rust/xpcom" } @@ -20,7 +20,7 @@ log = "0.4.0" qlog = "0.3.0" [dependencies.neqo-crypto] -tag = "v0.4.13" +tag = "v0.4.14" git = "https://github.com/mozilla/neqo" default-features = false features = ["gecko"] diff --git a/netwerk/test/http3server/Cargo.toml b/netwerk/test/http3server/Cargo.toml index 88dd599df6f3..e6a7c64cc07d 100644 --- a/netwerk/test/http3server/Cargo.toml +++ b/netwerk/test/http3server/Cargo.toml @@ -5,16 +5,16 @@ authors = ["Dragana Damjanovic "] edition = "2018" [dependencies] -neqo-transport = { tag = "v0.4.13", git = "https://github.com/mozilla/neqo" } -neqo-common = { tag = "v0.4.13", git = "https://github.com/mozilla/neqo" } -neqo-http3 = { tag = "v0.4.13", git = "https://github.com/mozilla/neqo" } -neqo-qpack = { tag = "v0.4.13", git = "https://github.com/mozilla/neqo" } +neqo-transport = { tag = "v0.4.14", git = "https://github.com/mozilla/neqo" } +neqo-common = { tag = "v0.4.14", git = "https://github.com/mozilla/neqo" } +neqo-http3 = { tag = "v0.4.14", git = "https://github.com/mozilla/neqo" } +neqo-qpack = { tag = "v0.4.14", git = "https://github.com/mozilla/neqo" } mio = "0.6.17" mio-extras = "2.0.5" log = "0.4.0" [dependencies.neqo-crypto] -tag = "v0.4.13" +tag = "v0.4.14" git = "https://github.com/mozilla/neqo" default-features = false features = ["gecko"] diff --git a/third_party/rust/neqo-common/.cargo-checksum.json b/third_party/rust/neqo-common/.cargo-checksum.json index 6e4b5f641c13..364a46357f69 100644 --- a/third_party/rust/neqo-common/.cargo-checksum.json +++ b/third_party/rust/neqo-common/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"ccb895e84973248c61155c911e6eeb471bab697afa18333c9f029d918ccc6955","src/codec.rs":"3f616cb453c99650d8db3a0f33c2707c83e9ef012c06e48e64a2ef15bc680b3f","src/datagram.rs":"569f8d9e34d7ee17144bf63d34136ecd9778da0d337e513f338738c50284615e","src/event.rs":"f60fee9f4b09ef47ff5e4bfa21c07e45ffd5873c292f2605f24d834070127d62","src/incrdecoder.rs":"f65afa390317ab2a306e8cd7e08f33490dc824242f9be87b2a16b136715f8094","src/lib.rs":"4a2f769b0125bfa8047a908f65eb6ac71720f2e04ac4f3e283721e4162fd15a9","src/log.rs":"b8da388073f72a21128d52b0d0c963e07a3d3cf3368438ae3a50be34b8add3a4","src/qlog.rs":"a8aa4f1f0110076b401f6e5a7057ec154c7ad3677374a21ceca1209469b9c07d","src/timer.rs":"66886b3697e1b4232d9d9892a12d93afd3381812a8ff901bceac4bb48b264607","tests/log.rs":"480b165b7907ec642c508b303d63005eee1427115d6973a349eaf6b2242ed18d"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"d15a2b810befe53f405c44d7f1aa179fc4f1047c3e42ae565230de5b3cd1ffd2","src/codec.rs":"232db2f1f6a5952a58710f5ec442f768dff71846dd108f71374b3ad7d6aea3ff","src/datagram.rs":"569f8d9e34d7ee17144bf63d34136ecd9778da0d337e513f338738c50284615e","src/event.rs":"f60fee9f4b09ef47ff5e4bfa21c07e45ffd5873c292f2605f24d834070127d62","src/incrdecoder.rs":"b97a40f89da6832ad92bd652cb6ceac82a0a5cc68a9b3d0c96f89d02e1ee9902","src/lib.rs":"5af4f0e7284b49d1b03b5eee78f2b814e5fa6eb9424507291e25b1955aebe007","src/log.rs":"b8da388073f72a21128d52b0d0c963e07a3d3cf3368438ae3a50be34b8add3a4","src/qlog.rs":"a8aa4f1f0110076b401f6e5a7057ec154c7ad3677374a21ceca1209469b9c07d","src/timer.rs":"66886b3697e1b4232d9d9892a12d93afd3381812a8ff901bceac4bb48b264607","tests/log.rs":"480b165b7907ec642c508b303d63005eee1427115d6973a349eaf6b2242ed18d"},"package":null} \ No newline at end of file diff --git a/third_party/rust/neqo-common/Cargo.toml b/third_party/rust/neqo-common/Cargo.toml index bc81d9810956..269bd5af6641 100644 --- a/third_party/rust/neqo-common/Cargo.toml +++ b/third_party/rust/neqo-common/Cargo.toml @@ -1,13 +1,13 @@ [package] name = "neqo-common" -version = "0.4.13" +version = "0.4.14" authors = ["Bobby Holley "] edition = "2018" license = "MIT/Apache-2.0" [dependencies] log = {version = "0.4.0", default-features = false} -env_logger = "0.6.1" +env_logger = "0.7.1" lazy_static = "1.3.0" qlog = "0.3.0" chrono = "0.4.10" diff --git a/third_party/rust/neqo-common/src/codec.rs b/third_party/rust/neqo-common/src/codec.rs index 62de044474f7..1824d7f414ed 100644 --- a/third_party/rust/neqo-common/src/codec.rs +++ b/third_party/rust/neqo-common/src/codec.rs @@ -165,7 +165,7 @@ impl<'a> Deref for Decoder<'a> { impl<'a> Debug for Decoder<'a> { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - f.write_str(&hex_with_len(self)) + f.write_str(&hex_with_len(&self[..])) } } @@ -212,6 +212,12 @@ impl Encoder { } } + /// Static helper to determine how long a varint-prefixed array encodes to. + #[must_use] + pub fn vvec_len(len: usize) -> usize { + Self::varint_len(u64::try_from(len).unwrap()) + len + } + /// Default construction of an empty buffer. #[must_use] pub fn new() -> Self { @@ -567,6 +573,40 @@ mod tests { dec.skip_vvec(); } + #[test] + fn encoded_lengths() { + assert_eq!(Encoder::varint_len(0), 1); + assert_eq!(Encoder::varint_len(0x3f), 1); + assert_eq!(Encoder::varint_len(0x40), 2); + assert_eq!(Encoder::varint_len(0x3fff), 2); + assert_eq!(Encoder::varint_len(0x4000), 4); + assert_eq!(Encoder::varint_len(0x3fff_ffff), 4); + assert_eq!(Encoder::varint_len(0x4000_0000), 8); + } + + #[test] + #[should_panic] + fn encoded_length_oob() { + let _ = Encoder::varint_len(1 << 62); + } + + #[test] + fn encoded_vvec_lengths() { + assert_eq!(Encoder::vvec_len(0), 1); + assert_eq!(Encoder::vvec_len(0x3f), 0x40); + assert_eq!(Encoder::vvec_len(0x40), 0x42); + assert_eq!(Encoder::vvec_len(0x3fff), 0x4001); + assert_eq!(Encoder::vvec_len(0x4000), 0x4004); + assert_eq!(Encoder::vvec_len(0x3fff_ffff), 0x4000_0003); + assert_eq!(Encoder::vvec_len(0x4000_0000), 0x4000_0008); + } + + #[test] + #[should_panic] + fn encoded_vvec_length_oob() { + let _ = Encoder::vvec_len(1 << 62); + } + #[test] fn encode_byte() { let mut enc = Encoder::default(); diff --git a/third_party/rust/neqo-common/src/incrdecoder.rs b/third_party/rust/neqo-common/src/incrdecoder.rs index 44a9216cb905..1ece719e2ca1 100644 --- a/third_party/rust/neqo-common/src/incrdecoder.rs +++ b/third_party/rust/neqo-common/src/incrdecoder.rs @@ -18,11 +18,7 @@ pub struct IncrementalDecoderUint { impl IncrementalDecoderUint { #[must_use] pub fn min_remaining(&self) -> usize { - if let Some(r) = self.remaining { - r - } else { - 1 - } + self.remaining.unwrap_or(1) } pub fn consume(&mut self, dv: &mut Decoder) -> Option { diff --git a/third_party/rust/neqo-common/src/lib.rs b/third_party/rust/neqo-common/src/lib.rs index 9d512cdbaf64..ba049e95325d 100644 --- a/third_party/rust/neqo-common/src/lib.rs +++ b/third_party/rust/neqo-common/src/lib.rs @@ -25,17 +25,18 @@ pub use self::incrdecoder::{ extern crate lazy_static; #[must_use] -pub fn hex(buf: &[u8]) -> String { - let mut ret = String::with_capacity(buf.len() * 2); - for b in buf { +pub fn hex(buf: impl AsRef<[u8]>) -> String { + let mut ret = String::with_capacity(buf.as_ref().len() * 2); + for b in buf.as_ref() { ret.push_str(&format!("{:02x}", b)); } ret } #[must_use] -pub fn hex_snip_middle(buf: &[u8]) -> String { +pub fn hex_snip_middle(buf: impl AsRef<[u8]>) -> String { const SHOW_LEN: usize = 8; + let buf = buf.as_ref(); if buf.len() <= SHOW_LEN * 2 { hex_with_len(buf) } else { @@ -53,7 +54,8 @@ pub fn hex_snip_middle(buf: &[u8]) -> String { } #[must_use] -pub fn hex_with_len(buf: &[u8]) -> String { +pub fn hex_with_len(buf: impl AsRef<[u8]>) -> String { + let buf = buf.as_ref(); let mut ret = String::with_capacity(10 + buf.len() * 2); ret.push_str(&format!("[{}]: ", buf.len())); for b in buf { diff --git a/third_party/rust/neqo-crypto/.cargo-checksum.json b/third_party/rust/neqo-crypto/.cargo-checksum.json index 9adae5bd12d9..0d7a502966f3 100644 --- a/third_party/rust/neqo-crypto/.cargo-checksum.json +++ b/third_party/rust/neqo-crypto/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"5993b4b76447b8aef5e85d9fc32aec6c49be3a61ed9de1fe1b4adad49022ebf2","TODO":"ac0f1c2ebcca03f5b3c0cc56c5aedbb030a4b511e438bc07a57361c789f91e9f","bindings/bindings.toml":"a896b4accf5fbaf146a45a060142974bfa2f59d6a5ab18c5080753078ae39474","bindings/mozpkix.hpp":"77072c8bb0f6eb6bfe8cbadc111dcd92e0c79936d13f2e501aae1e5d289a6675","bindings/nspr_err.h":"2d5205d017b536c2d838bcf9bc4ec79f96dd50e7bb9b73892328781f1ee6629d","bindings/nspr_error.h":"e41c03c77b8c22046f8618832c9569fbcc7b26d8b9bbc35eea7168f35e346889","bindings/nspr_io.h":"085b289849ef0e77f88512a27b4d9bdc28252bd4d39c6a17303204e46ef45f72","bindings/nspr_time.h":"2e637fd338a5cf0fd3fb0070a47f474a34c2a7f4447f31b6875f5a9928d0a261","bindings/nss_ciphers.h":"95ec6344a607558b3c5ba8510f463b6295f3a2fb3f538a01410531045a5f62d1","bindings/nss_init.h":"ef49045063782fb612aff459172cc6a89340f15005808608ade5320ca9974310","bindings/nss_p11.h":"0b81e64fe6db49b2ecff94edd850be111ef99ec11220e88ceb1c67be90143a78","bindings/nss_secerr.h":"713e8368bdae5159af7893cfa517dabfe5103cede051dee9c9557c850a2defc6","bindings/nss_ssl.h":"af222fb957b989e392e762fa2125c82608a0053aff4fb97e556691646c88c335","bindings/nss_sslerr.h":"24b97f092183d8486f774cdaef5030d0249221c78343570d83a4ee5b594210ae","bindings/nss_sslopt.h":"b7807eb7abdad14db6ad7bc51048a46b065a0ea65a4508c95a12ce90e59d1eea","build.rs":"0be611e6b25a18d5ed724071a3a471c2c5b584649b5fe36de28a56ed8caaf800","src/aead.rs":"e26ad34f7168f42aa87eb3a6455af3191a0e8555782b1d70032fe1d248922f98","src/agent.rs":"9dae2fa87a5a7b65dfc7fdd1b2ab1be0f75f1d9ee6704b44edd9bd406252b10a","src/agentio.rs":"cc562d09a09719b90b4e1d147fd579e3e89b683448709e920033bceaea108a61","src/auth.rs":"71ac7e297a5f872d26cf67b6bbd96e4548ea38374bdd84c1094f76a5de4ed1cb","src/cert.rs":"fd3fd2bbb38754bdcee3898549feae412943c9f719032531c1ad6e61783b5394","src/constants.rs":"c39ee506a10d685fda77c1d2ddf691b595b067b4e1044ac7a21e360119d6002b","src/err.rs":"04f38831ca62d29d8aadfe9daf95fd29e68ece184e6d3e00bfb9ee1d12744033","src/exp.rs":"61586662407359c1ecb8ed4987bc3c702f26ba2e203a091a51b6d6363cbd510f","src/ext.rs":"97cba23247e5f9656f27587214f7d7370a69174bae5960a012ce3e6fc99f9116","src/hkdf.rs":"40e44f4280497ef525c2b4c465f14f06d241150851668b264ee958f74321cfbe","src/hp.rs":"7fce64e0cc3a6a7e744bc797886bcfaa39679f0a81853b2e55ea0f54fb6bf700","src/lib.rs":"3b22108a069c8c9f1b78e94d48c8759b17e0941b28e2def3fa343d6acace4b6d","src/once.rs":"b9850384899a1a016e839743d3489c0d4d916e1973746ef8c89872105d7d9736","src/p11.rs":"0b62ee5938aefb82e8faee5aa14e990a00442cc9744e8ba22eda80b32030c42c","src/prio.rs":"bc4e97049563b136cb7b39f5171e7909d56a77ed46690aaacb781eeb4a4743e0","src/replay.rs":"40924865994396441a68e6009ecbdf352d6a02fdf539aa65604124e26bffb4d3","src/result.rs":"cef34dfcb907723e195b56501132e4560e250b327783cb5e41201da5b63e9b5c","src/secrets.rs":"acb5befa74e06281c6f80d7298efc58f568bb4e6d949b4225c335e3f392be741","src/selfencrypt.rs":"429cb889a4e9e2345888cc033115c0aa306d2ff90bdfe22b3067700eb1426c37","src/ssl.rs":"3e3a4f539f3c4d18bd6e774dc34fca611db0c75bba00badcd2078c975db055bf","src/time.rs":"5b2ab4028b04b6245c666f33f1c1449816d3d1eb8141f723f5773f21f8fe4388","tests/aead.rs":"a1d8eb69f5672e064f84dce3d214b347a396718e3de56d57ccc108ee87f1cbc1","tests/agent.rs":"d43e5b05dcc845394d3c7312974faae0fdcbc325c07c970aeb7ef30c3ade652e","tests/ext.rs":"eba9f03accdd598e38292ac88263a81b367d60d5a736a43117a3663de105ec48","tests/handshake.rs":"93c478fcd07d29691007abd6dcfcd2014c10c23b0206ba2d97d01594e4d64397","tests/hkdf.rs":"539235e9dcf2a56b72961a9a04f0080409adf6bf465bfad7c30026421b2d4326","tests/hp.rs":"e52a7d2f4387f2dfe8bfe1da5867e8e0d3eb51e171c6904e18b18c4343536af8","tests/init.rs":"20aad800ac793aaf83059cf860593750509fdedeeff0c08a648e7a5cb398dae0","tests/selfencrypt.rs":"46e9a1a09c2ae577eb106d23a5cdacf762575c0dea1948aedab06ef7389ce713"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"e4720002b381baa9ef4a2029a79155095d3126465b61e7b49b48618f0e645781","TODO":"ac0f1c2ebcca03f5b3c0cc56c5aedbb030a4b511e438bc07a57361c789f91e9f","bindings/bindings.toml":"a896b4accf5fbaf146a45a060142974bfa2f59d6a5ab18c5080753078ae39474","bindings/mozpkix.hpp":"77072c8bb0f6eb6bfe8cbadc111dcd92e0c79936d13f2e501aae1e5d289a6675","bindings/nspr_err.h":"2d5205d017b536c2d838bcf9bc4ec79f96dd50e7bb9b73892328781f1ee6629d","bindings/nspr_error.h":"e41c03c77b8c22046f8618832c9569fbcc7b26d8b9bbc35eea7168f35e346889","bindings/nspr_io.h":"085b289849ef0e77f88512a27b4d9bdc28252bd4d39c6a17303204e46ef45f72","bindings/nspr_time.h":"2e637fd338a5cf0fd3fb0070a47f474a34c2a7f4447f31b6875f5a9928d0a261","bindings/nss_ciphers.h":"95ec6344a607558b3c5ba8510f463b6295f3a2fb3f538a01410531045a5f62d1","bindings/nss_init.h":"ef49045063782fb612aff459172cc6a89340f15005808608ade5320ca9974310","bindings/nss_p11.h":"0b81e64fe6db49b2ecff94edd850be111ef99ec11220e88ceb1c67be90143a78","bindings/nss_secerr.h":"713e8368bdae5159af7893cfa517dabfe5103cede051dee9c9557c850a2defc6","bindings/nss_ssl.h":"af222fb957b989e392e762fa2125c82608a0053aff4fb97e556691646c88c335","bindings/nss_sslerr.h":"24b97f092183d8486f774cdaef5030d0249221c78343570d83a4ee5b594210ae","bindings/nss_sslopt.h":"b7807eb7abdad14db6ad7bc51048a46b065a0ea65a4508c95a12ce90e59d1eea","build.rs":"0be611e6b25a18d5ed724071a3a471c2c5b584649b5fe36de28a56ed8caaf800","src/aead.rs":"e26ad34f7168f42aa87eb3a6455af3191a0e8555782b1d70032fe1d248922f98","src/agent.rs":"9dae2fa87a5a7b65dfc7fdd1b2ab1be0f75f1d9ee6704b44edd9bd406252b10a","src/agentio.rs":"cc562d09a09719b90b4e1d147fd579e3e89b683448709e920033bceaea108a61","src/auth.rs":"71ac7e297a5f872d26cf67b6bbd96e4548ea38374bdd84c1094f76a5de4ed1cb","src/cert.rs":"fd3fd2bbb38754bdcee3898549feae412943c9f719032531c1ad6e61783b5394","src/constants.rs":"c39ee506a10d685fda77c1d2ddf691b595b067b4e1044ac7a21e360119d6002b","src/err.rs":"04f38831ca62d29d8aadfe9daf95fd29e68ece184e6d3e00bfb9ee1d12744033","src/exp.rs":"61586662407359c1ecb8ed4987bc3c702f26ba2e203a091a51b6d6363cbd510f","src/ext.rs":"97cba23247e5f9656f27587214f7d7370a69174bae5960a012ce3e6fc99f9116","src/hkdf.rs":"40e44f4280497ef525c2b4c465f14f06d241150851668b264ee958f74321cfbe","src/hp.rs":"7fce64e0cc3a6a7e744bc797886bcfaa39679f0a81853b2e55ea0f54fb6bf700","src/lib.rs":"3b22108a069c8c9f1b78e94d48c8759b17e0941b28e2def3fa343d6acace4b6d","src/once.rs":"b9850384899a1a016e839743d3489c0d4d916e1973746ef8c89872105d7d9736","src/p11.rs":"0b62ee5938aefb82e8faee5aa14e990a00442cc9744e8ba22eda80b32030c42c","src/prio.rs":"bc4e97049563b136cb7b39f5171e7909d56a77ed46690aaacb781eeb4a4743e0","src/replay.rs":"40924865994396441a68e6009ecbdf352d6a02fdf539aa65604124e26bffb4d3","src/result.rs":"cef34dfcb907723e195b56501132e4560e250b327783cb5e41201da5b63e9b5c","src/secrets.rs":"acb5befa74e06281c6f80d7298efc58f568bb4e6d949b4225c335e3f392be741","src/selfencrypt.rs":"429cb889a4e9e2345888cc033115c0aa306d2ff90bdfe22b3067700eb1426c37","src/ssl.rs":"3e3a4f539f3c4d18bd6e774dc34fca611db0c75bba00badcd2078c975db055bf","src/time.rs":"5b2ab4028b04b6245c666f33f1c1449816d3d1eb8141f723f5773f21f8fe4388","tests/aead.rs":"a1d8eb69f5672e064f84dce3d214b347a396718e3de56d57ccc108ee87f1cbc1","tests/agent.rs":"d43e5b05dcc845394d3c7312974faae0fdcbc325c07c970aeb7ef30c3ade652e","tests/ext.rs":"eba9f03accdd598e38292ac88263a81b367d60d5a736a43117a3663de105ec48","tests/handshake.rs":"93c478fcd07d29691007abd6dcfcd2014c10c23b0206ba2d97d01594e4d64397","tests/hkdf.rs":"539235e9dcf2a56b72961a9a04f0080409adf6bf465bfad7c30026421b2d4326","tests/hp.rs":"e52a7d2f4387f2dfe8bfe1da5867e8e0d3eb51e171c6904e18b18c4343536af8","tests/init.rs":"20aad800ac793aaf83059cf860593750509fdedeeff0c08a648e7a5cb398dae0","tests/selfencrypt.rs":"46e9a1a09c2ae577eb106d23a5cdacf762575c0dea1948aedab06ef7389ce713"},"package":null} \ No newline at end of file diff --git a/third_party/rust/neqo-crypto/Cargo.toml b/third_party/rust/neqo-crypto/Cargo.toml index bfbfccc0aee3..e490bdcd433c 100644 --- a/third_party/rust/neqo-crypto/Cargo.toml +++ b/third_party/rust/neqo-crypto/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "neqo-crypto" -version = "0.4.13" +version = "0.4.14" authors = ["Martin Thomson "] edition = "2018" build = "build.rs" diff --git a/third_party/rust/neqo-http3/.cargo-checksum.json b/third_party/rust/neqo-http3/.cargo-checksum.json index 285d7f28f7ba..459ad683c1a9 100644 --- a/third_party/rust/neqo-http3/.cargo-checksum.json +++ b/third_party/rust/neqo-http3/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"a74f1a0889751a8c1af51d2ac7e36efeb777bb67d58acec7bb8be4d20e4a7b4c","src/client_events.rs":"003e292cab6d432f3c45d581ba061defd324c50312357a5b04dfd384cc657a6a","src/connection.rs":"8d8093da1ccb0c4033ee5a27bd6298f92dab53095c5a4c75c2e7c57f021db10d","src/connection_client.rs":"9d86430d6cfab33afe45ba4a49b4cd3fbb5987f1271a4ae95dbbbafb73f8904e","src/connection_server.rs":"66d5705b43c5ab0c7a98a3ec6c7e06ff9b6b055d4d038ce6dac9b2dbe2e04c9a","src/control_stream_local.rs":"03d6259599543da2154388d5e48efbc06271e079429d3d946278c4c58c0521c7","src/control_stream_remote.rs":"1dfac4956a7d6971e2cef2c83963d838e73aa3bf3286b7bde97099978c41d527","src/hframe.rs":"a9e973dedfd1174f885787a68a6dbe0e719d117e6689829997f9e00d60e4e2c9","src/lib.rs":"b29d3140536fcf26cbf0c00d597b30ece4cffbba0fadc3616fabb0968453297c","src/push_controller.rs":"744372679db12ab179e10cf196397d8f0c2f4085cd627e8fbde07889f918638c","src/push_stream.rs":"5044e2d5a8a7aa0711901342f532a952be407327f844613afdae4f8a5e14d21a","src/qlog.rs":"29c0e3c4c9571eb7fe905967edeb1c4bc236b1e35a0e0f11a4a847f1d246681d","src/recv_message.rs":"cf6194e089d24037db7aee23b9f9b79d28430df92aa7909872ad20e0d0e563da","src/send_message.rs":"7e3f5280eca9b007b3b189134ce6832d3996f10405d396790acbced06a72af81","src/server.rs":"57501621ab1a997d1980f17ecc762378362da7798a53290b2ba34e79748b449e","src/server_connection_events.rs":"0c2b8831ce9d254a15a3af24d155a119ca1d4a29dd6d287114bf0607efe93076","src/server_events.rs":"27f23dc49f649fb66113c5a71345d9af30e7de04f791d4e1928d32c66b47d3f1","src/settings.rs":"7c746291b8110a233502a42a36f7dbd8e72aaf9371129719ac14d7cdfd410152","src/stream_type_reader.rs":"aacb2e865f79b3ac55a887fd670f2286d8ffef94f7d8b3ecfa7e0cccbfa9ec04","tests/httpconn.rs":"a5cc230596a8843011167d367879edde664894044357f7099f5b492c9bb90f27"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"b1729ba7146f0586ec16c7faa3584f5f644d71bb908a08a692577d02e28466ef","src/client_events.rs":"87e7c323ec2ceb759da7d6f2bf24e774f6f2e3833f63454d09cc09f47fccfa8e","src/connection.rs":"ee7c9c2cd68b11dd8c75c406d9e651b16c4722879da22c4b2606ec0391af9faf","src/connection_client.rs":"7f3e5f167581d232c814ca1f0dc3f774d7688957c1c055a23f5142bf6bb93dd9","src/connection_server.rs":"b3f4d42a984f7093fca737c64bb49c569f8f95d1586a6c02d4dba58f290ce92e","src/control_stream_local.rs":"03d6259599543da2154388d5e48efbc06271e079429d3d946278c4c58c0521c7","src/control_stream_remote.rs":"1dfac4956a7d6971e2cef2c83963d838e73aa3bf3286b7bde97099978c41d527","src/hframe.rs":"3620c6d114e6d5b98447a2dd2a7cb6872562ebda2cc6de828177b745c8dcbf4d","src/lib.rs":"772c71a6edef7a8d7dbdd6c0a30f560f35fa731eefc3e469f0c78e8680871a61","src/push_controller.rs":"3a44843f3d56246198bcc7378914941a337ddac2c2d4c389e8d0628448452aa7","src/push_stream.rs":"5f3a5c6d72c0a8e4d1c1c5041125f7aff9a65950fa60c77f42cd4b35c768f585","src/qlog.rs":"29c0e3c4c9571eb7fe905967edeb1c4bc236b1e35a0e0f11a4a847f1d246681d","src/recv_message.rs":"bc428969b69619ab5d153d455370e6a57ce03b40773ef5db4f66b760242e94fd","src/send_message.rs":"7e3f5280eca9b007b3b189134ce6832d3996f10405d396790acbced06a72af81","src/server.rs":"f369e5cae36598bb0bfa06aa5da310a55ce3c785bcb16cfb27acf4b5e98d068b","src/server_connection_events.rs":"762ddb87f700abe91ae1ae78ebbb87a88da0c0a341748b0751b01099870e9985","src/server_events.rs":"e780daa0d19d9a5594eee73f7ff27d56151a5a2ea969f2b4dcc64253e9570dab","src/settings.rs":"127a51fa7857b870718baa14340b0461d86a67e59bf1a8cb42d7bae0240c0ef1","src/stream_type_reader.rs":"aacb2e865f79b3ac55a887fd670f2286d8ffef94f7d8b3ecfa7e0cccbfa9ec04","tests/httpconn.rs":"1a97a80f7abe11c6ba0bd9b41003be6b293049164daa21e907365d93b00a782f"},"package":null} \ No newline at end of file diff --git a/third_party/rust/neqo-http3/Cargo.toml b/third_party/rust/neqo-http3/Cargo.toml index b006bde0490b..26ef54ec5c83 100644 --- a/third_party/rust/neqo-http3/Cargo.toml +++ b/third_party/rust/neqo-http3/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "neqo-http3" -version = "0.4.13" +version = "0.4.14" authors = ["Dragana Damjanovic "] edition = "2018" license = "MIT/Apache-2.0" diff --git a/third_party/rust/neqo-http3/src/client_events.rs b/third_party/rust/neqo-http3/src/client_events.rs index 970d2af69e3d..dc7d86126f5b 100644 --- a/third_party/rust/neqo-http3/src/client_events.rs +++ b/third_party/rust/neqo-http3/src/client_events.rs @@ -24,15 +24,20 @@ pub enum Http3ClientEvent { /// Response headers are received. HeaderReady { stream_id: u64, - headers: Option>, + headers: Vec
, + interim: bool, fin: bool, }, /// A stream can accept new data. DataWritable { stream_id: u64 }, /// New bytes available for reading. DataReadable { stream_id: u64 }, - /// Peer reset the stream. - Reset { stream_id: u64, error: AppError }, + /// Peer reset the stream or there was an parsing error. + Reset { + stream_id: u64, + error: AppError, + local: bool, + }, /// Peer has sent a STOP_SENDING. StopSending { stream_id: u64, error: AppError }, /// A new push promise. @@ -44,13 +49,17 @@ pub enum Http3ClientEvent { /// A push response headers are ready. PushHeaderReady { push_id: u64, - headers: Option>, + headers: Vec
, + interim: bool, fin: bool, }, /// New bytes are available on a push stream for reading. PushDataReadable { push_id: u64 }, /// A push has been canceled. PushCanceled { push_id: u64 }, + /// A push stream was been reset due to a HttpGeneralProtocol error. + /// Most common dase are malformed response headers. + PushReset { push_id: u64, error: AppError }, /// New stream can be created RequestsCreatable, /// Cert authentication needed @@ -72,10 +81,11 @@ pub struct Http3ClientEvents { impl RecvMessageEvents for Http3ClientEvents { /// Add a new `HeaderReady` event. - fn header_ready(&self, stream_id: u64, headers: Option>, fin: bool) { + fn header_ready(&self, stream_id: u64, headers: Vec
, interim: bool, fin: bool) { self.insert(Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, }); } @@ -86,7 +96,7 @@ impl RecvMessageEvents for Http3ClientEvents { } /// Add a new `Reset` event. - fn reset(&self, stream_id: u64, error: AppError) { + fn reset(&self, stream_id: u64, error: AppError, local: bool) { self.remove(|evt| { matches!(evt, Http3ClientEvent::HeaderReady { stream_id: x, .. } @@ -94,7 +104,11 @@ impl RecvMessageEvents for Http3ClientEvents { | Http3ClientEvent::PushPromise { request_stream_id: x, .. } | Http3ClientEvent::Reset { stream_id: x, .. } if *x == stream_id) }); - self.insert(Http3ClientEvent::Reset { stream_id, error }); + self.insert(Http3ClientEvent::Reset { + stream_id, + error, + local, + }); } } @@ -134,6 +148,11 @@ impl Http3ClientEvents { self.insert(Http3ClientEvent::PushCanceled { push_id }); } + pub fn push_reset(&self, push_id: u64, error: AppError) { + self.remove_events_for_push_id(push_id); + self.insert(Http3ClientEvent::PushReset { push_id, error }); + } + /// Add a new `RequestCreatable` event pub(crate) fn new_requests_creatable(&self, stream_type: StreamType) { if stream_type == StreamType::BiDi { diff --git a/third_party/rust/neqo-http3/src/connection.rs b/third_party/rust/neqo-http3/src/connection.rs index b5c7a1792361..bdd40e936169 100644 --- a/third_party/rust/neqo-http3/src/connection.rs +++ b/third_party/rust/neqo-http3/src/connection.rs @@ -12,7 +12,7 @@ use crate::hframe::HFrame; use crate::send_message::SendMessage; use crate::settings::{HSetting, HSettingType, HSettings, HttpZeroRttChecker}; use crate::stream_type_reader::NewStreamTypeReader; -use crate::RecvStream; +use crate::{RecvStream, ResetType}; use neqo_common::{qdebug, qerror, qinfo, qtrace, qwarn}; use neqo_qpack::decoder::{QPackDecoder, QPACK_UNI_STREAM_TYPE_DECODER}; use neqo_qpack::encoder::{QPackEncoder, QPACK_UNI_STREAM_TYPE_ENCODER}; @@ -354,7 +354,7 @@ impl Http3Connection { ); if let Some(s) = self.recv_streams.remove(&stream_id) { - s.stream_reset_recv(app_error, &mut self.qpack_decoder); + s.stream_reset(app_error, &mut self.qpack_decoder, ResetType::Remote); Ok(()) } else if self.is_critical_stream(stream_id) { Err(Error::HttpClosedCriticalStream) @@ -530,7 +530,7 @@ impl Http3Connection { let mut found = self.send_streams.remove(&stream_id).is_some(); if let Some(s) = self.recv_streams.remove(&stream_id) { - s.stream_reset(&mut self.qpack_decoder); + s.stream_reset(error, &mut self.qpack_decoder, ResetType::App); found = true; } diff --git a/third_party/rust/neqo-http3/src/connection_client.rs b/third_party/rust/neqo-http3/src/connection_client.rs index 53bbc4afadd0..3f5824abeec0 100644 --- a/third_party/rust/neqo-http3/src/connection_client.rs +++ b/third_party/rust/neqo-http3/src/connection_client.rs @@ -9,11 +9,10 @@ use crate::connection::{HandleReadableOutput, Http3Connection, Http3State}; use crate::hframe::HFrame; use crate::push_controller::PushController; use crate::push_stream::PushStream; -use crate::recv_message::RecvMessage; +use crate::recv_message::{MessageType, RecvMessage}; use crate::send_message::{SendMessage, SendMessageEvents}; use crate::settings::HSettings; -use crate::Header; -use crate::RecvMessageEvents; +use crate::{Header, RecvMessageEvents, ResetType}; use neqo_common::{ event::Provider as EventProvider, hex, hex_with_len, qdebug, qinfo, qlog::NeqoQlog, qtrace, Datagram, Decoder, Encoder, Role, @@ -289,6 +288,7 @@ impl Http3Client { id, SendMessage::new_with_headers(id, final_headers, Box::new(self.events.clone())), Box::new(RecvMessage::new( + MessageType::Response, id, Box::new(self.events.clone()), Some(self.push_handler.clone()), @@ -373,7 +373,8 @@ impl Http3Client { .get_mut(&stream_id) .ok_or(Error::InvalidStreamId)?; - match recv_stream.read_data(&mut self.conn, &mut self.base_handler.qpack_decoder, buf) { + let res = recv_stream.read_data(&mut self.conn, &mut self.base_handler.qpack_decoder, buf); + match res { Ok((amount, fin)) => { if recv_stream.done() { self.base_handler.recv_streams.remove(&stream_id); @@ -381,10 +382,15 @@ impl Http3Client { Ok((amount, fin)) } Err(e) => { - if e.connection_error() { + if e.stream_reset_error() { + self.reset_stream_on_error(stream_id, e.code()); + Ok((0, false)) + } else if e.connection_error() { self.close(now, e.code(), ""); + Err(e) + } else { + Err(e) } - Err(e) } } } @@ -525,7 +531,13 @@ impl Http3Client { } } ConnectionEvent::RecvStreamReadable { stream_id } => { - self.handle_stream_readable(stream_id)? + if let Err(e) = self.handle_stream_readable(stream_id) { + if e.stream_reset_error() { + self.reset_stream_on_error(stream_id, e.code()); + } else { + return Err(e); + } + } } ConnectionEvent::RecvStreamReset { stream_id, @@ -646,7 +658,8 @@ impl Http3Client { .iter() .filter_map(id_gte(goaway_stream_id)) { - self.events.reset(id, Error::HttpRequestRejected.code()); + self.events + .reset(id, Error::HttpRequestRejected.code(), false); } for id in self @@ -682,6 +695,17 @@ impl Http3Client { pub fn qpack_encoder_stats(&self) -> &Stats { self.base_handler.qpack_encoder.stats() } + + fn reset_stream_on_error(&mut self, stream_id: u64, app_error: AppError) { + let _ = self.conn.stream_stop_sending(stream_id, app_error); + if let Some(rs) = self.base_handler.recv_streams.remove(&stream_id) { + rs.stream_reset( + app_error, + &mut self.base_handler.qpack_decoder, + ResetType::Local, + ); + } + } } impl EventProvider for Http3Client { @@ -996,6 +1020,22 @@ mod tests { assert_eq!(amount, expected_data.len()); assert_eq!(&buf[..amount], expected_data); } + + pub fn encode_headers( + &mut self, + stream_id: u64, + headers: &[Header], + encoder: &mut Encoder, + ) { + let header_block = self + .encoder + .encode_header_block(&mut self.conn, &headers, stream_id) + .unwrap(); + let hframe = HFrame::Headers { + header_block: header_block.to_vec(), + }; + hframe.encode(encoder); + } } // Perform only Quic transport handshake. @@ -1312,16 +1352,46 @@ mod tests { // 2) push_id // 3) PUSH_DATA that contains encoded headers and a data frame. // This function can only handle small push_id numbers that fit in a varint of length 1 byte. - fn send_push_data(conn: &mut Connection, push_id: u8, close_push_stream: bool) -> u64 { - // create a push stream. - let push_stream_id = conn.stream_create(StreamType::UniDi).unwrap(); + fn send_data_on_push( + conn: &mut Connection, + push_stream_id: u64, + push_id: u8, + data: &[u8], + close_push_stream: bool, + ) { // send data let _ = conn.stream_send(push_stream_id, PUSH_STREAM_TYPE).unwrap(); let _ = conn.stream_send(push_stream_id, &[push_id]).unwrap(); - let _ = conn.stream_send(push_stream_id, PUSH_DATA).unwrap(); + let _ = conn.stream_send(push_stream_id, data).unwrap(); if close_push_stream { conn.stream_close_send(push_stream_id).unwrap(); } + } + + // Send push data on a push stream: + // 1) push_stream_type PUSH_STREAM_TYPE + // 2) push_id + // 3) PUSH_DATA that contains encoded headers and a data frame. + // This function can only handle small push_id numbers that fit in a varint of length 1 byte. + fn send_push_data(conn: &mut Connection, push_id: u8, close_push_stream: bool) -> u64 { + send_push_with_data(conn, push_id, PUSH_DATA, close_push_stream) + } + + // Send push data on a push stream: + // 1) push_stream_type PUSH_STREAM_TYPE + // 2) push_id + // 3) and supplied push data. + // This function can only handle small push_id numbers that fit in a varint of length 1 byte. + fn send_push_with_data( + conn: &mut Connection, + push_id: u8, + data: &[u8], + close_push_stream: bool, + ) -> u64 { + // create a push stream + let push_stream_id = conn.stream_create(StreamType::UniDi).unwrap(); + // send data + send_data_on_push(conn, push_stream_id, push_id, data, close_push_stream); push_stream_id } @@ -1361,12 +1431,14 @@ mod tests { Http3ClientEvent::PushHeaderReady { push_id, headers, + interim, fin, } => { assert!(push_streams.contains(&push_id)); - check_push_response_header(&headers.unwrap()); + check_push_response_header(&headers); num_push_stream_headers += 1; assert_eq!(fin, false); + assert_eq!(interim, false); } Http3ClientEvent::PushDataReadable { push_id } => { assert!(push_streams.contains(&push_id)); @@ -1380,11 +1452,13 @@ mod tests { Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } => { assert_eq!(stream_id, response_stream_id); - check_response_header_2(&headers.unwrap()); + check_response_header_2(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); } Http3ClientEvent::DataReadable { stream_id } => { assert_eq!(stream_id, response_stream_id); @@ -1801,11 +1875,13 @@ mod tests { Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } => { assert_eq!(stream_id, request_stream_id); - check_response_header_1(&headers.unwrap()); + check_response_header_1(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); } Http3ClientEvent::DataReadable { stream_id } => { assert_eq!(stream_id, request_stream_id); @@ -1840,11 +1916,13 @@ mod tests { Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } => { assert_eq!(stream_id, request_stream_id); - check_response_header_2(&headers.unwrap()); + check_response_header_2(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); } Http3ClientEvent::DataReadable { stream_id } => { assert_eq!(stream_id, request_stream_id); @@ -2190,11 +2268,13 @@ mod tests { Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } => { assert_eq!(stream_id, request_stream_id); - check_response_header_2(&headers.unwrap()); + check_response_header_2(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); response_headers = true; } Http3ClientEvent::DataReadable { stream_id } => { @@ -2258,9 +2338,14 @@ mod tests { assert_eq!(error, Error::HttpRequestRejected.code()); stop_sending = true; } - Http3ClientEvent::Reset { stream_id, error } => { + Http3ClientEvent::Reset { + stream_id, + error, + local, + } => { assert_eq!(stream_id, request_stream_id); assert_eq!(error, Error::HttpRequestRejected.code()); + assert_eq!(local, false); reset = true; } Http3ClientEvent::HeaderReady { .. } | Http3ClientEvent::DataReadable { .. } => { @@ -2372,9 +2457,14 @@ mod tests { assert_eq!(stream_id, request_stream_id); assert_eq!(error, Error::HttpRequestCancelled.code()); } - Http3ClientEvent::Reset { stream_id, error } => { + Http3ClientEvent::Reset { + stream_id, + error, + local, + } => { assert_eq!(stream_id, request_stream_id); assert_eq!(error, Error::HttpRequestCancelled.code()); + assert_eq!(local, false); reset = true; } Http3ClientEvent::HeaderReady { .. } | Http3ClientEvent::DataReadable { .. } => { @@ -2485,9 +2575,14 @@ mod tests { Http3ClientEvent::StopSending { .. } => { panic!("We should not get StopSending."); } - Http3ClientEvent::Reset { stream_id, error } => { + Http3ClientEvent::Reset { + stream_id, + error, + local, + } => { assert_eq!(stream_id, request_stream_id); assert_eq!(error, Error::HttpRequestCancelled.code()); + assert_eq!(local, false); reset = true; } Http3ClientEvent::HeaderReady { .. } | Http3ClientEvent::DataReadable { .. } => { @@ -2588,7 +2683,7 @@ mod tests { while let Some(e) = client.next_event() { match e { Http3ClientEvent::HeaderReady { headers, fin, .. } => { - check_response_header_1(&headers.unwrap()); + check_response_header_1(&headers); assert_eq!(fin, false); } Http3ClientEvent::DataReadable { stream_id } => { @@ -2603,9 +2698,14 @@ mod tests { .unwrap() ); } - Http3ClientEvent::Reset { stream_id, error } => { + Http3ClientEvent::Reset { + stream_id, + error, + local, + } => { assert_eq!(stream_id, request_stream_id_3); assert_eq!(error, Error::HttpRequestRejected.code()); + assert_eq!(local, false); stream_reset = true; } _ => {} @@ -2648,9 +2748,15 @@ mod tests { // Check that there is one reset for stream_id 8 let mut stream_reset_1 = 0; while let Some(e) = client.next_event() { - if let Http3ClientEvent::Reset { stream_id, error } = e { + if let Http3ClientEvent::Reset { + stream_id, + error, + local, + } = e + { assert_eq!(stream_id, request_stream_id_3); assert_eq!(error, Error::HttpRequestRejected.code()); + assert_eq!(local, false); stream_reset_1 += 1; } } @@ -2676,7 +2782,7 @@ mod tests { while let Some(e) = client.next_event() { match e { Http3ClientEvent::HeaderReady { headers, fin, .. } => { - check_response_header_1(&headers.unwrap()); + check_response_header_1(&headers); assert_eq!(fin, false); } Http3ClientEvent::DataReadable { stream_id } => { @@ -2689,9 +2795,14 @@ mod tests { .unwrap() ); } - Http3ClientEvent::Reset { stream_id, error } => { + Http3ClientEvent::Reset { + stream_id, + error, + local, + } => { assert_eq!(stream_id, request_stream_id_2); assert_eq!(error, Error::HttpRequestRejected.code()); + assert_eq!(local, false); stream_reset_2 += 1; } _ => {} @@ -2759,18 +2870,14 @@ mod tests { // Recv HeaderReady wo headers with fin. let e = client.events().next().unwrap(); - if let Http3ClientEvent::HeaderReady { - stream_id, - headers, - fin, - } = e - { - assert_eq!(stream_id, request_stream_id); - assert_eq!(headers, None); - assert_eq!(fin, true); - } else { - panic!("wrong event type"); - } + assert_eq!( + e, + Http3ClientEvent::Reset { + stream_id: request_stream_id, + error: Error::HttpGeneralProtocolStream.code(), + local: true, + } + ); // Stream should now be closed and gone let mut buf = [0_u8; 100]; @@ -2798,12 +2905,14 @@ mod tests { if let Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } = e { assert_eq!(stream_id, request_stream_id); - check_response_header_2(&headers.unwrap()); + check_response_header_2(&headers); assert_eq!(fin, true); + assert_eq!(interim, false); } else { panic!("wrong event type"); } @@ -2836,11 +2945,13 @@ mod tests { Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } => { assert_eq!(stream_id, request_stream_id); - check_response_header_2(&headers.unwrap()); + check_response_header_2(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); } Http3ClientEvent::DataReadable { .. } => { panic!("We should not receive a DataGeadable event!"); @@ -2904,11 +3015,13 @@ mod tests { Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } => { assert_eq!(stream_id, request_stream_id); - check_response_header_2(&headers.unwrap()); + check_response_header_2(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); } Http3ClientEvent::DataReadable { stream_id } => { assert_eq!(stream_id, request_stream_id); @@ -2952,11 +3065,13 @@ mod tests { Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } => { assert_eq!(stream_id, request_stream_id); - check_response_header_2(&headers.unwrap()); + check_response_header_2(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); } Http3ClientEvent::DataReadable { .. } => { panic!("We should not receive a DataGeadable event!"); @@ -3015,11 +3130,13 @@ mod tests { Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } => { assert_eq!(stream_id, request_stream_id); - check_response_header_2(&headers.unwrap()); + check_response_header_2(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); } Http3ClientEvent::DataReadable { stream_id } => { assert_eq!(stream_id, request_stream_id); @@ -3260,12 +3377,14 @@ mod tests { if let Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } = e { assert_eq!(stream_id, request_stream_id); - assert_eq!(headers.unwrap(), sent_headers); + assert_eq!(headers, sent_headers); assert_eq!(fin, true); + assert_eq!(interim, false); recv_header = true; } else { panic!("event {:?}", e); @@ -3792,12 +3911,14 @@ mod tests { if let Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } = e { assert_eq!(stream_id, request_stream_id); - check_response_header_0(&headers.unwrap()); + check_response_header_0(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); response_headers = true; } } @@ -3851,12 +3972,14 @@ mod tests { if let Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } = e { assert_eq!(stream_id, request_stream_id); - check_response_header_0(&headers.unwrap()); + check_response_header_0(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); response_headers = true; } } @@ -3919,12 +4042,14 @@ mod tests { if let Http3ClientEvent::HeaderReady { stream_id, headers, + interim, fin, } = e { assert_eq!(stream_id, request_stream_id); - check_response_header_0(&headers.unwrap()); + check_response_header_0(&headers); assert_eq!(fin, false); + assert_eq!(interim, false); response_headers = true; } } @@ -5562,4 +5687,245 @@ mod tests { assert_closed(&client, &Error::HttpSettings); } } + + #[test] + fn response_w_1xx() { + let (mut client, mut server, request_stream_id) = connect_and_send_request(true); + + setup_server_side_encoder(&mut client, &mut server); + + let mut d = Encoder::default(); + let headers1xx = vec![(String::from(":status"), String::from("103"))]; + server.encode_headers(request_stream_id, &headers1xx, &mut d); + + let headers200 = vec![ + (String::from(":status"), String::from("200")), + (String::from("my-header"), String::from("my-header")), + (String::from("content-length"), String::from("3")), + ]; + server.encode_headers(request_stream_id, &headers200, &mut d); + + // Send 1xx and 200 headers response. + server_send_response_and_exchange_packet( + &mut client, + &mut server, + request_stream_id, + &d, + false, + ); + + // Sending response data. + server_send_response_and_exchange_packet( + &mut client, + &mut server, + request_stream_id, + HTTP_RESPONSE_DATA_FRAME_ONLY_2, + true, + ); + + let mut events = client.events().filter_map(|e| { + if let Http3ClientEvent::HeaderReady { + stream_id, + interim, + headers, + .. + } = e + { + Some((stream_id, interim, headers)) + } else { + None + } + }); + let (stream_id_1xx_rec, interim1xx_rec, headers1xx_rec) = events.next().unwrap(); + assert_eq!( + (stream_id_1xx_rec, interim1xx_rec, headers1xx_rec), + (request_stream_id, true, headers1xx) + ); + + let (stream_id_200_rec, interim200_rec, headers200_rec) = events.next().unwrap(); + assert_eq!( + (stream_id_200_rec, interim200_rec, headers200_rec), + (request_stream_id, false, headers200) + ); + assert!(events.next().is_none()); + } + + #[test] + fn response_wo_status() { + let (mut client, mut server, request_stream_id) = connect_and_send_request(true); + + setup_server_side_encoder(&mut client, &mut server); + + let mut d = Encoder::default(); + let headers = vec![ + (String::from("my-header"), String::from("my-header")), + (String::from("content-length"), String::from("3")), + ]; + server.encode_headers(request_stream_id, &headers, &mut d); + + // Send response + server_send_response_and_exchange_packet( + &mut client, + &mut server, + request_stream_id, + &d, + false, + ); + + // Stream has been reset because of the malformed headers. + let e = client.events().next().unwrap(); + assert_eq!( + e, + Http3ClientEvent::Reset { + stream_id: request_stream_id, + error: Error::HttpGeneralProtocolStream.code(), + local: true, + } + ); + + let out = client.process(None, now()).dgram(); + let _ = server.conn.process(out, now()); + + // Check that server has received a reset. + let stop_sending_event = |e| { + matches!(e, ConnectionEvent::SendStreamStopSending { + stream_id, + app_error + } if stream_id == request_stream_id && app_error == Error::HttpGeneralProtocolStream.code()) + }; + assert!(server.conn.events().any(stop_sending_event)); + + // Stream should now be closed and gone + let mut buf = [0_u8; 100]; + assert_eq!( + client.read_response_data(now(), 0, &mut buf), + Err(Error::InvalidStreamId) + ); + } + + // Client: receive a push stream + #[test] + fn push_single_with_1xx() { + const FIRST_PUSH_ID: u64 = 0; + // Connect and send a request + let (mut client, mut server, request_stream_id) = connect_and_send_request(true); + + // Send a push promise. + send_push_promise(&mut server.conn, request_stream_id, FIRST_PUSH_ID); + // Create a push stream + let push_stream_id = server.conn.stream_create(StreamType::UniDi).unwrap(); + + let mut d = Encoder::default(); + let headers1xx = vec![(String::from(":status"), String::from("101"))]; + server.encode_headers(push_stream_id, &headers1xx, &mut d); + + let headers200 = vec![ + (String::from(":status"), String::from("200")), + (String::from("my-header"), String::from("my-header")), + (String::from("content-length"), String::from("3")), + ]; + server.encode_headers(push_stream_id, &headers200, &mut d); + + // create a push stream. + send_data_on_push( + &mut server.conn, + push_stream_id, + u8::try_from(FIRST_PUSH_ID).unwrap(), + &d, + true, + ); + + server_send_response_and_exchange_packet( + &mut client, + &mut server, + request_stream_id, + HTTP_RESPONSE_2, + true, + ); + + let mut events = client.events().filter_map(|e| { + if let Http3ClientEvent::PushHeaderReady { + push_id, + interim, + headers, + .. + } = e + { + Some((push_id, interim, headers)) + } else { + None + } + }); + + let (push_id_1xx_rec, interim1xx_rec, headers1xx_rec) = events.next().unwrap(); + assert_eq!( + (push_id_1xx_rec, interim1xx_rec, headers1xx_rec), + (FIRST_PUSH_ID, true, headers1xx) + ); + + let (push_id_200_rec, interim200_rec, headers200_rec) = events.next().unwrap(); + assert_eq!( + (push_id_200_rec, interim200_rec, headers200_rec), + (FIRST_PUSH_ID, false, headers200) + ); + assert!(events.next().is_none()); + } + + // Client: receive a push stream + #[test] + fn push_single_wo_status() { + const FIRST_PUSH_ID: u64 = 0; + // Connect and send a request + let (mut client, mut server, request_stream_id) = connect_and_send_request(true); + + // Send a push promise. + send_push_promise(&mut server.conn, request_stream_id, FIRST_PUSH_ID); + // Create a push stream + let push_stream_id = server.conn.stream_create(StreamType::UniDi).unwrap(); + + let mut d = Encoder::default(); + let headers = vec![ + (String::from("my-header"), String::from("my-header")), + (String::from("content-length"), String::from("3")), + ]; + server.encode_headers(request_stream_id, &headers, &mut d); + + send_data_on_push( + &mut server.conn, + push_stream_id, + u8::try_from(FIRST_PUSH_ID).unwrap(), + &d, + false, + ); + + server_send_response_and_exchange_packet( + &mut client, + &mut server, + request_stream_id, + HTTP_RESPONSE_2, + true, + ); + + // Stream has been reset because of thei malformed headers. + let push_reset_event = |e| { + matches!(e, Http3ClientEvent::PushReset { + push_id, + error, + } if push_id == FIRST_PUSH_ID && error == Error::HttpGeneralProtocol.code()) + }; + + assert!(client.events().any(push_reset_event)); + + let out = client.process(None, now()).dgram(); + let _ = server.conn.process(out, now()); + + // Check that server has received a reset. + let stop_sending_event = |e| { + matches!(e, ConnectionEvent::SendStreamStopSending { + stream_id, + app_error + } if stream_id == push_stream_id && app_error == Error::HttpGeneralProtocolStream.code()) + }; + assert!(server.conn.events().any(stop_sending_event)); + } } diff --git a/third_party/rust/neqo-http3/src/connection_server.rs b/third_party/rust/neqo-http3/src/connection_server.rs index 41103520d377..f0b18dd14c79 100644 --- a/third_party/rust/neqo-http3/src/connection_server.rs +++ b/third_party/rust/neqo-http3/src/connection_server.rs @@ -6,7 +6,7 @@ use crate::connection::{HandleReadableOutput, Http3Connection, Http3State}; use crate::hframe::HFrame; -use crate::recv_message::RecvMessage; +use crate::recv_message::{MessageType, RecvMessage}; use crate::send_message::SendMessage; use crate::server_connection_events::{Http3ServerConnEvent, Http3ServerConnEvents}; use crate::{Error, Header, Res}; @@ -126,6 +126,7 @@ impl Http3ServerHandler { stream_id.as_u64(), SendMessage::new(stream_id.as_u64(), Box::new(self.events.clone())), Box::new(RecvMessage::new( + MessageType::Request, stream_id.as_u64(), Box::new(self.events.clone()), None, diff --git a/third_party/rust/neqo-http3/src/hframe.rs b/third_party/rust/neqo-http3/src/hframe.rs index e302caf7fd9c..f8043527e726 100644 --- a/third_party/rust/neqo-http3/src/hframe.rs +++ b/third_party/rust/neqo-http3/src/hframe.rs @@ -31,7 +31,7 @@ pub const H3_RESERVED_FRAME_TYPES: &[HFrameType] = &[0x2, 0x6, 0x8, 0x9]; const MAX_READ_SIZE: usize = 4096; // data for DATA frame is not read into HFrame::Data. #[derive(PartialEq, Debug)] -pub(crate) enum HFrame { +pub enum HFrame { Data { len: u64, // length of the data }, @@ -129,7 +129,7 @@ enum HFrameReaderState { } #[derive(Debug)] -pub(crate) struct HFrameReader { +pub struct HFrameReader { state: HFrameReaderState, hframe_type: u64, hframe_len: u64, diff --git a/third_party/rust/neqo-http3/src/lib.rs b/third_party/rust/neqo-http3/src/lib.rs index 2c94902146e0..ec9530e7bb0c 100644 --- a/third_party/rust/neqo-http3/src/lib.rs +++ b/third_party/rust/neqo-http3/src/lib.rs @@ -36,6 +36,7 @@ pub use client_events::Http3ClientEvent; pub use connection::Http3State; pub use connection_client::Http3Client; pub use connection_client::Http3Parameters; +pub use hframe::HFrameReader; pub use neqo_qpack::Header; pub use server::Http3Server; pub use server_events::Http3ServerEvent; @@ -46,6 +47,7 @@ type Res = Result; pub enum Error { HttpNoError, HttpGeneralProtocol, + HttpGeneralProtocolStream, //this is the same as the above but it should only close a stream not a connection. HttpInternal, HttpStreamCreation, HttpClosedCriticalStream, @@ -87,7 +89,7 @@ impl Error { pub fn code(&self) -> AppError { match self { Self::HttpNoError => 0x100, - Self::HttpGeneralProtocol => 0x101, + Self::HttpGeneralProtocol | Self::HttpGeneralProtocolStream => 0x101, Self::HttpInternal => 0x102, Self::HttpStreamCreation => 0x103, Self::HttpClosedCriticalStream => 0x104, @@ -127,6 +129,11 @@ impl Error { } } + #[must_use] + pub fn stream_reset_error(&self) -> bool { + matches!(self, Self::HttpGeneralProtocolStream) + } + #[must_use] pub fn map_stream_send_errors(err: &TransportError) -> Self { match err { @@ -244,8 +251,7 @@ impl ::std::fmt::Display for Error { } pub trait RecvStream: Debug { - fn stream_reset_recv(&self, app_error: AppError, decoder: &mut QPackDecoder); - fn stream_reset(&self, decoder: &mut QPackDecoder); + fn stream_reset(&self, error: AppError, decoder: &mut QPackDecoder, reset_type: ResetType); /// # Errors /// An error may happen while reading a stream, e.g. early close, protocol error, etc. fn receive(&mut self, conn: &mut Connection, decoder: &mut QPackDecoder) -> Res<()>; @@ -264,7 +270,14 @@ pub trait RecvStream: Debug { } pub(crate) trait RecvMessageEvents: Debug { - fn header_ready(&self, stream_id: u64, headers: Option>, fin: bool); + fn header_ready(&self, stream_id: u64, headers: Vec
, interim: bool, fin: bool); fn data_readable(&self, stream_id: u64); - fn reset(&self, stream_id: u64, error: AppError); + fn reset(&self, stream_id: u64, error: AppError, local: bool); +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ResetType { + App, + Remote, + Local, } diff --git a/third_party/rust/neqo-http3/src/push_controller.rs b/third_party/rust/neqo-http3/src/push_controller.rs index 65bb3cb32d42..4dfef1f9f7d8 100644 --- a/third_party/rust/neqo-http3/src/push_controller.rs +++ b/third_party/rust/neqo-http3/src/push_controller.rs @@ -6,8 +6,8 @@ use crate::client_events::{Http3ClientEvent, Http3ClientEvents}; use crate::connection::Http3Connection; use crate::hframe::HFrame; -use crate::RecvMessageEvents; use crate::{Error, Header, Res}; +use crate::{RecvMessageEvents, ResetType}; use neqo_common::{qerror, qinfo, qtrace}; use neqo_transport::{AppError, Connection}; use std::cell::RefCell; @@ -369,7 +369,7 @@ impl PushController { } } - pub fn push_stream_reset(&mut self, push_id: u64) { + pub fn push_stream_reset(&mut self, push_id: u64, app_error: AppError, reset_type: ResetType) { qtrace!("Push stream has been reset, push_id={}", push_id); if let Some(push_state) = self.push_streams.get(push_id) { @@ -380,7 +380,11 @@ impl PushController { PushState::Active { .. } => { self.push_streams.close(push_id); self.conn_events.remove_events_for_push_id(push_id); - self.conn_events.push_canceled(push_id); + if reset_type == ResetType::Local { + self.conn_events.push_reset(push_id, app_error); + } else { + self.conn_events.push_canceled(push_id); + } } _ => { debug_assert!( @@ -457,12 +461,13 @@ impl RecvPushEvents { } impl RecvMessageEvents for RecvPushEvents { - fn header_ready(&self, _stream_id: u64, headers: Option>, fin: bool) { + fn header_ready(&self, _stream_id: u64, headers: Vec
, interim: bool, fin: bool) { self.push_handler.borrow_mut().new_stream_event( self.push_id, Http3ClientEvent::PushHeaderReady { push_id: self.push_id, headers, + interim, fin, }, ); @@ -477,5 +482,5 @@ impl RecvMessageEvents for RecvPushEvents { ); } - fn reset(&self, _stream_id: u64, _error: AppError) {} + fn reset(&self, _stream_id: u64, _error: AppError, _local: bool) {} } diff --git a/third_party/rust/neqo-http3/src/push_stream.rs b/third_party/rust/neqo-http3/src/push_stream.rs index 58d0582f426d..7bcce22a01e1 100644 --- a/third_party/rust/neqo-http3/src/push_stream.rs +++ b/third_party/rust/neqo-http3/src/push_stream.rs @@ -6,9 +6,9 @@ use crate::client_events::Http3ClientEvents; use crate::push_controller::{PushController, RecvPushEvents}; -use crate::recv_message::RecvMessage; +use crate::recv_message::{MessageType, RecvMessage}; use crate::stream_type_reader::NewStreamTypeReader; -use crate::{Error, RecvStream, Res}; +use crate::{Error, RecvStream, Res, ResetType}; use neqo_qpack::decoder::QPackDecoder; use neqo_transport::{AppError, Connection}; use std::cell::RefCell; @@ -104,6 +104,7 @@ impl RecvStream for PushStream { self.state = PushStreamState::ReadResponse { push_id: p, response: RecvMessage::new( + MessageType::Response, self.stream_id, Box::new(RecvPushEvents::new(p, self.push_handler.clone())), None, @@ -139,18 +140,19 @@ impl RecvStream for PushStream { matches!(self.state, PushStreamState::Closed) } - fn stream_reset_recv(&self, _app_error: AppError, decoder: &mut QPackDecoder) { + fn stream_reset(&self, app_error: AppError, decoder: &mut QPackDecoder, reset_type: ResetType) { if !self.done() { decoder.cancel_stream(self.stream_id); } - if let Some(push_id) = self.state.push_id() { - self.push_handler.borrow_mut().push_stream_reset(push_id); - } - } - - fn stream_reset(&self, decoder: &mut QPackDecoder) { - if !self.done() { - decoder.cancel_stream(self.stream_id); + match reset_type { + ResetType::App => {} + t => { + if let Some(push_id) = self.state.push_id() { + self.push_handler + .borrow_mut() + .push_stream_reset(push_id, app_error, t); + } + } } } diff --git a/third_party/rust/neqo-http3/src/recv_message.rs b/third_party/rust/neqo-http3/src/recv_message.rs index 894de6bed2ff..512238c8b509 100644 --- a/third_party/rust/neqo-http3/src/recv_message.rs +++ b/third_party/rust/neqo-http3/src/recv_message.rs @@ -7,9 +7,8 @@ use crate::hframe::{HFrame, HFrameReader}; use crate::push_controller::PushController; use crate::qlog; -use crate::RecvMessageEvents; -use crate::RecvStream; use crate::{Error, Header, Res}; +use crate::{RecvMessageEvents, RecvStream, ResetType}; use neqo_common::{qdebug, qinfo, qtrace}; use neqo_qpack::decoder::QPackDecoder; @@ -21,6 +20,12 @@ use std::convert::TryFrom; use std::fmt::Debug; use std::rc::Rc; +#[derive(Debug)] +pub enum MessageType { + Request, + Response, +} + /* * Response stream state: * WaitingForResponseHeaders : we wait for headers. in this state we can @@ -57,6 +62,7 @@ struct PushInfo { #[derive(Debug)] pub(crate) struct RecvMessage { state: RecvMessageState, + message_type: MessageType, conn_events: Box, push_handler: Option>>, stream_id: u64, @@ -71,6 +77,7 @@ impl ::std::fmt::Display for RecvMessage { impl RecvMessage { pub fn new( + message_type: MessageType, stream_id: u64, conn_events: Box, push_handler: Option>>, @@ -79,6 +86,7 @@ impl RecvMessage { state: RecvMessageState::WaitingForResponseHeaders { frame_reader: HFrameReader::new(), }, + message_type, conn_events, push_handler, stream_id, @@ -86,16 +94,11 @@ impl RecvMessage { } } - fn handle_headers_frame( - &mut self, - header_block: Vec, - fin: bool, - decoder: &mut QPackDecoder, - ) -> Res<()> { + fn handle_headers_frame(&mut self, header_block: Vec, fin: bool) -> Res<()> { match self.state { RecvMessageState::WaitingForResponseHeaders {..} => { if header_block.is_empty() { - self.add_headers(None, fin, decoder); + return Err(Error::HttpGeneralProtocolStream); } else { self.state = RecvMessageState::DecodingHeaders { header_block, fin }; } @@ -132,32 +135,45 @@ impl RecvMessage { Ok(()) } - fn add_headers(&mut self, headers: Option>, fin: bool, decoder: &mut QPackDecoder) { + fn add_headers( + &mut self, + headers: Vec
, + fin: bool, + decoder: &mut QPackDecoder, + ) -> Res<()> { + let interim = self.is_interim(&headers)?; + + if fin && interim { + return Err(Error::HttpGeneralProtocolStream); + } + + self.conn_events + .header_ready(self.stream_id, headers, interim, fin); + if fin { - self.conn_events.header_ready(self.stream_id, headers, true); self.set_closed(decoder); } else { - self.conn_events - .header_ready(self.stream_id, headers, false); - self.state = RecvMessageState::WaitingForData { - frame_reader: HFrameReader::new(), + self.state = if interim { + RecvMessageState::WaitingForResponseHeaders { + frame_reader: HFrameReader::new(), + } + } else { + RecvMessageState::WaitingForData { + frame_reader: HFrameReader::new(), + } }; } + Ok(()) } - fn set_state_to_close_pending( - &mut self, - decoder: &mut QPackDecoder, - post_readable_event: bool, - ) { + fn set_state_to_close_pending(&mut self, post_readable_event: bool) -> Res<()> { // Stream has received fin. Depending on headers state set header_ready // or data_readable event so that app can pick up the fin. qtrace!([self], "set_state_to_close_pending: state={:?}", self.state); match self.state { RecvMessageState::WaitingForResponseHeaders { .. } => { - self.conn_events.header_ready(self.stream_id, None, true); - self.set_closed(decoder); + return Err(Error::HttpGeneralProtocolStream); } RecvMessageState::ReadingData { .. } => {} RecvMessageState::WaitingForData { .. } @@ -171,6 +187,7 @@ impl RecvMessage { if !matches!(self.state, RecvMessageState::Closed) { self.state = RecvMessageState::ClosePending; } + Ok(()) } fn handle_push_promise( @@ -219,8 +236,7 @@ impl RecvMessage { | RecvMessageState::WaitingForFinAfterTrailers { frame_reader } => { match frame_reader.receive(conn, self.stream_id)? { (None, true) => { - self.set_state_to_close_pending(decoder, post_readable_event); - break Ok(()); + break self.set_state_to_close_pending(post_readable_event); } (None, false) => break Ok(()), (Some(frame), fin) => { @@ -233,7 +249,7 @@ impl RecvMessage { ); match frame { HFrame::Headers { header_block } => { - self.handle_headers_frame(header_block, fin, decoder)? + self.handle_headers_frame(header_block, fin)? } HFrame::Data { len } => self.handle_data_frame(len, fin)?, HFrame::PushPromise { @@ -246,8 +262,7 @@ impl RecvMessage { break Ok(()); } if fin && !matches!(self.state, RecvMessageState::DecodingHeaders{..}) { - self.set_state_to_close_pending(decoder, post_readable_event); - break Ok(()); + break self.set_state_to_close_pending(post_readable_event); } } }; @@ -269,8 +284,8 @@ impl RecvMessage { if let Some(headers) = decoder.decode_header_block(header_block, self.stream_id)? { - self.add_headers(Some(headers), done, decoder); - if done { + self.add_headers(headers, done, decoder)?; + if matches!(self.state, RecvMessageState::Closed) { break Ok(()); } } else { @@ -304,6 +319,23 @@ impl RecvMessage { RecvMessageState::ClosePending | RecvMessageState::Closed ) } + + fn is_interim(&self, headers: &[Header]) -> Res { + match self.message_type { + MessageType::Response => { + let status = headers.iter().find(|(name, _value)| name == ":status"); + if let Some((_name, value)) = status { + let status_code = value + .parse::() + .map_err(|_| Error::HttpGeneralProtocolStream)?; + Ok(status_code >= 100 && status_code < 200) + } else { + Err(Error::HttpGeneralProtocolStream) + } + } + MessageType::Request => Ok(false), + } + } } impl RecvStream for RecvMessage { @@ -333,16 +365,18 @@ impl RecvStream for RecvMessage { matches!(self.state, RecvMessageState::Closed) } - fn stream_reset_recv(&self, app_error: AppError, decoder: &mut QPackDecoder) { + fn stream_reset(&self, app_error: AppError, decoder: &mut QPackDecoder, reset_type: ResetType) { if !self.closing() || !self.blocked_push_promise.is_empty() { decoder.cancel_stream(self.stream_id); } - self.conn_events.reset(self.stream_id, app_error); - } - - fn stream_reset(&self, decoder: &mut QPackDecoder) { - if !self.closing() || !self.blocked_push_promise.is_empty() { - decoder.cancel_stream(self.stream_id); + match reset_type { + ResetType::Local => { + self.conn_events.reset(self.stream_id, app_error, true); + } + ResetType::Remote => { + self.conn_events.reset(self.stream_id, app_error, false); + } + ResetType::App => {} } } diff --git a/third_party/rust/neqo-http3/src/server.rs b/third_party/rust/neqo-http3/src/server.rs index d28c86c2033f..249f97deec29 100644 --- a/third_party/rust/neqo-http3/src/server.rs +++ b/third_party/rust/neqo-http3/src/server.rs @@ -739,7 +739,7 @@ mod tests { while let Some(event) = hconn.next_event() { match event { Http3ServerEvent::Headers { headers, fin, .. } => { - check_request_header(&headers.unwrap()); + check_request_header(&headers); assert_eq!(fin, false); headers_frames += 1; } @@ -790,7 +790,7 @@ mod tests { headers, fin, } => { - check_request_header(&headers.unwrap()); + check_request_header(&headers); assert_eq!(fin, false); headers_frames += 1; request @@ -860,7 +860,7 @@ mod tests { headers, fin, } => { - check_request_header(&headers.unwrap()); + check_request_header(&headers); assert_eq!(fin, false); headers_frames += 1; request diff --git a/third_party/rust/neqo-http3/src/server_connection_events.rs b/third_party/rust/neqo-http3/src/server_connection_events.rs index 7f0083e6323c..63549825102a 100644 --- a/third_party/rust/neqo-http3/src/server_connection_events.rs +++ b/third_party/rust/neqo-http3/src/server_connection_events.rs @@ -20,7 +20,7 @@ pub(crate) enum Http3ServerConnEvent { /// Headers are ready. Headers { stream_id: u64, - headers: Option>, + headers: Vec
, fin: bool, }, /// Request data is ready. @@ -39,7 +39,7 @@ pub(crate) struct Http3ServerConnEvents { impl RecvMessageEvents for Http3ServerConnEvents { /// Add a new `HeaderReady` event. - fn header_ready(&self, stream_id: u64, headers: Option>, fin: bool) { + fn header_ready(&self, stream_id: u64, headers: Vec
, _interim: bool, fin: bool) { self.insert(Http3ServerConnEvent::Headers { stream_id, headers, @@ -52,7 +52,7 @@ impl RecvMessageEvents for Http3ServerConnEvents { self.insert(Http3ServerConnEvent::DataReadable { stream_id }); } - fn reset(&self, _stream_id: u64, _error: AppError) {} + fn reset(&self, _stream_id: u64, _error: AppError, _local: bool) {} } impl SendMessageEvents for Http3ServerConnEvents { diff --git a/third_party/rust/neqo-http3/src/server_events.rs b/third_party/rust/neqo-http3/src/server_events.rs index 25f9fba72600..9005e44a7a78 100644 --- a/third_party/rust/neqo-http3/src/server_events.rs +++ b/third_party/rust/neqo-http3/src/server_events.rs @@ -86,7 +86,7 @@ pub enum Http3ServerEvent { /// Headers are ready. Headers { request: ClientRequestStream, - headers: Option>, + headers: Vec
, fin: bool, }, /// Request data is ready. @@ -128,12 +128,7 @@ impl Http3ServerEvents { } /// Insert a `Headers` event. - pub(crate) fn headers( - &self, - request: ClientRequestStream, - headers: Option>, - fin: bool, - ) { + pub(crate) fn headers(&self, request: ClientRequestStream, headers: Vec
, fin: bool) { self.insert(Http3ServerEvent::Headers { request, headers, diff --git a/third_party/rust/neqo-http3/src/settings.rs b/third_party/rust/neqo-http3/src/settings.rs index d0a21352b164..9eee0fe4b4d7 100644 --- a/third_party/rust/neqo-http3/src/settings.rs +++ b/third_party/rust/neqo-http3/src/settings.rs @@ -4,6 +4,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![allow(clippy::module_name_repetitions)] + use crate::{Error, Res}; use neqo_common::{Decoder, Encoder}; use neqo_crypto::{ZeroRttCheckResult, ZeroRttChecker}; @@ -23,7 +25,7 @@ const SETTINGS_QPACK_BLOCKED_STREAMS: SettingsType = 0x7; pub const H3_RESERVED_SETTINGS: &[SettingsType] = &[0x2, 0x3, 0x4, 0x5]; #[derive(Clone, PartialEq, Debug, Copy)] -pub(crate) enum HSettingType { +pub enum HSettingType { MaxHeaderListSize, MaxTableCapacity, BlockedStreams, @@ -37,7 +39,7 @@ fn hsetting_default(setting_type: HSettingType) -> u64 { } #[derive(Debug, Clone, PartialEq)] -pub(crate) struct HSetting { +pub struct HSetting { pub setting_type: HSettingType, pub value: u64, } @@ -52,7 +54,7 @@ impl HSetting { } #[derive(Clone, Debug, Default, PartialEq)] -pub(crate) struct HSettings { +pub struct HSettings { settings: Vec, } diff --git a/third_party/rust/neqo-http3/tests/httpconn.rs b/third_party/rust/neqo-http3/tests/httpconn.rs index 94f091a4fa45..f53c8a93ba00 100644 --- a/third_party/rust/neqo-http3/tests/httpconn.rs +++ b/third_party/rust/neqo-http3/tests/httpconn.rs @@ -24,12 +24,12 @@ fn process_server_events(server: &mut Http3Server) { { assert_eq!( headers, - Some(vec![ + vec![ (String::from(":method"), String::from("GET")), (String::from(":scheme"), String::from("https")), (String::from(":authority"), String::from("something.com")), (String::from(":path"), String::from("/")) - ]) + ] ); assert_eq!(fin, true); request @@ -55,10 +55,10 @@ fn process_client_events(conn: &mut Http3Client) { Http3ClientEvent::HeaderReady { headers, fin, .. } => { assert_eq!( headers, - Some(vec![ + vec![ (String::from(":status"), String::from("200")), (String::from("content-length"), String::from("3")), - ]) + ] ); assert_eq!(fin, false); response_header_found = true; diff --git a/third_party/rust/neqo-qpack/.cargo-checksum.json b/third_party/rust/neqo-qpack/.cargo-checksum.json index 8f93de511cf6..f1e395a54054 100644 --- a/third_party/rust/neqo-qpack/.cargo-checksum.json +++ b/third_party/rust/neqo-qpack/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"1ee9cfe1442a94fce8c81218f061e9a443ad4646abf5708b800e2a5e0ea43498","src/decoder.rs":"e57f30c5e3a4e4f8b52b44c6bdf5f34ab8780cc4a18f89328af03c456eeef4f4","src/decoder_instructions.rs":"a8e04dff5fc4c658322a10daadab947dc2e41932c00c3f8d387671a86d0516af","src/encoder.rs":"0daf6111bb00dd93d724a664697f87bd3c92a0d154cfbd882016110995677466","src/encoder_instructions.rs":"1d4424bf21c0ac26b7c8fee6450b943346c5493ab86dd7ec2edc5f566454721e","src/header_block.rs":"f935872919154f678947732270d688be4790309f9e390a0c8eb6c9484a41a8dd","src/huffman.rs":"68fa0bada0c35d20f793980596accdcc548970214841f71789290fc334e51fc1","src/huffman_decode_helper.rs":"2970c57f052878b727c2f764490c54184f5c2608e1d6aa961c3b01509e290122","src/huffman_table.rs":"06fea766a6276ac56c7ee0326faed800a742c15fda1f33bf2513e6cc6a5e6d27","src/lib.rs":"8ee2082fd94064e61286e24e5192ee02eee50b1ca82b9105a1e1945e596ccaa8","src/prefix.rs":"72c587c40aef4ed38cf13b2de91091d671611679be2a9da6f0b24abafaf50dc5","src/qlog.rs":"7618085e27bb3fb1f4d1c73ba501b9a293723293c4020b7cc4129676eb278131","src/qpack_send_buf.rs":"5170b93afaf0c1609463e6c5ae4dccb1a2ce4e4407296db1bcaf06c6b5bb97ab","src/reader.rs":"4bcea0de1d7dc09ec0cdff364d8f62da54bbbe1f6db55a495f943f31369b4074","src/static_table.rs":"fda9d5c6f38f94b0bf92d3afdf8432dce6e27e189736596e16727090c77b78ec","src/stats.rs":"bc82c8f86b54981be234948e285af3d778ba9d23ddaaf2fbedf9c03121eaada7","src/table.rs":"f7091bdd9ad1f8fe3b2298a7dbfd3d285c212d69569cda54f9bcf251cb758a21"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"fa417a52bf7190cd40dc13f921b40ade2ee3b7d95d00586938f3a4386fc1eaf4","src/decoder.rs":"e57f30c5e3a4e4f8b52b44c6bdf5f34ab8780cc4a18f89328af03c456eeef4f4","src/decoder_instructions.rs":"a8e04dff5fc4c658322a10daadab947dc2e41932c00c3f8d387671a86d0516af","src/encoder.rs":"0daf6111bb00dd93d724a664697f87bd3c92a0d154cfbd882016110995677466","src/encoder_instructions.rs":"1d4424bf21c0ac26b7c8fee6450b943346c5493ab86dd7ec2edc5f566454721e","src/header_block.rs":"f935872919154f678947732270d688be4790309f9e390a0c8eb6c9484a41a8dd","src/huffman.rs":"68fa0bada0c35d20f793980596accdcc548970214841f71789290fc334e51fc1","src/huffman_decode_helper.rs":"2970c57f052878b727c2f764490c54184f5c2608e1d6aa961c3b01509e290122","src/huffman_table.rs":"06fea766a6276ac56c7ee0326faed800a742c15fda1f33bf2513e6cc6a5e6d27","src/lib.rs":"b4270c5fc95e814d3fe2177be56f7d6a791b0ec25edefdce9c4dd4f92035bf6e","src/prefix.rs":"72c587c40aef4ed38cf13b2de91091d671611679be2a9da6f0b24abafaf50dc5","src/qlog.rs":"7618085e27bb3fb1f4d1c73ba501b9a293723293c4020b7cc4129676eb278131","src/qpack_send_buf.rs":"5170b93afaf0c1609463e6c5ae4dccb1a2ce4e4407296db1bcaf06c6b5bb97ab","src/reader.rs":"4bcea0de1d7dc09ec0cdff364d8f62da54bbbe1f6db55a495f943f31369b4074","src/static_table.rs":"fda9d5c6f38f94b0bf92d3afdf8432dce6e27e189736596e16727090c77b78ec","src/stats.rs":"bc82c8f86b54981be234948e285af3d778ba9d23ddaaf2fbedf9c03121eaada7","src/table.rs":"f7091bdd9ad1f8fe3b2298a7dbfd3d285c212d69569cda54f9bcf251cb758a21"},"package":null} \ No newline at end of file diff --git a/third_party/rust/neqo-qpack/Cargo.toml b/third_party/rust/neqo-qpack/Cargo.toml index 25e0a5e501f2..7088143c0383 100644 --- a/third_party/rust/neqo-qpack/Cargo.toml +++ b/third_party/rust/neqo-qpack/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "neqo-qpack" -version = "0.4.13" +version = "0.4.14" authors = ["Dragana Damjanovic "] edition = "2018" license = "MIT/Apache-2.0" diff --git a/third_party/rust/neqo-qpack/src/lib.rs b/third_party/rust/neqo-qpack/src/lib.rs index 10576cf6fb07..f5a3df11a95b 100644 --- a/third_party/rust/neqo-qpack/src/lib.rs +++ b/third_party/rust/neqo-qpack/src/lib.rs @@ -27,6 +27,9 @@ mod static_table; pub mod stats; mod table; +pub use decoder::QPackDecoder; +pub use encoder::QPackEncoder; + pub type Header = (String, String); type Res = Result; diff --git a/third_party/rust/neqo-transport/.cargo-checksum.json b/third_party/rust/neqo-transport/.cargo-checksum.json index d2c4d5c92a88..3c819811f428 100644 --- a/third_party/rust/neqo-transport/.cargo-checksum.json +++ b/third_party/rust/neqo-transport/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"5a19321e3bf63e29e20ae4fe01cd032fc239da9b0925be834b471711641bd415","TODO":"d759cb804b32fa9d96ea8d3574a3c4073da9fe6a0b02b708a0e22cce5a5b4a0f","src/addr_valid.rs":"76d2f16f321c4d4e3e6c1333d4d3b36cdfab7d4655dc9f10011bd3808354bd8a","src/cc/classic_cc.rs":"d0c55a2645d9b0ec59d8227f4340e26e8922376d68402fabcacbd13fd6b10293","src/cc/mod.rs":"36e9018f70507a80404764e646467f769f9ad0d87d5ff806450ae242dcfd18a5","src/cc/new_reno.rs":"4734a85122dae88a087952ce1f9ef006aea8405a2bc1d4a0565c5bfd238e4ef4","src/cid.rs":"936ed772eddf533cb962332a68034a0e209023991fa44ec72e2805cdff29a3c2","src/connection/idle.rs":"e3c902858737de89e2e8955796c6e60dcfbcffaf58b3a1c4473fed04c7eae55c","src/connection/mod.rs":"8b43cd4e7ef2deb9fdc3ecf385159f21379083307622dc3775f349dd2d0beea9","src/connection/saved.rs":"94f9823182e91de5919b5dfb3978a2e0655eb5748594bf24d324877039c78372","src/connection/state.rs":"ddb61eb78f4ff6be1dcb9eac986549a6918ca95beb24cabd223450b6132191bd","src/connection/tests/cc.rs":"e22db134f67f11954c58ee026fca8b8d8e9357b0f157dd3dea8b5e1991d9ce7b","src/connection/tests/close.rs":"037856e0de3ede9821ddd6245b440512889ba125c706484a7afed3b5d9609473","src/connection/tests/handshake.rs":"bb671b2e3154f95b37a34fc975269dfa6017cafd8ef8c5481df34e2320d181b3","src/connection/tests/idle.rs":"f0c9b0f0820d26d6bb294cbffbce50c9e54bc5974f1e39c1ec32db9610e946ea","src/connection/tests/keys.rs":"02ea223d3d60dd4c7decc861f121584f32a030c952f9482a032aa21a44b4972b","src/connection/tests/mod.rs":"c228a5060d39521cd7bfdf5a4ead26dbaf639c1530b1dcbc00dae55c7ff07cbf","src/connection/tests/recovery.rs":"a7e85674a10dc96eccba7a8bc633d3059cad6e5cd9bc6ffd5ec1193be462b9e6","src/connection/tests/resumption.rs":"538c0ecb7eed22ea9c22882e934bf91cc87c064c6d4286e5cc7fbbaecbc650fc","src/connection/tests/stream.rs":"3e09f2d333f8fa18118b99233411d1b60832532031ad6609cb8e73bb131e76f5","src/connection/tests/vn.rs":"ffb1c34be57b2cae704b387a2599cedeb1a8775d5a5e48eb0d0113d767c4e6a6","src/connection/tests/zerortt.rs":"8c5e874b36e34306f640ba7fc760d38847292b399f6eec97b565de12b77cbd76","src/crypto.rs":"d65740f1737339836ef2152b3679e616c1ec70a643e2d63862f446b32846116c","src/dump.rs":"d69ccb0e3b240823b886a791186afbac9f2e26d1f1f67a55dbf86f8cd3e6203e","src/events.rs":"6d1e168eb8c425018281e82ba3ff05ea0321035eb677f1d1bdd19fe32c1e62ce","src/flow_mgr.rs":"ff820f4f45fb5dbed5e4b1e9433483a36fc624bd5fa1a22dd7abb779ba723caf","src/frame.rs":"f3a32a6e77d5df42c3c174143b0dde1aff5c7a2e08dd0e260131952e68368f6e","src/lib.rs":"580c2065f035c5759d3ee9f735ade6dbf86a5351000a4e86f61fa40001e00c27","src/pace.rs":"eb9094cfcae54162022f70f230b6a9811add0063b878100f147a9365473f6612","src/packet/mod.rs":"20d542201721aff6239db19022c39d78645df400e94913522842167d15d504b7","src/packet/retry.rs":"0473f4b93f506a9a6c6a72ea20c0d0811b98e42d1f92a412821180cf3b6916fa","src/path.rs":"92b2701d1e486250f472b2cdc487b1e9e9c8753c9c40b626c2d6d1ccd6e70c38","src/qlog.rs":"b2853fdc7acfb86c8b802a39f32b0462dd82bf86f5c427ffdb523be933268532","src/recovery.rs":"e2a1c81e7f34209d7463d5587bfae7d5e4de565586d52565ac32d80fb3f14c70","src/recv_stream.rs":"5549e6c50951b4ec5854032a615b35119b5ae1b3f53af51192155c5534717147","src/send_stream.rs":"d77e439c9411269f4afb6be3db29fe918525c2034a52f56a65abe283c41373c0","src/sender.rs":"fdb16a6c44473dbd2dce3f0982f467e4cc8af579b9bd2abf2f3444845d2c07d0","src/server.rs":"cdb0eb1e194110a39335e75018b861e34fab855adf76fb218c7b982bd10f7833","src/stats.rs":"b2573f5c2a16f8c45dea0d9b689b826f4f3128dd0f495c8c69e79f9f97ee3e92","src/stream_id.rs":"98f656157e0eeb7f32802cf2b3dbd68ef8d7a89578a097a88db7157d4a712202","src/tparams.rs":"d4a69bff044f51acab5dcbe56531bfd5eee79b99908c79074409d38ead8d3467","src/tracking.rs":"9d89236c43fde583fe4f78c981280ad152a32fbd2027effaba6d1e246bfbd481","tests/conn_vectors.rs":"d5bd7ba17dfd3f7c6fe421ebd346ba7df8ddc08756fa7a747ecba52bcca6a7f0","tests/connection.rs":"a93985c199a9ef987106f4a20b35ebf803cdbbb855c07b1362b403eed7101ef8","tests/network.rs":"a986c22da7132ec843a44c4bcb5a7d2726132aa27a47a8ea91634cd88e1b763b","tests/server.rs":"da8d967437cfd1847d923c60685c879e62004c4be45c2e1d74c01a9727239424","tests/sim/connection.rs":"5e65e7247f69ad9d992cffc9daac6a8af360c6c312e6588da07478337a12819f","tests/sim/delay.rs":"9efa722adb89e37262369e9f3c67405f0acc8c24997271811e48df9e856e5a8d","tests/sim/drop.rs":"bd89e5c71cdd1b27cd755faaedd87d5feadf2f424df721a7df41a51bcebcbb58","tests/sim/mod.rs":"9a930682cf92e7279bccdd2145f19ff17f5aa950994e7b3e25749651511c2753","tests/sim/net.rs":"597f4d37bc26c3d82eeeaa6d14dd03bc2be3930686df2b293748b43c07c497d7","tests/sim/rng.rs":"2c90b0bbaf0c952ebee232deb3594f7a86af387737b15474de3e97ee6b623d90","tests/sim/taildrop.rs":"5c505d150f0071e8cc2d540b3a817a6942fdf13df32f1fbc6822952f2e146176"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"9e69f568d271290db9bf33d044c5bf41f1274c185226fe5711187ad29bedc8f1","TODO":"d759cb804b32fa9d96ea8d3574a3c4073da9fe6a0b02b708a0e22cce5a5b4a0f","src/addr_valid.rs":"1b3bef8edf87d29dc630bda7a8fdb6287093612cd8da4756f1a0e2473bb01b0f","src/cc/classic_cc.rs":"d0c55a2645d9b0ec59d8227f4340e26e8922376d68402fabcacbd13fd6b10293","src/cc/mod.rs":"36e9018f70507a80404764e646467f769f9ad0d87d5ff806450ae242dcfd18a5","src/cc/new_reno.rs":"4734a85122dae88a087952ce1f9ef006aea8405a2bc1d4a0565c5bfd238e4ef4","src/cid.rs":"e231286df599b9ee7cbae8cab274908e0197baf2b2e88bea9d8f40dbeb02c57d","src/connection/idle.rs":"e3c902858737de89e2e8955796c6e60dcfbcffaf58b3a1c4473fed04c7eae55c","src/connection/mod.rs":"d1869a2fff3e5edd96bd123ec0a5f4bd48a8d88133f4dad64ef445886fd1d6ce","src/connection/saved.rs":"94f9823182e91de5919b5dfb3978a2e0655eb5748594bf24d324877039c78372","src/connection/state.rs":"52b8def3097cb949888796cb07cea5cf0bb3e045fc2b22a4ca7b181e87db3640","src/connection/tests/cc.rs":"57c47c8ef7a13df2dee330951d79746fe7e995e39fc9c2d009b99f800cb8ff31","src/connection/tests/close.rs":"ddab204ae0c88bac55aed02c5eff3c793a53113dd07b489bdb7a07867df4aeac","src/connection/tests/handshake.rs":"ab244675df5add3bd4ed58f3511cd2c9c8db75e02f2b99ab92cb902e9e8940c4","src/connection/tests/idle.rs":"7c73be2a1079d2ee66960d403076b059356736b35bba657f278348493068a6a5","src/connection/tests/keys.rs":"029e0cb2a043828059f006d0e17eb36b7d978c48d16340a80711e144a709245e","src/connection/tests/mod.rs":"5ee29828f06cc92cca582206fdf5ad2254d74a93b3467222b3f3d846edb33158","src/connection/tests/recovery.rs":"f6dbdeeaec18510c0f9345ee25947c0ed988b14fddf30e124dfe6d1b9a160a71","src/connection/tests/resumption.rs":"538c0ecb7eed22ea9c22882e934bf91cc87c064c6d4286e5cc7fbbaecbc650fc","src/connection/tests/stream.rs":"37e91c34851e85763684afceca9a8498101602d629e161d99e10248f3b908a81","src/connection/tests/vn.rs":"ffb1c34be57b2cae704b387a2599cedeb1a8775d5a5e48eb0d0113d767c4e6a6","src/connection/tests/zerortt.rs":"8c5e874b36e34306f640ba7fc760d38847292b399f6eec97b565de12b77cbd76","src/crypto.rs":"d254c11595311b24f81ace1d6b5f4ec8f275c409b392361ef9b6d3ea1ea362bc","src/dump.rs":"d69ccb0e3b240823b886a791186afbac9f2e26d1f1f67a55dbf86f8cd3e6203e","src/events.rs":"6d1e168eb8c425018281e82ba3ff05ea0321035eb677f1d1bdd19fe32c1e62ce","src/flow_mgr.rs":"4f667a5b8574c248e7422aaac3492d89f4a2e7e202a19083b9f84cf129e49037","src/frame.rs":"d2d175fb058c4157c8717595784c2ab26df8ae9b5e2ba2e99d579203314991c0","src/lib.rs":"580c2065f035c5759d3ee9f735ade6dbf86a5351000a4e86f61fa40001e00c27","src/pace.rs":"eb9094cfcae54162022f70f230b6a9811add0063b878100f147a9365473f6612","src/packet/mod.rs":"ab37449f8bff0c86a3affe66fdc22a34b077b815e0fedb3658cfca411fdb6d07","src/packet/retry.rs":"0473f4b93f506a9a6c6a72ea20c0d0811b98e42d1f92a412821180cf3b6916fa","src/path.rs":"92b2701d1e486250f472b2cdc487b1e9e9c8753c9c40b626c2d6d1ccd6e70c38","src/qlog.rs":"b2853fdc7acfb86c8b802a39f32b0462dd82bf86f5c427ffdb523be933268532","src/recovery.rs":"e2a1c81e7f34209d7463d5587bfae7d5e4de565586d52565ac32d80fb3f14c70","src/recv_stream.rs":"71143d3ce2670d55dbc5cf7ecd9d3b5826a3060029162f335fffd197bedf149f","src/send_stream.rs":"4c019d99cdb1ad76bc5434fbde009371a49ef61138e84609595686af8e2ca651","src/sender.rs":"fdb16a6c44473dbd2dce3f0982f467e4cc8af579b9bd2abf2f3444845d2c07d0","src/server.rs":"cdb0eb1e194110a39335e75018b861e34fab855adf76fb218c7b982bd10f7833","src/stats.rs":"2f99e4e545e7df0b69aec5e1153cb5b0d2e274a9ead1e8d1cccbf4217109c7a9","src/stream_id.rs":"98f656157e0eeb7f32802cf2b3dbd68ef8d7a89578a097a88db7157d4a712202","src/tparams.rs":"d4a69bff044f51acab5dcbe56531bfd5eee79b99908c79074409d38ead8d3467","src/tracking.rs":"7dfe2f6702609361348eeddfc0ea61b9a58c31d88884adfe5c550d94cdbc2268","tests/conn_vectors.rs":"d5bd7ba17dfd3f7c6fe421ebd346ba7df8ddc08756fa7a747ecba52bcca6a7f0","tests/connection.rs":"fe9b7069b34fea7f5154e9b1ea3bffb290a4aa9a7c97c30992c51fa7a6ad6673","tests/network.rs":"a986c22da7132ec843a44c4bcb5a7d2726132aa27a47a8ea91634cd88e1b763b","tests/server.rs":"142eaf443650e0d2a6cb75e439c65fa35313d574946ac6d604861bf8866190de","tests/sim/connection.rs":"5e65e7247f69ad9d992cffc9daac6a8af360c6c312e6588da07478337a12819f","tests/sim/delay.rs":"9efa722adb89e37262369e9f3c67405f0acc8c24997271811e48df9e856e5a8d","tests/sim/drop.rs":"bd89e5c71cdd1b27cd755faaedd87d5feadf2f424df721a7df41a51bcebcbb58","tests/sim/mod.rs":"9a930682cf92e7279bccdd2145f19ff17f5aa950994e7b3e25749651511c2753","tests/sim/net.rs":"597f4d37bc26c3d82eeeaa6d14dd03bc2be3930686df2b293748b43c07c497d7","tests/sim/rng.rs":"2c90b0bbaf0c952ebee232deb3594f7a86af387737b15474de3e97ee6b623d90","tests/sim/taildrop.rs":"5c505d150f0071e8cc2d540b3a817a6942fdf13df32f1fbc6822952f2e146176"},"package":null} \ No newline at end of file diff --git a/third_party/rust/neqo-transport/Cargo.toml b/third_party/rust/neqo-transport/Cargo.toml index ef0a69f06941..3b057cdc00d4 100644 --- a/third_party/rust/neqo-transport/Cargo.toml +++ b/third_party/rust/neqo-transport/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "neqo-transport" -version = "0.4.13" +version = "0.4.14" authors = ["EKR ", "Andy Grover "] edition = "2018" license = "MIT/Apache-2.0" diff --git a/third_party/rust/neqo-transport/src/addr_valid.rs b/third_party/rust/neqo-transport/src/addr_valid.rs index 48a6ea470830..1ff5959a37a2 100644 --- a/third_party/rust/neqo-transport/src/addr_valid.rs +++ b/third_party/rust/neqo-transport/src/addr_valid.rs @@ -13,8 +13,9 @@ use neqo_crypto::{ }; use crate::cid::ConnectionId; -use crate::frame::Frame; +use crate::packet::PacketBuilder; use crate::recovery::RecoveryToken; +use crate::stats::FrameStats; use crate::Res; use smallvec::SmallVec; @@ -349,11 +350,14 @@ impl NewTokenState { /// If this is a server, maybe send a frame. /// If this is a client, do nothing. - pub fn get_frame(&mut self, space: usize) -> Option<(Frame, Option)> { + pub fn write_frames( + &mut self, + builder: &mut PacketBuilder, + tokens: &mut Vec, + stats: &mut FrameStats, + ) { if let Self::Server(ref mut sender) = self { - sender.get_frame(space) - } else { - None + sender.write_frames(builder, tokens, stats); } } @@ -421,19 +425,23 @@ impl NewTokenSender { self.next_seqno += 1; } - pub fn get_frame(&mut self, space: usize) -> Option<(Frame, Option)> { + pub fn write_frames( + &mut self, + builder: &mut PacketBuilder, + tokens: &mut Vec, + stats: &mut FrameStats, + ) { for t in self.tokens.iter_mut() { - if t.needs_sending && t.fits(space) { + if t.needs_sending && t.fits(builder.remaining()) { t.needs_sending = false; - return Some(( - Frame::NewToken { - token: t.token.clone(), - }, - Some(RecoveryToken::NewToken(t.seqno)), - )); + + builder.encode_varint(crate::frame::FRAME_TYPE_NEW_TOKEN); + builder.encode_vvec(&t.token); + + tokens.push(RecoveryToken::NewToken(t.seqno)); + stats.new_token += 1; } } - None } pub fn lost(&mut self, seqno: usize) { diff --git a/third_party/rust/neqo-transport/src/cid.rs b/third_party/rust/neqo-transport/src/cid.rs index 8699c915f3f7..ef2b938c2871 100644 --- a/third_party/rust/neqo-transport/src/cid.rs +++ b/third_party/rust/neqo-transport/src/cid.rs @@ -106,7 +106,7 @@ impl<'a> ::std::fmt::Debug for ConnectionIdRef<'a> { impl<'a> ::std::fmt::Display for ConnectionIdRef<'a> { fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { - write!(f, "{}", hex_with_len(&self.cid)) + write!(f, "{}", hex(&self.cid)) } } diff --git a/third_party/rust/neqo-transport/src/connection/mod.rs b/third_party/rust/neqo-transport/src/connection/mod.rs index 3316a6c1f3fd..fd58097c63c6 100644 --- a/third_party/rust/neqo-transport/src/connection/mod.rs +++ b/third_party/rust/neqo-transport/src/connection/mod.rs @@ -256,13 +256,16 @@ pub struct Connection { valid_cids: Vec, address_validation: AddressValidationInfo, + /// The source connection ID that this endpoint uses for the handshake. /// Since we need to communicate this to our peer in tparams, setting this /// value is part of constructing the struct. local_initial_source_cid: ConnectionId, - /// Checked against tparam value from peer + /// The source connection ID from the first packet from the other end. + /// This is checked against the peer's transport parameters. remote_initial_source_cid: Option, - /// Checked against tparam value from peer - remote_original_destination_cid: Option, + /// The destination connection ID from the first packet from the client. + /// This is checked by the client against the server's transport parameters. + original_destination_cid: Option, /// We sometimes save a datagram against the possibility that keys will later /// become available. This avoids reporting packets as dropped during the handshake @@ -274,7 +277,7 @@ pub struct Connection { pub(crate) acks: AckTracker, idle_timeout: IdleTimeout, pub(crate) indexes: StreamIndexes, - connection_ids: HashMap, [u8; 16])>, // (sequence number, (connection id, reset token)) + connection_ids: HashMap, // (sequence number, (connection id, reset token)) pub(crate) send_streams: SendStreams, pub(crate) recv_streams: RecvStreams, pub(crate) flow_mgr: Rc>, @@ -322,7 +325,7 @@ impl Connection { quic_version, )?; c.crypto.states.init(quic_version, Role::Client, &dcid); - c.remote_original_destination_cid = Some(dcid); + c.original_destination_cid = Some(dcid); c.initialize_path(local_addr, remote_addr); Ok(c) } @@ -410,7 +413,7 @@ impl Connection { address_validation: AddressValidationInfo::None, local_initial_source_cid, remote_initial_source_cid: None, - remote_original_destination_cid: None, + original_destination_cid: None, saved_datagrams: SavedDatagrams::default(), crypto, acks: AckTracker::default(), @@ -453,7 +456,7 @@ impl Connection { /// will always be present for Role::Client but not if Role::Server is in /// State::Init. pub fn odcid(&self) -> Option<&ConnectionId> { - self.remote_original_destination_cid.as_ref() + self.original_destination_cid.as_ref() } /// Set a local transport parameter, possibly overriding a default value. @@ -836,16 +839,6 @@ impl Connection { self.cleanup_streams(); } - /// Just like above but returns frames parsed from the datagram - #[cfg(test)] - #[must_use] - pub fn test_process_input(&mut self, dgram: Datagram, now: Instant) -> Vec<(Frame, PNSpace)> { - let res = self.input(dgram, now); - let frames = self.absorb_error(now, res).unwrap_or_default(); - self.cleanup_streams(); - frames - } - /// Get the time that we next need to be called back, relative to `now`. fn next_delay(&mut self, now: Instant, paced: bool) -> Duration { qtrace!([self], "Get callback delay {:?}", now); @@ -953,7 +946,7 @@ impl Connection { self.stats.borrow_mut().pkt_dropped("Retry without a token"); return Ok(()); } - if !packet.is_valid_retry(&self.remote_original_destination_cid.as_ref().unwrap()) { + if !packet.is_valid_retry(&self.original_destination_cid.as_ref().unwrap()) { self.stats .borrow_mut() .pkt_dropped("Retry with bad integrity tag"); @@ -1067,9 +1060,16 @@ impl Connection { fn preprocess( &mut self, packet: &PublicPacket, - first: bool, + dcid: Option<&ConnectionId>, now: Instant, ) -> Res { + if dcid.map_or(false, |d| d != packet.dcid()) { + self.stats + .borrow_mut() + .pkt_dropped("Coalesced packet has different DCID"); + return Ok(PreprocessResult::Next); + } + match (packet.packet_type(), &self.state, &self.role) { (PacketType::Initial, State::Init, Role::Server) => { if !packet.is_valid_initial() { @@ -1135,7 +1135,7 @@ impl Connection { // Resend Initial CRYPTO frames immediately a few times just // in case. As we don't have an RTT estimate yet, this helps // when there is a short RTT and losses. - if first + if dcid.is_none() && self.is_valid_cid(packet.dcid()) && self.stats.borrow().saved_datagrams <= EXTRA_INITIALS { @@ -1165,7 +1165,7 @@ impl Connection { if !self.is_valid_cid(packet.dcid()) { self.stats .borrow_mut() - .pkt_dropped(format!("Ignoring packet with CID {:?}", packet.dcid())); + .pkt_dropped(format!("Invalid DCID {:?}", packet.dcid())); PreprocessResult::Next } else { if self.role == Role::Server && packet.packet_type() == PacketType::Handshake { @@ -1193,10 +1193,9 @@ impl Connection { } /// Take a datagram as input. This reports an error if the packet was bad. - fn input(&mut self, d: Datagram, now: Instant) -> Res> { + fn input(&mut self, d: Datagram, now: Instant) -> Res<()> { let mut slc = &d[..]; - let mut frames = Vec::new(); - let mut first = true; + let mut dcid = None; qtrace!([self], "input {}", hex(&**d)); @@ -1213,10 +1212,10 @@ impl Connection { break; } }; - match self.preprocess(&packet, first, now)? { + match self.preprocess(&packet, dcid.as_ref(), now)? { PreprocessResult::Continue => (), PreprocessResult::Next => break, - PreprocessResult::End => return Ok(frames), + PreprocessResult::End => return Ok(()), } qtrace!([self], "Received unverified packet {:?}", packet); @@ -1244,7 +1243,7 @@ impl Connection { self.remote_initial_source_cid = Some(ConnectionId::from(packet.scid())); self.initialize_path(d.destination(), d.source()); } - frames.extend(res?); + res?; if self.state == State::WaitInitial { self.start_handshake(&packet, &d)?; } @@ -1257,7 +1256,7 @@ impl Connection { // Don't check this packet for a stateless reset, just return. let remaining = slc.len(); self.save_datagram(cspace, d, remaining, now); - return Ok(frames); + return Ok(()); } Error::KeysExhausted => { // Exhausting read keys is fatal. @@ -1268,23 +1267,19 @@ impl Connection { // Decryption failure, or not having keys is not fatal. // If the state isn't available, or we can't decrypt the packet, drop // the rest of the datagram on the floor, but don't generate an error. - self.check_stateless_reset(&d, first, now)?; + self.check_stateless_reset(&d, dcid.is_none(), now)?; self.stats.borrow_mut().pkt_dropped("Decryption failure"); qlog::packet_dropped(&mut self.qlog, &packet); } } slc = remainder; - first = false; + dcid = Some(ConnectionId::from(packet.dcid())); } - self.check_stateless_reset(&d, first, now)?; - Ok(frames) + self.check_stateless_reset(&d, dcid.is_none(), now)?; + Ok(()) } - fn process_packet( - &mut self, - packet: &DecryptedPacket, - now: Instant, - ) -> Res> { + fn process_packet(&mut self, packet: &DecryptedPacket, now: Instant) -> Res<()> { // TODO(ekr@rtfm.com): Have the server blow away the initial // crypto state if this fails? Otherwise, we will get a panic // on the assert for doesn't exist. @@ -1294,14 +1289,12 @@ impl Connection { if self.acks.get_mut(space).unwrap().is_duplicate(packet.pn()) { qdebug!([self], "Duplicate packet from {} pn={}", space, packet.pn()); self.stats.borrow_mut().dups_rx += 1; - return Ok(vec![]); + return Ok(()); } let mut ack_eliciting = false; let mut d = Decoder::from(&packet[..]); let mut consecutive_padding = 0; - #[allow(unused_mut)] - let mut frames = Vec::new(); while d.remaining() > 0 { let mut f = Frame::decode(&mut d)?; @@ -1319,9 +1312,6 @@ impl Connection { consecutive_padding = 0; } - if cfg!(test) { - frames.push((f.clone(), space)); - } ack_eliciting |= f.ack_eliciting(); let t = f.get_type(); let res = self.input_frame(packet.packet_type(), f, now); @@ -1332,7 +1322,7 @@ impl Connection { .unwrap() .set_received(now, packet.pn(), ack_eliciting); - Ok(frames) + Ok(()) } fn initialize_path(&mut self, local_addr: SocketAddr, remote_addr: SocketAddr) { @@ -1345,7 +1335,7 @@ impl Connection { // But we will use our own guess if necessary. self.remote_initial_source_cid .as_ref() - .or_else(|| self.remote_original_destination_cid.as_ref()) + .or_else(|| self.original_destination_cid.as_ref()) .unwrap() .clone(), )); @@ -1358,6 +1348,7 @@ impl Connection { // A server needs to accept the client's selected CID during the handshake. self.valid_cids.push(ConnectionId::from(packet.dcid())); + self.original_destination_cid = Some(ConnectionId::from(packet.dcid())); // Install a path. self.initialize_path(d.destination(), d.source()); @@ -1487,10 +1478,25 @@ impl Connection { ); // ConnectionError::Application is only allowed at 1RTT. - if *space == PNSpace::ApplicationData { - frame.marshal(&mut builder); + let sanitized = if *space == PNSpace::ApplicationData { + &frame } else { - frame.sanitize_close().marshal(&mut builder); + frame.sanitize_close() + }; + if let Frame::ConnectionClose { + error_code, + frame_type, + reason_phrase, + } = sanitized + { + builder.encode_varint(sanitized.get_type()); + builder.encode_varint(error_code.code()); + if let CloseError::Transport(_) = error_code { + builder.encode_varint(*frame_type); + } + builder.encode_vvec(reason_phrase); + } else { + unreachable!(); } encoder = builder.build(tx)?; @@ -1499,68 +1505,66 @@ impl Connection { Ok(SendOption::Yes(path.datagram(encoder))) } - /// Add frames to the provided builder and - /// return whether any of them were ACK eliciting. - fn add_frames( + /// Write frames to the provided builder. Returns a list of tokens used for + /// tracking loss or acknowledgment and whether any frame was ACK eliciting. + fn write_frames( &mut self, - builder: &mut PacketBuilder, space: PNSpace, profile: &SendProfile, + builder: &mut PacketBuilder, now: Instant, ) -> (Vec, bool) { let mut tokens = Vec::new(); + let stats = &mut self.stats.borrow_mut().frame_tx; - let mut ack_eliciting = if profile.should_probe(space) { - // Send PING in all spaces that allow a probe. - // This might get a more expedient ACK. - builder.encode_varint(Frame::Ping.get_type()); - true - } else { - false - }; - - // Try to get a frame from all frame sources. - if let Some(t) = self.acks.write_frame(space, now, builder) { - tokens.push(t); - } + let ack_token = self.acks.write_frame(space, now, builder, stats); if profile.ack_only(space) { - return (tokens, ack_eliciting); - } - - // All useful frames are at least 2 bytes. - while builder.remaining() >= 2 { - let remaining = builder.remaining(); // If we are CC limited we can only send acks! - let mut frame = if space == PNSpace::ApplicationData && self.role == Role::Server { - self.state_signaling.send_done() - } else { - None - }; - if frame.is_none() { - frame = self.crypto.streams.get_frame(space, remaining) - } - if frame.is_none() { - frame = self.flow_mgr.borrow_mut().get_frame(space, remaining); - } - if frame.is_none() { - frame = self.send_streams.get_frame(space, remaining); - } - if frame.is_none() && space == PNSpace::ApplicationData { - frame = self.new_token.get_frame(remaining); + if let Some(t) = ack_token { + tokens.push(t); } + return (tokens, false); + } - if let Some((frame, token)) = frame { - ack_eliciting = true; - debug_assert!(frame.ack_eliciting()); - frame.marshal(builder); - if let Some(t) = token { - tokens.push(t); - } - } else { - return (tokens, ack_eliciting); + if space == PNSpace::ApplicationData && self.role == Role::Server { + if let Some(t) = self.state_signaling.write_done(builder) { + tokens.push(t); + stats.handshake_done += 1; } } + + if let Some(t) = self.crypto.streams.write_frame(space, builder) { + tokens.push(t); + stats.crypto += 1; + } + + if space == PNSpace::ApplicationData { + self.flow_mgr + .borrow_mut() + .write_frames(builder, &mut tokens, stats); + + self.send_streams.write_frames(builder, &mut tokens, stats); + self.new_token.write_frames(builder, &mut tokens, stats); + } + + // Anything - other than ACK - that registered a token wants an acknowledgment. + let ack_eliciting = !tokens.is_empty() + || if profile.should_probe(space) { + // Nothing ack-eliciting and we need to probe; send PING. + debug_assert_ne!(builder.remaining(), 0); + builder.encode_varint(crate::frame::FRAME_TYPE_PING); + stats.ping += 1; + stats.all += 1; + true + } else { + false + }; + + if let Some(t) = ack_token { + tokens.push(t); + } + stats.all += tokens.len(); (tokens, ack_eliciting) } @@ -1609,8 +1613,8 @@ impl Connection { // Add frames to the packet. let limit = profile.limit() - aead_expansion; builder.set_limit(limit); - let (tokens, ack_eliciting) = self.add_frames(&mut builder, *space, &profile, now); - if builder.is_empty() { + let (tokens, ack_eliciting) = self.write_frames(*space, &profile, &mut builder, now); + if builder.packet_empty() { // Nothing to include in this packet. encoder = builder.abort(); continue; @@ -1641,13 +1645,13 @@ impl Connection { Rc::new(tokens), encoder.len() - header_start, ); - if pt == PacketType::Initial && self.role == Role::Client { + if pt == PacketType::Initial && (self.role == Role::Client || ack_eliciting) { // Packets containing Initial packets might need padding, and we want to // track that padding along with the Initial packet. So defer tracking. initial_sent = Some(sent); needs_padding = true; } else { - if pt != PacketType::ZeroRtt { + if pt != PacketType::ZeroRtt && self.role == Role::Client { needs_padding = false; } self.loss_recovery.on_packet_sent(sent); @@ -1790,7 +1794,7 @@ impl Connection { .unwrap() .get_bytes(tparams::ORIGINAL_DESTINATION_CONNECTION_ID); if self - .remote_original_destination_cid + .original_destination_cid .as_ref() .map(ConnectionId::as_cid_ref) != tp.map(ConnectionIdRef::from) @@ -1813,8 +1817,8 @@ impl Connection { != tp.map(ConnectionIdRef::from) { qwarn!( - "{} ISCID test failed: self cid {:?} != tp cid {:?}", - self.role, + [self], + "ISCID test failed: self cid {:?} != tp cid {:?}", self.remote_initial_source_cid, tp.map(hex), ); @@ -1824,15 +1828,15 @@ impl Connection { if self.role == Role::Client { let tp = remote_tps.get_bytes(tparams::ORIGINAL_DESTINATION_CONNECTION_ID); if self - .remote_original_destination_cid + .original_destination_cid .as_ref() .map(ConnectionId::as_cid_ref) != tp.map(ConnectionIdRef::from) { qwarn!( - "{} ODCID test failed: self cid {:?} != tp cid {:?}", - self.role, - self.remote_original_destination_cid, + [self], + "ODCID test failed: self cid {:?} != tp cid {:?}", + self.original_destination_cid, tp.map(hex), ); return Err(Error::ProtocolViolation); @@ -1849,8 +1853,8 @@ impl Connection { }; if expected != tp.map(ConnectionIdRef::from) { qwarn!( - "{} RSCID test failed. self cid {:?} != tp cid {:?}", - self.role, + [self], + "RSCID test failed. self cid {:?} != tp cid {:?}", expected, tp.map(hex), ); @@ -1916,14 +1920,17 @@ impl Connection { qerror!("frame not allowed: {:?} {:?}", frame, ptype); return Err(Error::ProtocolViolation); } + self.stats.borrow_mut().frame_rx.all += 1; let space = PNSpace::from(ptype); match frame { Frame::Padding => { - // Ignore + // Note: This counts contiguous padding as a single frame. + self.stats.borrow_mut().frame_rx.padding += 1; } Frame::Ping => { // If we get a PING and there are outstanding CRYPTO frames, // prepare to resend them. + self.stats.borrow_mut().frame_rx.ping += 1; self.crypto.resend_unacked(space); } Frame::Ack { @@ -1947,6 +1954,7 @@ impl Connection { .. } => { // TODO(agrover@mozilla.com): use final_size for connection MaxData calc + self.stats.borrow_mut().frame_rx.reset_stream += 1; if let (_, Some(rs)) = self.obtain_stream(stream_id)? { rs.reset(application_error_code); } @@ -1955,6 +1963,7 @@ impl Connection { stream_id, application_error_code, } => { + self.stats.borrow_mut().frame_rx.stop_sending += 1; self.events .send_stream_stop_sending(stream_id, application_error_code); if let (Some(ss), _) = self.obtain_stream(stream_id)? { @@ -1969,7 +1978,8 @@ impl Connection { offset, &data ); - self.crypto.streams.inbound_frame(space, offset, data)?; + self.stats.borrow_mut().frame_rx.crypto += 1; + self.crypto.streams.inbound_frame(space, offset, data); if self.crypto.streams.data_ready(space) { let mut buf = Vec::new(); let read = self.crypto.streams.read_to_end(space, &mut buf); @@ -1982,7 +1992,8 @@ impl Connection { } } Frame::NewToken { token } => { - self.new_token.save_token(token); + self.stats.borrow_mut().frame_rx.new_token += 1; + self.new_token.save_token(token.to_vec()); self.create_resumption_token(now); } Frame::Stream { @@ -1992,15 +2003,20 @@ impl Connection { data, .. } => { + self.stats.borrow_mut().frame_rx.stream += 1; if let (_, Some(rs)) = self.obtain_stream(stream_id)? { rs.inbound_stream_frame(fin, offset, data)?; } } - Frame::MaxData { maximum_data } => self.handle_max_data(maximum_data), + Frame::MaxData { maximum_data } => { + self.stats.borrow_mut().frame_rx.max_data += 1; + self.handle_max_data(maximum_data); + } Frame::MaxStreamData { stream_id, maximum_stream_data, } => { + self.stats.borrow_mut().frame_rx.max_stream_data += 1; if let (Some(ss), _) = self.obtain_stream(stream_id)? { ss.set_max_stream_data(maximum_stream_data); } @@ -2009,6 +2025,7 @@ impl Connection { stream_type, maximum_streams, } => { + self.stats.borrow_mut().frame_rx.max_streams += 1; let remote_max = match stream_type { StreamType::BiDi => &mut self.indexes.remote_max_stream_bidi, StreamType::UniDi => &mut self.indexes.remote_max_stream_uni, @@ -2026,6 +2043,7 @@ impl Connection { "Received DataBlocked with data limit {}", data_limit ); + self.stats.borrow_mut().frame_rx.data_blocked += 1; // But if it does, open it up all the way self.flow_mgr.borrow_mut().max_data(LOCAL_MAX_DATA); } @@ -2033,6 +2051,7 @@ impl Connection { stream_id, stream_data_limit, } => { + self.stats.borrow_mut().frame_rx.stream_data_blocked += 1; // Terminate connection with STREAM_STATE_ERROR if send-only // stream (-transport 19.13) if stream_id.is_send_only(self.role()) { @@ -2055,6 +2074,7 @@ impl Connection { } } Frame::StreamsBlocked { stream_type, .. } => { + self.stats.borrow_mut().frame_rx.streams_blocked += 1; let local_max = match stream_type { StreamType::BiDi => &mut self.indexes.local_max_stream_bidi, StreamType::UniDi => &mut self.indexes.local_max_stream_uni, @@ -2070,23 +2090,31 @@ impl Connection { stateless_reset_token, .. } => { - self.connection_ids - .insert(sequence_number, (connection_id, stateless_reset_token)); + self.stats.borrow_mut().frame_rx.new_connection_id += 1; + let cid = ConnectionId::from(connection_id); + let srt = stateless_reset_token.to_owned(); + self.connection_ids.insert(sequence_number, (cid, srt)); } Frame::RetireConnectionId { sequence_number } => { + self.stats.borrow_mut().frame_rx.retire_connection_id += 1; self.connection_ids.remove(&sequence_number); } - Frame::PathChallenge { data } => self.flow_mgr.borrow_mut().path_response(data), + Frame::PathChallenge { data } => { + self.stats.borrow_mut().frame_rx.path_challenge += 1; + self.flow_mgr.borrow_mut().path_response(data); + } Frame::PathResponse { .. } => { // Should never see this, we don't support migration atm and // do not send path challenges qwarn!([self], "Received Path Response"); + self.stats.borrow_mut().frame_rx.path_response += 1; } Frame::ConnectionClose { error_code, frame_type, reason_phrase, } => { + self.stats.borrow_mut().frame_rx.connection_close += 1; let reason_phrase = String::from_utf8_lossy(&reason_phrase); qinfo!( [self], @@ -2116,6 +2144,7 @@ impl Connection { }); } Frame::HandshakeDone => { + self.stats.borrow_mut().frame_rx.handshake_done += 1; if self.role == Role::Server || !self.state.connected() { return Err(Error::ProtocolViolation); } @@ -2205,6 +2234,9 @@ impl Connection { } self.handle_lost_packets(&lost_packets); qlog::packets_lost(&mut self.qlog, &lost_packets); + let stats = &mut self.stats.borrow_mut().frame_rx; + stats.ack += 1; + stats.largest_acknowledged = max(stats.largest_acknowledged, largest_acknowledged); Ok(()) } diff --git a/third_party/rust/neqo-transport/src/connection/state.rs b/third_party/rust/neqo-transport/src/connection/state.rs index 13e0ca2f06f1..9667ea8da484 100644 --- a/third_party/rust/neqo-transport/src/connection/state.rs +++ b/third_party/rust/neqo-transport/src/connection/state.rs @@ -9,6 +9,7 @@ use std::mem; use std::time::Instant; use crate::frame::{Frame, FrameType}; +use crate::packet::PacketBuilder; use crate::recovery::RecoveryToken; use crate::{CloseError, ConnectionError}; @@ -70,6 +71,8 @@ impl PartialOrd for State { } } +type ClosingFrame = Frame<'static>; + /// `StateSignaling` manages whether we need to send HANDSHAKE_DONE and CONNECTION_CLOSE. /// Valid state transitions are: /// * Idle -> HandshakeDone: at the server when the handshake completes @@ -83,11 +86,11 @@ pub enum StateSignaling { Idle, HandshakeDone, /// These states save the frame that needs to be sent. - Closing(Frame), - Draining(Frame), + Closing(ClosingFrame), + Draining(ClosingFrame), /// This state saves the frame that might need to be sent again. /// If it is `None`, then we are draining and don't send. - CloseSent(Option), + CloseSent(Option), Reset, } @@ -100,10 +103,11 @@ impl StateSignaling { *self = Self::HandshakeDone } - pub fn send_done(&mut self) -> Option<(Frame, Option)> { - if *self == Self::HandshakeDone { + pub fn write_done(&mut self, builder: &mut PacketBuilder) -> Option { + if *self == Self::HandshakeDone && builder.remaining() >= 1 { *self = Self::Idle; - Some((Frame::HandshakeDone, Some(RecoveryToken::HandshakeDone))) + builder.encode_varint(Frame::HandshakeDone.get_type()); + Some(RecoveryToken::HandshakeDone) } else { None } @@ -113,7 +117,7 @@ impl StateSignaling { error: ConnectionError, frame_type: FrameType, message: impl AsRef, - ) -> Frame { + ) -> ClosingFrame { let reason_phrase = message.as_ref().as_bytes().to_owned(); Frame::ConnectionClose { error_code: CloseError::from(error), @@ -145,7 +149,7 @@ impl StateSignaling { } /// If a close is pending, take a frame. - pub fn close_frame(&mut self) -> Option { + pub fn close_frame(&mut self) -> Option { match self { Self::Closing(frame) => { // When we are closing, we might need to send the close frame again. diff --git a/third_party/rust/neqo-transport/src/connection/tests/cc.rs b/third_party/rust/neqo-transport/src/connection/tests/cc.rs index 7042aa0dc7fe..5f793accc46a 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/cc.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/cc.rs @@ -10,13 +10,13 @@ use super::{ default_server, fill_cwnd, send_something, AT_LEAST_PTO, DEFAULT_RTT, POST_HANDSHAKE_CWND, }; use crate::cc::{CWND_MIN, MAX_DATAGRAM_SIZE}; -use crate::frame::{Frame, StreamType}; +use crate::frame::StreamType; use crate::packet::PacketNumber; use crate::recovery::{ACK_ONLY_SIZE_LIMIT, PACKET_THRESHOLD}; use crate::sender::PACING_BURST_SIZE; use crate::stats::MAX_PTO_COUNTS; use crate::tparams::{self, TransportParameter}; -use crate::tracking::{PNSpace, MAX_UNACKED_PKTS}; +use crate::tracking::MAX_UNACKED_PKTS; use neqo_common::{qdebug, qinfo, qtrace, Datagram}; use std::convert::TryFrom; @@ -64,7 +64,7 @@ fn induce_persistent_congestion( assert_eq!(client.stats.borrow().pto_counts, pto_counts); // Generate ACK - let (s_tx_dgram, _) = ack_bytes(server, 0, c_tx_dgrams, now); + let s_tx_dgram = ack_bytes(server, 0, c_tx_dgrams, now); // An ACK for the third PTO causes persistent congestion. for dgram in s_tx_dgram { @@ -76,23 +76,17 @@ fn induce_persistent_congestion( } // Receive multiple packets and generate an ack-only packet. -fn ack_bytes( - dest: &mut Connection, - stream: u64, - in_dgrams: D, - now: Instant, -) -> (Vec, Vec) +fn ack_bytes(dest: &mut Connection, stream: u64, in_dgrams: D, now: Instant) -> Vec where D: IntoIterator, D::IntoIter: ExactSizeIterator, { let mut srv_buf = [0; 4_096]; - let mut recvd_frames = Vec::new(); let in_dgrams = in_dgrams.into_iter(); qdebug!([dest], "ack_bytes {} datagrams", in_dgrams.len()); for dgram in in_dgrams { - recvd_frames.extend(dest.test_process_input(dgram, now)); + dest.process_input(dgram, now); } loop { @@ -109,11 +103,7 @@ where } assert!((tx_dgrams.len() == 1) || (tx_dgrams.len() == 2)); - - ( - tx_dgrams, - recvd_frames.into_iter().map(|(f, _e)| f).collect(), - ) + tx_dgrams } #[test] @@ -157,27 +147,21 @@ fn cc_slow_start_to_cong_avoidance_recovery_period() { // Server: Receive and generate ack now += DEFAULT_RTT / 2; - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams, now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams, now); + assert_eq!( + server.stats().frame_tx.largest_acknowledged, + flight1_largest + ); // Client: Process ack now += DEFAULT_RTT / 2; for dgram in s_tx_dgram { - let recvd_frames = client.test_process_input(dgram, now); - - // Verify that server-sent frame was what we thought. - if let ( - Frame::Ack { - largest_acknowledged, - .. - }, - PNSpace::ApplicationData, - ) = recvd_frames[0] - { - assert_eq!(largest_acknowledged, flight1_largest); - } else { - panic!("Expected an application ACK"); - } + client.process_input(dgram, now); } + assert_eq!( + client.stats().frame_rx.largest_acknowledged, + flight1_largest + ); // Client: send more let (mut c_tx_dgrams, mut now) = fill_cwnd(&mut client, 0, now); @@ -187,27 +171,21 @@ fn cc_slow_start_to_cong_avoidance_recovery_period() { // Server: Receive and generate ack again, but drop first packet now += DEFAULT_RTT / 2; c_tx_dgrams.remove(0); - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams, now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams, now); + assert_eq!( + server.stats().frame_tx.largest_acknowledged, + flight2_largest + ); // Client: Process ack now += DEFAULT_RTT / 2; for dgram in s_tx_dgram { - let recvd_frames = client.test_process_input(dgram, now); - - // Verify that server-sent frame was what we thought. - if let ( - Frame::Ack { - largest_acknowledged, - .. - }, - PNSpace::ApplicationData, - ) = recvd_frames[0] - { - assert_eq!(largest_acknowledged, flight2_largest); - } else { - panic!("Expected an application ACK"); - } + client.process_input(dgram, now); } + assert_eq!( + client.stats().frame_rx.largest_acknowledged, + flight2_largest + ); } #[test] @@ -231,7 +209,7 @@ fn cc_cong_avoidance_recovery_period_unchanged() { let c_tx_dgrams2 = c_tx_dgrams.split_off(5); // Server: Receive and generate ack - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams, now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams, now); for dgram in s_tx_dgram { client.process_input(dgram, now); } @@ -239,7 +217,7 @@ fn cc_cong_avoidance_recovery_period_unchanged() { let cwnd1 = client.loss_recovery.cwnd(); // Generate ACK for more received packets - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams2, now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams2, now); // ACK more packets but they were sent before end of recovery period for dgram in s_tx_dgram { @@ -305,7 +283,7 @@ fn cc_cong_avoidance_recovery_period_to_cong_avoidance() { // Server: Receive and generate ack now += DEFAULT_RTT / 2; - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams, now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams, now); // Client: Process ack now += DEFAULT_RTT / 2; @@ -342,12 +320,12 @@ fn cc_cong_avoidance_recovery_period_to_cong_avoidance() { // Note that we need the client to process ACK frames in stages, so split the // datagrams into two, ensuring that we allow for an ACK for each batch. let most = c_tx_dgrams.len() - MAX_UNACKED_PKTS - 1; - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams.drain(..most), now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams.drain(..most), now); for dgram in s_tx_dgram { assert_eq!(client.loss_recovery.cwnd(), expected_cwnd); client.process_input(dgram, now); } - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams, now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams, now); for dgram in s_tx_dgram { assert_eq!(client.loss_recovery.cwnd(), expected_cwnd); client.process_input(dgram, now); @@ -373,7 +351,7 @@ fn cc_slow_start_to_persistent_congestion_no_acks() { // Server: Receive and generate ack now += DEFAULT_RTT / 2; - let (_s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams, now); + let _ = ack_bytes(&mut server, 0, c_tx_dgrams, now); // ACK lost. induce_persistent_congestion(&mut client, &mut server, now); @@ -395,7 +373,7 @@ fn cc_slow_start_to_persistent_congestion_some_acks() { // Server: Receive and generate ack now += Duration::from_millis(100); - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams, now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams, now); now += Duration::from_millis(100); for dgram in s_tx_dgram { @@ -442,7 +420,7 @@ fn cc_persistent_congestion_to_slow_start() { // Server: Receive and generate ack now = next_now + Duration::from_millis(100); - let (s_tx_dgram, _) = ack_bytes(&mut server, 0, c_tx_dgrams, now); + let s_tx_dgram = ack_bytes(&mut server, 0, c_tx_dgrams, now); // No longer in CARP. (pkts acked from after start of CARP) // Should be in slow start now. @@ -487,12 +465,9 @@ fn ack_are_not_cc() { let ack_pkt = client.process(ack_eliciting_packet, now).dgram(); assert!(ack_pkt.is_some()); qdebug!([server], "Handle ACK"); - let frames = server.test_process_input(ack_pkt.unwrap(), now); - assert_eq!(frames.len(), 1); - assert!(matches!( - frames[0], - (Frame::Ack { .. }, PNSpace::ApplicationData) - )); + let prev_ack_count = server.stats().frame_rx.ack; + server.process_input(ack_pkt.unwrap(), now); + assert_eq!(server.stats().frame_rx.ack, prev_ack_count + 1); } #[test] diff --git a/third_party/rust/neqo-transport/src/connection/tests/close.rs b/third_party/rust/neqo-transport/src/connection/tests/close.rs index b827fd2a1554..088816f95ede 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/close.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/close.rs @@ -4,17 +4,28 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use super::super::{Output, State}; +use super::super::{Connection, Output, State}; use super::{connect, connect_force_idle, default_client, default_server, send_something}; -use crate::frame::{CloseError, Frame}; use crate::tparams::{self, TransportParameter}; -use crate::tracking::PNSpace; -use crate::{AppError, ConnectionError, Error}; +use crate::{AppError, ConnectionError, Error, ERROR_APPLICATION_CLOSE}; use neqo_common::Datagram; use std::time::Duration; use test_fixture::{self, loopback, now}; +fn assert_draining(c: &Connection, expected: &Error) { + assert!(c.state().closed()); + if let State::Draining { + error: ConnectionError::Transport(error), + .. + } = c.state() + { + assert_eq!(error, expected); + } else { + panic!(); + } +} + #[test] fn connection_close() { let mut client = default_client(); @@ -27,18 +38,8 @@ fn connection_close() { let out = client.process(None, now); - let frames = server.test_process_input(out.dgram().unwrap(), now); - assert_eq!(frames.len(), 1); - assert!(matches!( - frames[0], - ( - Frame::ConnectionClose { - error_code: CloseError::Application(42), - .. - }, - PNSpace::ApplicationData, - ) - )); + server.process_input(out.dgram().unwrap(), now); + assert_draining(&server, &Error::PeerApplicationError(42)); } // During the handshake, an application close should be sanitized. @@ -58,18 +59,8 @@ fn early_application_close() { let dgram = server.process(None, now()).dgram(); assert!(dgram.is_some()); - let frames = client.test_process_input(dgram.unwrap(), now()); - assert!(matches!( - frames[0], - ( - Frame::ConnectionClose { - error_code: CloseError::Transport(code), - .. - }, - PNSpace::Initial, - ) if code == Error::ApplicationError.code() - )); - assert!(client.state().closed()); + client.process_input(dgram.unwrap(), now()); + assert_draining(&client, &Error::PeerError(ERROR_APPLICATION_CLOSE)); } #[test] @@ -82,6 +73,7 @@ fn bad_tls_version() { .set_option(neqo_crypto::Opt::Tls13CompatMode, true) .unwrap(); let mut server = default_server(); + let dgram = client.process(None, now()).dgram(); assert!(dgram.is_some()); let dgram = server.process(dgram, now()).dgram(); @@ -90,17 +82,8 @@ fn bad_tls_version() { State::Closed(ConnectionError::Transport(Error::ProtocolViolation)) ); assert!(dgram.is_some()); - let frames = client.test_process_input(dgram.unwrap(), now()); - assert!(matches!( - frames[0], - ( - Frame::ConnectionClose { - error_code: CloseError::Transport(_), - .. - }, - PNSpace::Initial, - ) - )); + client.process_input(dgram.unwrap(), now()); + assert_draining(&client, &Error::PeerError(Error::ProtocolViolation.code())); } /// Test the interaction between the loss recovery timer @@ -202,5 +185,5 @@ fn stateless_reset_client() { connect_force_idle(&mut client, &mut server); client.process_input(Datagram::new(loopback(), loopback(), vec![77; 21]), now()); - assert!(matches!(client.state(), State::Draining { .. })); + assert_draining(&client, &Error::StatelessReset); } diff --git a/third_party/rust/neqo-transport/src/connection/tests/handshake.rs b/third_party/rust/neqo-transport/src/connection/tests/handshake.rs index 2fc35ca29b1c..b9a7b13fc571 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/handshake.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/handshake.rs @@ -7,8 +7,7 @@ use super::super::{Connection, FixedConnectionIdManager, Output, State, LOCAL_IDLE_TIMEOUT}; use super::{ assert_error, connect_force_idle, connect_with_rtt, default_client, default_server, get_tokens, - handshake, maybe_authenticate, send_something, split_datagram, AT_LEAST_PTO, DEFAULT_RTT, - DEFAULT_STREAM_DATA, + handshake, maybe_authenticate, send_something, AT_LEAST_PTO, DEFAULT_RTT, DEFAULT_STREAM_DATA, }; use crate::connection::AddressValidation; use crate::events::ConnectionEvent; @@ -22,7 +21,7 @@ use neqo_crypto::{constants::TLS_CHACHA20_POLY1305_SHA256, AuthenticationStatus} use std::cell::RefCell; use std::rc::Rc; use std::time::Duration; -use test_fixture::{self, assertions, fixture_init, loopback, now}; +use test_fixture::{self, assertions, fixture_init, loopback, now, split_datagram}; #[test] fn full_handshake() { @@ -31,18 +30,16 @@ fn full_handshake() { let out = client.process(None, now()); assert!(out.as_dgram_ref().is_some()); assert_eq!(out.as_dgram_ref().unwrap().len(), PATH_MTU_V6); - qdebug!("Output={:0x?}", out.as_dgram_ref()); qdebug!("---- server: CH -> SH, EE, CERT, CV, FIN"); let mut server = default_server(); let out = server.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); + assert_eq!(out.as_dgram_ref().unwrap().len(), PATH_MTU_V6); qdebug!("---- client: cert verification"); let out = client.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); let out = server.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_none()); @@ -52,19 +49,16 @@ fn full_handshake() { qdebug!("---- client: SH..FIN -> FIN"); let out = client.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); assert_eq!(*client.state(), State::Connected); qdebug!("---- server: FIN -> ACKS"); let out = server.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); assert_eq!(*server.state(), State::Confirmed); qdebug!("---- client: ACKS -> 0"); let out = client.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_none()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); assert_eq!(*client.state(), State::Confirmed); } @@ -74,22 +68,18 @@ fn handshake_failed_authentication() { let mut client = default_client(); let out = client.process(None, now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); qdebug!("---- server: CH -> SH, EE, CERT, CV, FIN"); let mut server = default_server(); let out = server.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); qdebug!("---- client: cert verification"); let out = client.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); let out = server.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_none()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); let authentication_needed = |e| matches!(e, ConnectionEvent::AuthenticationNeeded); assert!(client.events().any(authentication_needed)); @@ -99,12 +89,10 @@ fn handshake_failed_authentication() { qdebug!("---- client: -> Alert(certificate_revoked)"); let out = client.process(None, now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); qdebug!("---- server: Alert(certificate_revoked)"); let out = server.process(out.dgram(), now()); assert!(out.as_dgram_ref().is_some()); - qdebug!("Output={:0x?}", out.as_dgram_ref()); assert_error(&client, &ConnectionError::Transport(Error::CryptoAlert(44))); assert_error(&server, &ConnectionError::Transport(Error::PeerError(300))); } @@ -164,8 +152,9 @@ fn dup_server_flight1() { assert!(out.as_dgram_ref().is_some()); qdebug!("Output={:0x?}", out.as_dgram_ref()); - assert_eq!(2, client.stats().packets_rx); + assert_eq!(3, client.stats().packets_rx); assert_eq!(0, client.stats().dups_rx); + assert_eq!(1, client.stats().dropped_rx); qdebug!("---- Dup, ignored"); let out = client.process(out_to_rep.dgram(), now()); @@ -173,10 +162,10 @@ fn dup_server_flight1() { qdebug!("Output={:0x?}", out.as_dgram_ref()); // Four packets total received, 1 of them is a dup and one has been dropped because Initial keys - // are dropped. - assert_eq!(4, client.stats().packets_rx); + // are dropped. Add 2 counts of the padding that the server adds to Initial packets. + assert_eq!(6, client.stats().packets_rx); assert_eq!(1, client.stats().dups_rx); - assert_eq!(1, client.stats().dropped_rx); + assert_eq!(3, client.stats().dropped_rx); } // Test that we split crypto data if they cannot fit into one packet. @@ -445,7 +434,7 @@ fn coalesce_05rtt() { maybe_authenticate(&mut client); let c3 = client.process(None, now).dgram(); assert!(c3.is_some()); - assert_eq!(client.stats().dropped_rx, 0); + assert_eq!(client.stats().dropped_rx, 1); // Just Initial padding. assert_eq!(client.stats().packets_rx, 4); assert_eq!(client.stats().saved_datagrams, 1); @@ -458,7 +447,7 @@ fn coalesce_05rtt() { let _ = client.process(s3, now).dgram(); assert_eq!(*client.state(), State::Confirmed); - assert_eq!(client.stats().dropped_rx, 0); + assert_eq!(client.stats().dropped_rx, 1); // Just Initial padding. } #[test] @@ -507,7 +496,7 @@ fn reorder_handshake() { client.process_input(s_init, now); // Each saved packet should now be "received" again. - assert_eq!(client.stats().packets_rx, 5); + assert_eq!(client.stats().packets_rx, 7); maybe_authenticate(&mut client); let c3 = client.process(None, now).dgram(); assert!(c3.is_some()); @@ -611,8 +600,8 @@ fn corrupted_initial() { // The server should have received two packets, // the first should be dropped, the second saved. assert_eq!(server.stats().packets_rx, 2); - assert_eq!(server.stats().dropped_rx, 1); - assert_eq!(server.stats().saved_datagrams, 1); + assert_eq!(server.stats().dropped_rx, 2); + assert_eq!(server.stats().saved_datagrams, 0); } #[test] diff --git a/third_party/rust/neqo-transport/src/connection/tests/idle.rs b/third_party/rust/neqo-transport/src/connection/tests/idle.rs index 4909d80a6ba0..962645eff3c4 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/idle.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/idle.rs @@ -7,14 +7,16 @@ use super::super::{IdleTimeout, Output, State, LOCAL_IDLE_TIMEOUT}; use super::{ connect, connect_force_idle, connect_with_rtt, default_client, default_server, - maybe_authenticate, send_something, split_datagram, AT_LEAST_PTO, + maybe_authenticate, send_something, AT_LEAST_PTO, }; -use crate::frame::{Frame, StreamType}; +use crate::frame::StreamType; +use crate::packet::PacketBuilder; use crate::tparams::{self, TransportParameter}; use crate::tracking::PNSpace; +use neqo_common::Encoder; use std::time::Duration; -use test_fixture::{self, now}; +use test_fixture::{self, now, split_datagram}; #[test] fn idle_timeout() { @@ -215,6 +217,7 @@ fn idle_caching() { let mut client = default_client(); let mut server = default_server(); let start = now(); + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); // Perform the first round trip, but drop the Initial from the server. // The client then caches the Handshake packet. @@ -233,21 +236,29 @@ fn idle_caching() { let _ = server.process_output(middle).dgram(); // Now let the server process the client PING. This causes the server // to send CRYPTO frames again, so manually extract and discard those. - let frames = server.test_process_input(dgram.unwrap(), middle); - assert_eq!(frames, vec![(Frame::Ping, PNSpace::Initial)]); - let crypto = server.crypto.streams.get_frame(PNSpace::Initial, 1000); + let ping_before_s = server.stats().frame_rx.ping; + server.process_input(dgram.unwrap(), middle); + assert_eq!(server.stats().frame_rx.ping, ping_before_s + 1); + let crypto = server + .crypto + .streams + .write_frame(PNSpace::Initial, &mut builder); assert!(crypto.is_some()); - let crypto = server.crypto.streams.get_frame(PNSpace::Initial, 1000); + let crypto = server + .crypto + .streams + .write_frame(PNSpace::Initial, &mut builder); assert!(crypto.is_none()); let dgram = server.process_output(middle).dgram(); // Now only allow the Initial packet from the server through; // it shouldn't contain a CRYPTO frame. let (initial, _) = split_datagram(&dgram.unwrap()); - let frames = client.test_process_input(initial, middle); - assert_eq!(frames.len(), 2); - assert_eq!(frames[0], (Frame::Ping, PNSpace::Initial)); - assert!(matches!(frames[1], (Frame::Ack { .. }, PNSpace::Initial))); + let ping_before_c = client.stats().frame_rx.ping; + let ack_before = client.stats().frame_rx.ack; + client.process_input(initial, middle); + assert_eq!(client.stats().frame_rx.ping, ping_before_c + 1); + assert_eq!(client.stats().frame_rx.ack, ack_before + 1); let end = start + LOCAL_IDLE_TIMEOUT + (AT_LEAST_PTO / 2); // Now let the server Initial through, with the CRYPTO frame. diff --git a/third_party/rust/neqo-transport/src/connection/tests/keys.rs b/third_party/rust/neqo-transport/src/connection/tests/keys.rs index 01f7be478970..cc572e85ca88 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/keys.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/keys.rs @@ -62,8 +62,8 @@ fn discarded_initial_keys() { // The client has received handshake packet. It will remove the Initial keys. // We will check this by processing init_pkt_s a second time. // The initial packet should be dropped. The packet contains a Handshake packet as well, which - // will be marked as dup. - check_discarded(&mut client, init_pkt_s.unwrap(), 1, 1); + // will be marked as dup. And it will contain padding, which will be "dropped". + check_discarded(&mut client, init_pkt_s.unwrap(), 2, 1); assert!(maybe_authenticate(&mut client)); diff --git a/third_party/rust/neqo-transport/src/connection/tests/mod.rs b/third_party/rust/neqo-transport/src/connection/tests/mod.rs index fe08fc122f04..6a7b73a514f5 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/mod.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/mod.rs @@ -22,7 +22,7 @@ use std::mem; use std::rc::Rc; use std::time::{Duration, Instant}; -use neqo_common::{event::Provider, qdebug, qtrace, Datagram, Decoder}; +use neqo_common::{event::Provider, qdebug, qtrace, Datagram}; use neqo_crypto::{AllowZeroRtt, AuthenticationStatus, ResumptionToken}; use test_fixture::{self, fixture_init, loopback, now}; @@ -297,40 +297,6 @@ fn send_and_receive( receiver.process(Some(dgram), now).dgram() } -/// Split the first packet off a coalesced packet. -fn split_packet(buf: &[u8]) -> (&[u8], Option<&[u8]>) { - if buf[0] & 0x80 == 0 { - // Short header: easy. - return (buf, None); - } - let mut dec = Decoder::from(buf); - let first = dec.decode_byte().unwrap(); - assert_ne!(first & 0b0011_0000, 0b0011_0000, "retry not supported"); - dec.skip(4); // Version. - dec.skip_vec(1); // DCID - dec.skip_vec(1); // SCID - if first & 0b0011_0000 == 0 { - dec.skip_vvec(); // Initial token - } - dec.skip_vvec(); // The rest of the packet. - let p1 = &buf[..dec.offset()]; - let p2 = if dec.remaining() > 0 { - Some(dec.decode_remainder()) - } else { - None - }; - (p1, p2) -} - -/// Split the first datagram off a coalesced datagram. -fn split_datagram(d: &Datagram) -> (Datagram, Option) { - let (a, b) = split_packet(&d[..]); - ( - Datagram::new(d.source(), d.destination(), a), - b.map(|b| Datagram::new(d.source(), d.destination(), b)), - ) -} - fn get_tokens(client: &mut Connection) -> Vec { client .events() diff --git a/third_party/rust/neqo-transport/src/connection/tests/recovery.rs b/third_party/rust/neqo-transport/src/connection/tests/recovery.rs index ede3044bacd3..ba7069ccb2da 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/recovery.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/recovery.rs @@ -7,20 +7,20 @@ use super::super::{Output, State, LOCAL_IDLE_TIMEOUT}; use super::{ assert_full_cwnd, connect, connect_force_idle, connect_with_rtt, default_client, - default_server, fill_cwnd, maybe_authenticate, send_and_receive, send_something, - split_datagram, AT_LEAST_PTO, POST_HANDSHAKE_CWND, + default_server, fill_cwnd, maybe_authenticate, send_and_receive, send_something, AT_LEAST_PTO, + POST_HANDSHAKE_CWND, }; -use crate::frame::{Frame, StreamType}; +use crate::frame::StreamType; use crate::path::PATH_MTU_V6; use crate::recovery::PTO_PACKET_COUNT; use crate::stats::MAX_PTO_COUNTS; use crate::tparams::TransportParameter; -use crate::tracking::{PNSpace, ACK_DELAY}; +use crate::tracking::ACK_DELAY; use neqo_common::qdebug; use neqo_crypto::AuthenticationStatus; use std::time::Duration; -use test_fixture::{self, now}; +use test_fixture::{self, now, split_datagram}; #[test] fn pto_works_basic() { @@ -54,16 +54,9 @@ fn pto_works_basic() { now += AT_LEAST_PTO; let out = client.process(None, now); - let frames = server.test_process_input(out.dgram().unwrap(), now); - - assert!(frames.iter().all(|(_, sp)| *sp == PNSpace::ApplicationData)); - assert!(frames.iter().any(|(f, _)| *f == Frame::Ping)); - assert!(frames - .iter() - .any(|(f, _)| matches!(f, Frame::Stream { stream_id, .. } if stream_id.as_u64() == 2))); - assert!(frames - .iter() - .any(|(f, _)| matches!(f, Frame::Stream { stream_id, .. } if stream_id.as_u64() == 6))); + let stream_before = server.stats().frame_rx.stream; + server.process_input(out.dgram().unwrap(), now); + assert_eq!(server.stats().frame_rx.stream, stream_before + 2); } #[test] @@ -80,28 +73,19 @@ fn pto_works_full_cwnd() { let (dgrams, now) = fill_cwnd(&mut client, 2, now()); assert_full_cwnd(&dgrams, POST_HANDSHAKE_CWND); + neqo_common::qwarn!("waiting over"); // Fill the CWND after waiting for a PTO. let (dgrams, now) = fill_cwnd(&mut client, 2, now + AT_LEAST_PTO); - assert_eq!(dgrams.len(), 2); // Two packets in the PTO. + // Two packets in the PTO. + // The first should be full sized; the second might be small. + assert_eq!(dgrams.len(), 2); + assert_eq!(dgrams[0].len(), PATH_MTU_V6); - // All (2) datagrams contain one PING frame and at least one STREAM frame. + // Both datagrams contain a STREAM frame. for d in dgrams { - assert_eq!(d.len(), PATH_MTU_V6); - let frames = server.test_process_input(d, now); - assert_eq!( - frames - .iter() - .filter(|i| matches!(i, (Frame::Ping, PNSpace::ApplicationData))) - .count(), - 1 - ); - assert!( - frames - .iter() - .filter(|i| matches!(i, (Frame::Stream { .. }, PNSpace::ApplicationData))) - .count() - >= 1 - ); + let stream_before = server.stats().frame_rx.stream; + server.process_input(d, now); + assert_eq!(server.stats().frame_rx.stream, stream_before + 1); } } @@ -188,12 +172,12 @@ fn pto_works_ping() { now + Duration::from_secs(10) + Duration::from_millis(110), ); - let frames = server.test_process_input( + let ping_before = server.stats().frame_rx.ping; + server.process_input( pkt6.dgram().unwrap(), now + Duration::from_secs(10) + Duration::from_millis(110), ); - - assert_eq!(frames[0], (Frame::Ping, PNSpace::ApplicationData)); + assert_eq!(server.stats().frame_rx.ping, ping_before + 1); } #[test] @@ -313,14 +297,15 @@ fn pto_handshake_complete() { // Check that the PTO packets (pkt2, pkt3) are Handshake packets. // The server discarded the Handshake keys already, therefore they are dropped. let dropped_before1 = server.stats().dropped_rx; - let frames = server.test_process_input(pkt2.unwrap(), now); + let frames_before = server.stats().frame_rx.all; + server.process_input(pkt2.unwrap(), now); assert_eq!(1, server.stats().dropped_rx - dropped_before1); - assert!(frames.is_empty()); + assert_eq!(server.stats().frame_rx.all, frames_before); let dropped_before2 = server.stats().dropped_rx; - let frames = server.test_process_input(pkt3.unwrap(), now); + server.process_input(pkt3.unwrap(), now); assert_eq!(1, server.stats().dropped_rx - dropped_before2); - assert!(frames.is_empty()); + assert_eq!(server.stats().frame_rx.all, frames_before); now += Duration::from_millis(10); // Client receive ack for the first packet @@ -381,14 +366,9 @@ fn pto_handshake_frames() { assert!(pkt2.is_some()); now += Duration::from_millis(10); - let frames = server.test_process_input(pkt2.unwrap(), now); - - assert_eq!(frames.len(), 2); - assert_eq!(frames[0], (Frame::Ping, PNSpace::Handshake)); - assert!(matches!( - frames[1], - (Frame::Crypto { .. }, PNSpace::Handshake) - )); + let crypto_before = server.stats().frame_rx.crypto; + server.process_input(pkt2.unwrap(), now); + assert_eq!(server.stats().frame_rx.crypto, crypto_before + 1) } /// In the case that the Handshake takes too many packets, the server might @@ -432,8 +412,9 @@ fn handshake_ack_pto() { assert!(c3.is_some()); now += RTT / 2; - let frames = server.test_process_input(c3.unwrap(), now); - assert_eq!(frames, vec![(Frame::Ping, PNSpace::Handshake)]); + let ping_before = server.stats().frame_rx.ping; + server.process_input(c3.unwrap(), now); + assert_eq!(server.stats().frame_rx.ping, ping_before + 1); pto_counts[0] = 1; assert_eq!(client.stats.borrow().pto_counts, pto_counts); @@ -514,13 +495,12 @@ fn ack_after_pto() { let ack = client.process(Some(dgram), now).dgram(); assert!(ack.is_some()); - // Make sure that the packet only contained ACK frames. - let frames = server.test_process_input(ack.unwrap(), now); - assert_eq!(frames.len(), 1); - for (frame, space) in frames { - assert_eq!(space, PNSpace::ApplicationData); - assert!(matches!(frame, Frame::Ack { .. })); - } + // Make sure that the packet only contained an ACK frame. + let all_frames_before = server.stats().frame_rx.all; + let ack_before = server.stats().frame_rx.ack; + server.process_input(ack.unwrap(), now); + assert_eq!(server.stats().frame_rx.all, all_frames_before + 1); + assert_eq!(server.stats().frame_rx.ack, ack_before + 1); } /// When we declare a packet as lost, we keep it around for a while for another loss period. diff --git a/third_party/rust/neqo-transport/src/connection/tests/stream.rs b/third_party/rust/neqo-transport/src/connection/tests/stream.rs index c90483eba1b0..1c9eebaa178c 100644 --- a/third_party/rust/neqo-transport/src/connection/tests/stream.rs +++ b/third_party/rust/neqo-transport/src/connection/tests/stream.rs @@ -5,14 +5,17 @@ // except according to those terms. use super::super::State; -use super::{connect, default_client, default_server, maybe_authenticate, send_something}; +use super::{ + connect, default_client, default_server, maybe_authenticate, send_something, + DEFAULT_STREAM_DATA, +}; use crate::events::ConnectionEvent; -use crate::frame::{Frame, StreamType}; +use crate::frame::StreamType; use crate::recv_stream::RECV_BUFFER_SIZE; use crate::send_stream::SEND_BUFFER_SIZE; use crate::tparams::{self, TransportParameter}; use crate::tracking::MAX_UNACKED_PKTS; -use crate::Error; +use crate::{Error, StreamId}; use neqo_common::{event::Provider, qdebug}; use std::convert::TryFrom; @@ -423,30 +426,46 @@ fn stream_data_blocked_generates_max_stream_data() { let now = now(); - // Try to say we're blocked beyond the initial data window - server - .flow_mgr - .borrow_mut() - .stream_data_blocked(3.into(), RECV_BUFFER_SIZE as u64 * 4); + // Send some data and include STREAM_DATA_BLOCKED with any value. + let stream_id = server.stream_create(StreamType::UniDi).unwrap(); + let _ = server.stream_send(stream_id, DEFAULT_STREAM_DATA).unwrap(); + server.flow_mgr.borrow_mut().stream_data_blocked( + StreamId::from(stream_id), + u64::try_from(DEFAULT_STREAM_DATA.len()).unwrap(), + ); - let out = server.process(None, now); - assert!(out.as_dgram_ref().is_some()); + let dgram = server.process(None, now).dgram(); + assert!(dgram.is_some()); - let frames = client.test_process_input(out.dgram().unwrap(), now); - assert!(frames - .iter() - .any(|(f, _)| matches!(f, Frame::StreamDataBlocked { .. }))); + let sdb_before = client.stats().frame_rx.stream_data_blocked; + client.process_input(dgram.unwrap(), now); + assert_eq!(client.stats().frame_rx.stream_data_blocked, sdb_before + 1); - let out = client.process_output(now); - assert!(out.as_dgram_ref().is_some()); + // Consume the data. + let mut buf = [0; 10]; + let (count, end) = client.stream_recv(stream_id, &mut buf[..]).unwrap(); + assert_eq!(count, DEFAULT_STREAM_DATA.len()); + assert!(!end); - let frames = server.test_process_input(out.dgram().unwrap(), now); - // Client should have sent a MaxStreamData frame with just the initial - // window value. - assert!(frames.iter().any( - |(f, _)| matches!(f, Frame::MaxStreamData { maximum_stream_data, .. } - if *maximum_stream_data == RECV_BUFFER_SIZE as u64) - )); + let dgram = client.process_output(now).dgram(); + + // Client should have sent a MAX_STREAM_DATA frame with just a small increase + // on the default window size. + let msd_before = server.stats().frame_rx.max_stream_data; + server.process_input(dgram.unwrap(), now); + assert_eq!(server.stats().frame_rx.max_stream_data, msd_before + 1); + + // Test that more space is available, but that it is small. + let mut written = 0; + loop { + const LARGE_BUFFER: &[u8] = &[0; 1024]; + let amount = server.stream_send(stream_id, LARGE_BUFFER).unwrap(); + if amount == 0 { + break; + } + written += amount; + } + assert_eq!(written, RECV_BUFFER_SIZE - DEFAULT_STREAM_DATA.len()); } /// See diff --git a/third_party/rust/neqo-transport/src/crypto.rs b/third_party/rust/neqo-transport/src/crypto.rs index 208fe05edff0..f5c5abec0ed4 100644 --- a/third_party/rust/neqo-transport/src/crypto.rs +++ b/third_party/rust/neqo-transport/src/crypto.rs @@ -5,7 +5,8 @@ // except according to those terms. use std::cell::RefCell; -use std::cmp::max; +use std::cmp::{max, min}; +use std::convert::TryFrom; use std::mem; use std::ops::{Index, IndexMut, Range}; use std::rc::Rc; @@ -20,8 +21,7 @@ use neqo_crypto::{ TLS_VERSION_1_3, }; -use crate::frame::Frame; -use crate::packet::{PacketNumber, QuicVersion}; +use crate::packet::{PacketBuilder, PacketNumber, QuicVersion}; use crate::recovery::RecoveryToken; use crate::recv_stream::RxStreamOrderer; use crate::send_stream::TxBuffer; @@ -1149,8 +1149,8 @@ impl CryptoStreams { self.get_mut(space).unwrap().tx.send(data); } - pub fn inbound_frame(&mut self, space: PNSpace, offset: u64, data: Vec) -> Res<()> { - self.get_mut(space).unwrap().rx.inbound_frame(offset, data) + pub fn inbound_frame(&mut self, space: PNSpace, offset: u64, data: &[u8]) { + self.get_mut(space).unwrap().rx.inbound_frame(offset, data); } pub fn data_ready(&self, space: PNSpace) -> bool { @@ -1225,30 +1225,38 @@ impl CryptoStreams { } } - pub fn get_frame( + pub fn write_frame( &mut self, space: PNSpace, - remaining: usize, - ) -> Option<(Frame, Option)> { + builder: &mut PacketBuilder, + ) -> Option { let cs = self.get_mut(space).unwrap(); if let Some((offset, data)) = cs.tx.next_bytes() { - let (frame, length) = Frame::new_crypto(offset, data, remaining); + let mut header_len = 1 + Encoder::varint_len(offset) + 1; + + // Don't bother if there isn't room for the header and some data. + if builder.remaining() < header_len + 1 { + return None; + } + // Calculate length of data based on the minimum of: + // - available data + // - remaining space, less the header, which counts only one byte + // for the length at first to avoid underestimating length + let length = min(data.len(), builder.remaining() - header_len); + header_len += Encoder::varint_len(u64::try_from(length).unwrap()) - 1; + let length = min(data.len(), builder.remaining() - header_len); + + builder.encode_varint(crate::frame::FRAME_TYPE_CRYPTO); + builder.encode_varint(offset); + builder.encode_vvec(&data[..length]); cs.tx.mark_as_sent(offset, length); - qdebug!( - "Emitting crypto frame space={}, offset={}, len={}", + qdebug!("CRYPTO for {} offset={}, len={}", space, offset, length); + Some(RecoveryToken::Crypto(CryptoRecoveryToken { space, offset, - length - ); - Some(( - frame, - Some(RecoveryToken::Crypto(CryptoRecoveryToken { - space, - offset, - length, - })), - )) + length, + })) } else { None } diff --git a/third_party/rust/neqo-transport/src/flow_mgr.rs b/third_party/rust/neqo-transport/src/flow_mgr.rs index f77b7026af90..fb464f21b593 100644 --- a/third_party/rust/neqo-transport/src/flow_mgr.rs +++ b/third_party/rust/neqo-transport/src/flow_mgr.rs @@ -10,30 +10,33 @@ use std::collections::HashMap; use std::mem; -use neqo_common::{qinfo, qtrace, qwarn, Encoder}; +use neqo_common::{qinfo, qwarn, Encoder}; +use smallvec::{smallvec, SmallVec}; use crate::frame::{Frame, StreamType}; +use crate::packet::PacketBuilder; use crate::recovery::RecoveryToken; use crate::recv_stream::RecvStreams; use crate::send_stream::SendStreams; +use crate::stats::FrameStats; use crate::stream_id::{StreamId, StreamIndex, StreamIndexes}; -use crate::tracking::PNSpace; use crate::AppError; -pub type FlowControlRecoveryToken = Frame; +type FlowFrame = Frame<'static>; +pub type FlowControlRecoveryToken = FlowFrame; #[derive(Debug, Default)] pub struct FlowMgr { // Discriminant as key ensures only 1 of every frame type will be queued. - from_conn: HashMap, Frame>, + from_conn: HashMap, FlowFrame>, // (id, discriminant) as key ensures only 1 of every frame type per stream // will be queued. - from_streams: HashMap<(StreamId, mem::Discriminant), Frame>, + from_streams: HashMap<(StreamId, mem::Discriminant), FlowFrame>, // (stream_type, discriminant) as key ensures only 1 of every frame type // per stream type will be queued. - from_stream_types: HashMap<(StreamType, mem::Discriminant), Frame>, + from_stream_types: HashMap<(StreamType, mem::Discriminant), FlowFrame>, used_data: u64, max_data: u64, @@ -53,10 +56,10 @@ impl FlowMgr { /// Returns whether max credit was actually increased. pub fn conn_increase_max_credit(&mut self, new: u64) -> bool { + const DB_FRAME: FlowFrame = Frame::DataBlocked { data_limit: 0 }; + if new > self.max_data { self.max_data = new; - - const DB_FRAME: Frame = Frame::DataBlocked { data_limit: 0 }; self.from_conn.remove(&mem::discriminant(&DB_FRAME)); true @@ -275,37 +278,102 @@ impl FlowMgr { } } - pub(crate) fn get_frame( + pub(crate) fn write_frames( &mut self, - space: PNSpace, - remaining: usize, - ) -> Option<(Frame, Option)> { - if space != PNSpace::ApplicationData { - return None; - } + builder: &mut PacketBuilder, + tokens: &mut Vec, + stats: &mut FrameStats, + ) { + while let Some(frame) = self.peek() { + // All these frames are bags of varints, so we can just extract the + // varints and use common code for writing. + let values: SmallVec<[_; 3]> = match frame { + Frame::ResetStream { + stream_id, + application_error_code, + final_size, + } => { + stats.reset_stream += 1; + smallvec![stream_id.as_u64(), *application_error_code, *final_size] + } + Frame::StopSending { + stream_id, + application_error_code, + } => { + stats.stop_sending += 1; + smallvec![stream_id.as_u64(), *application_error_code] + } - if let Some(frame) = self.peek() { - // A suboptimal way to figure out if the frame fits within remaining - // space. - let mut d = Encoder::default(); - frame.marshal(&mut d); - if d.len() > remaining { - qtrace!("flowc frame doesn't fit in remaining"); - return None; + Frame::MaxStreams { + maximum_streams, .. + } => { + stats.max_streams += 1; + smallvec![maximum_streams.as_u64()] + } + Frame::StreamsBlocked { stream_limit, .. } => { + stats.streams_blocked += 1; + smallvec![stream_limit.as_u64()] + } + + Frame::MaxData { maximum_data } => { + stats.max_data += 1; + smallvec![*maximum_data] + } + Frame::DataBlocked { data_limit } => { + stats.data_blocked += 1; + smallvec![*data_limit] + } + + Frame::MaxStreamData { + stream_id, + maximum_stream_data, + } => { + stats.max_stream_data += 1; + smallvec![stream_id.as_u64(), *maximum_stream_data] + } + Frame::StreamDataBlocked { + stream_id, + stream_data_limit, + } => { + stats.stream_data_blocked += 1; + smallvec![stream_id.as_u64(), *stream_data_limit] + } + + // A special case, just write it out and move on.. + Frame::PathResponse { data } => { + stats.path_response += 1; + if builder.remaining() < 1 + data.len() { + builder.encode_varint(frame.get_type()); + builder.encode(data); + tokens.push(RecoveryToken::Flow(self.next().unwrap())); + continue; + } else { + return; + } + } + + _ => unreachable!("{:?}", frame), + }; + debug_assert!(!values.spilled()); + + if builder.remaining() >= values.iter().map(|&v| Encoder::varint_len(v)).sum() { + builder.encode_varint(frame.get_type()); + for v in values { + builder.encode_varint(v); + } + tokens.push(RecoveryToken::Flow(self.next().unwrap())); + } else { + return; } - } else { - return None; } - // There is enough space we can add this frame to the packet. - let frame = self.next().expect("just peeked this"); - Some((frame.clone(), Some(RecoveryToken::Flow(frame)))) } } impl Iterator for FlowMgr { - type Item = Frame; + type Item = FlowFrame; + /// Used by generator to get a flow control frame. - fn next(&mut self) -> Option { + fn next(&mut self) -> Option { let first_key = self.from_conn.keys().next(); if let Some(&first_key) = first_key { return self.from_conn.remove(&first_key); diff --git a/third_party/rust/neqo-transport/src/frame.rs b/third_party/rust/neqo-transport/src/frame.rs index 430c7d6f00e7..5f3e7c9b78b6 100644 --- a/third_party/rust/neqo-transport/src/frame.rs +++ b/third_party/rust/neqo-transport/src/frame.rs @@ -6,14 +6,13 @@ // Directly relating to QUIC frames. -use neqo_common::{qdebug, qtrace, Decoder, Encoder}; +use neqo_common::{qtrace, Decoder}; use crate::cid::MAX_CONNECTION_ID_LEN; use crate::packet::PacketType; use crate::stream_id::{StreamId, StreamIndex}; use crate::{AppError, ConnectionError, Error, Res, TransportError, ERROR_APPLICATION_CLOSE}; -use std::cmp::{min, Ordering}; use std::convert::TryFrom; use std::ops::RangeInclusive; @@ -21,13 +20,13 @@ use std::ops::RangeInclusive; pub type FrameType = u64; const FRAME_TYPE_PADDING: FrameType = 0x0; -const FRAME_TYPE_PING: FrameType = 0x1; +pub const FRAME_TYPE_PING: FrameType = 0x1; pub const FRAME_TYPE_ACK: FrameType = 0x2; const FRAME_TYPE_ACK_ECN: FrameType = 0x3; const FRAME_TYPE_RST_STREAM: FrameType = 0x4; const FRAME_TYPE_STOP_SENDING: FrameType = 0x5; -const FRAME_TYPE_CRYPTO: FrameType = 0x6; -const FRAME_TYPE_NEW_TOKEN: FrameType = 0x7; +pub const FRAME_TYPE_CRYPTO: FrameType = 0x6; +pub const FRAME_TYPE_NEW_TOKEN: FrameType = 0x7; const FRAME_TYPE_STREAM: FrameType = 0x8; const FRAME_TYPE_STREAM_MAX: FrameType = 0xf; const FRAME_TYPE_MAX_DATA: FrameType = 0x10; @@ -127,7 +126,7 @@ pub struct AckRange { } #[derive(PartialEq, Debug, Clone)] -pub enum Frame { +pub enum Frame<'a> { Padding, Ping, Ack { @@ -147,16 +146,16 @@ pub enum Frame { }, Crypto { offset: u64, - data: Vec, + data: &'a [u8], }, NewToken { - token: Vec, + token: &'a [u8], }, Stream { - fin: bool, stream_id: StreamId, offset: u64, - data: Vec, + data: &'a [u8], + fin: bool, fill: bool, }, MaxData { @@ -184,8 +183,8 @@ pub enum Frame { NewConnectionId { sequence_number: u64, retire_prior: u64, - connection_id: Vec, - stateless_reset_token: [u8; 16], + connection_id: &'a [u8], + stateless_reset_token: &'a [u8; 16], }, RetireConnectionId { sequence_number: u64, @@ -199,12 +198,14 @@ pub enum Frame { ConnectionClose { error_code: CloseError, frame_type: u64, + // Not a reference as we use this to hold the value. + // This is not used in optimized builds anyway. reason_phrase: Vec, }, HandshakeDone, } -impl Frame { +impl<'a> Frame<'a> { pub fn get_type(&self) -> FrameType { match self { Self::Padding => FRAME_TYPE_PADDING, @@ -216,19 +217,7 @@ impl Frame { Self::NewToken { .. } => FRAME_TYPE_NEW_TOKEN, Self::Stream { fin, offset, fill, .. - } => { - let mut t = FRAME_TYPE_STREAM; - if *fin { - t |= STREAM_FRAME_BIT_FIN; - } - if *offset > 0 { - t |= STREAM_FRAME_BIT_OFF; - } - if !*fill { - t |= STREAM_FRAME_BIT_LEN; - } - t - } + } => Self::stream_type(*fin, *offset > 0, *fill), Self::MaxData { .. } => FRAME_TYPE_MAX_DATA, Self::MaxStreamData { .. } => FRAME_TYPE_MAX_STREAM_DATA, Self::MaxStreams { stream_type, .. } => { @@ -250,197 +239,18 @@ impl Frame { } } - /// Create a CRYPTO frame that fits the available space and its length. - pub fn new_crypto(offset: u64, data: &[u8], space: usize) -> (Self, usize) { - // Subtract the frame type and offset from available space. - let mut remaining = space - 1 - Encoder::varint_len(offset); - // Then subtract space for the length field. - let data_len = min(remaining - 1, data.len()); - remaining -= Encoder::varint_len(u64::try_from(data_len).unwrap()); - remaining = min(data.len(), remaining); - ( - Self::Crypto { - offset, - data: data[..remaining].to_vec(), - }, - remaining, - ) - } - - /// Create a STREAM frame that fits the available space. - /// Return a tuple of a frame and the amount of data it carries. - pub fn new_stream( - stream_id: u64, - offset: u64, - data: &[u8], - fin: bool, - space: usize, - ) -> Option<(Self, usize)> { - let mut overhead = 1 + Encoder::varint_len(stream_id); - if offset > 0 { - overhead += Encoder::varint_len(offset); + pub fn stream_type(fin: bool, nonzero_offset: bool, fill: bool) -> u64 { + let mut t = FRAME_TYPE_STREAM; + if fin { + t |= STREAM_FRAME_BIT_FIN; } - - let (fin, fill) = match (data.len() + overhead).cmp(&space) { - // More data than fits, fill the packet and negate |fin|. - Ordering::Greater => (false, true), - // Exact fit, fill the packet, keep |fin|. - Ordering::Equal => (fin, true), - // Too small, so include a length. - Ordering::Less => { - let data_len = min(space.saturating_sub(overhead + 1), data.len()); - overhead += Encoder::varint_len(u64::try_from(data_len).unwrap()); - - // If all data isn't going to make it in the frame, don't keep fin. - let keep_fin = data.len() + overhead <= space; - (fin && keep_fin, false) - } - }; - - if overhead > space { - qdebug!( - "Frame::new_stream -> None; ovr {} > space {}", - overhead, - space - ); - return None; + if nonzero_offset { + t |= STREAM_FRAME_BIT_OFF; } - - let data_len = min(data.len(), space - overhead); - if data_len == 0 && !fin { - qdebug!("Frame::new_stream -> None; no data, no fin"); - return None; - } - - qdebug!( - "Frame::new_stream fill {} fin {} data {} space {} ovr {}", - fill, - fin, - data_len, - space, - overhead - ); - - Some(( - Self::Stream { - stream_id: stream_id.into(), - offset, - data: data[..data_len].to_vec(), - fin, - fill, - }, - data_len, - )) - } - - pub fn marshal(&self, enc: &mut Encoder) { - enc.encode_varint(self.get_type()); - - match self { - Self::Padding | Self::Ping => (), - Self::Ack { .. } => unreachable!(), - Self::ResetStream { - stream_id, - application_error_code, - final_size, - } => { - enc.encode_varint(stream_id.as_u64()); - enc.encode_varint(*application_error_code); - enc.encode_varint(*final_size); - } - Self::StopSending { - stream_id, - application_error_code, - } => { - enc.encode_varint(stream_id.as_u64()); - enc.encode_varint(*application_error_code); - } - Self::Crypto { offset, data } => { - enc.encode_varint(*offset); - enc.encode_vvec(&data); - } - Self::NewToken { token } => { - enc.encode_vvec(token); - } - Self::Stream { - stream_id, - offset, - data, - fill, - .. - } => { - enc.encode_varint(stream_id.as_u64()); - if *offset > 0 { - enc.encode_varint(*offset); - } - if *fill { - enc.encode(&data); - } else { - enc.encode_vvec(&data); - } - } - Self::MaxData { maximum_data } => { - enc.encode_varint(*maximum_data); - } - Self::MaxStreamData { - stream_id, - maximum_stream_data, - } => { - enc.encode_varint(stream_id.as_u64()); - enc.encode_varint(*maximum_stream_data); - } - Self::MaxStreams { - maximum_streams, .. - } => { - enc.encode_varint(maximum_streams.as_u64()); - } - Self::DataBlocked { data_limit } => { - enc.encode_varint(*data_limit); - } - Self::StreamDataBlocked { - stream_id, - stream_data_limit, - } => { - enc.encode_varint(stream_id.as_u64()); - enc.encode_varint(*stream_data_limit); - } - Self::StreamsBlocked { stream_limit, .. } => { - enc.encode_varint(stream_limit.as_u64()); - } - Self::NewConnectionId { - sequence_number, - retire_prior, - connection_id, - stateless_reset_token, - } => { - enc.encode_varint(*sequence_number); - enc.encode_varint(*retire_prior); - enc.encode_uint(1, connection_id.len() as u64); - enc.encode(connection_id); - enc.encode(stateless_reset_token); - } - Self::RetireConnectionId { sequence_number } => { - enc.encode_varint(*sequence_number); - } - Self::PathChallenge { data } => { - enc.encode(data); - } - Self::PathResponse { data } => { - enc.encode(data); - } - Self::ConnectionClose { - error_code, - frame_type, - reason_phrase, - } => { - enc.encode_varint(error_code.code()); - if let CloseError::Transport(_) = error_code { - enc.encode_varint(*frame_type); - } - enc.encode_vvec(reason_phrase); - } - Self::HandshakeDone => (), + if !fill { + t |= STREAM_FRAME_BIT_LEN; } + t } /// Convert a CONNECTION_CLOSE into a nicer CONNECTION_CLOSE. @@ -517,7 +327,7 @@ impl Frame { data, fin, } => Some(format!( - "Stream {{ stream_id: {}, offset: {}, len: {}{} fin: {} }}", + "Stream {{ stream_id: {}, offset: {}, len: {}{}, fin: {} }}", stream_id.as_u64(), offset, if *fill { ">>" } else { "" }, @@ -543,7 +353,7 @@ impl Frame { } } - pub fn decode(dec: &mut Decoder) -> Res { + pub fn decode(dec: &mut Decoder<'a>) -> Res { fn d(v: Option) -> Res { v.ok_or(Error::NoMoreData) } @@ -557,7 +367,7 @@ impl Frame { FRAME_TYPE_PADDING => Ok(Self::Padding), FRAME_TYPE_PING => Ok(Self::Ping), FRAME_TYPE_RST_STREAM => Ok(Self::ResetStream { - stream_id: dv(dec)?.into(), + stream_id: StreamId::from(dv(dec)?), application_error_code: d(dec.decode_varint())?, final_size: match dec.decode_varint() { Some(v) => v, @@ -593,22 +403,19 @@ impl Frame { }) } FRAME_TYPE_STOP_SENDING => Ok(Self::StopSending { - stream_id: dv(dec)?.into(), + stream_id: StreamId::from(dv(dec)?), application_error_code: d(dec.decode_varint())?, }), FRAME_TYPE_CRYPTO => { - let o = dv(dec)?; + let offset = dv(dec)?; let data = d(dec.decode_vvec())?; - if o + u64::try_from(data.len()).unwrap() > ((1 << 62) - 1) { + if offset + u64::try_from(data.len()).unwrap() > ((1 << 62) - 1) { return Err(Error::FrameEncodingError); } - Ok(Self::Crypto { - offset: o, - data: data.to_vec(), // TODO(mt) unnecessary copy - }) + Ok(Self::Crypto { offset, data }) } FRAME_TYPE_NEW_TOKEN => { - let token = d(dec.decode_vvec())?.to_vec(); + let token = d(dec.decode_vvec())?; if token.is_empty() { return Err(Error::FrameEncodingError); } @@ -634,9 +441,9 @@ impl Frame { } Ok(Self::Stream { fin: (t & STREAM_FRAME_BIT_FIN) != 0, - stream_id: s.into(), + stream_id: StreamId::from(s), offset: o, - data: data.to_vec(), // TODO(mt) unnecessary copy. + data, fill, }) } @@ -644,7 +451,7 @@ impl Frame { maximum_data: dv(dec)?, }), FRAME_TYPE_MAX_STREAM_DATA => Ok(Self::MaxStreamData { - stream_id: dv(dec)?.into(), + stream_id: StreamId::from(dv(dec)?), maximum_stream_data: dv(dec)?, }), FRAME_TYPE_MAX_STREAMS_BIDI | FRAME_TYPE_MAX_STREAMS_UNIDI => { @@ -671,21 +478,20 @@ impl Frame { }) } FRAME_TYPE_NEW_CONNECTION_ID => { - let s = dv(dec)?; + let sequence_number = dv(dec)?; let retire_prior = dv(dec)?; - let cid = d(dec.decode_vec(1))?.to_vec(); // TODO(mt) unnecessary copy - if cid.len() > MAX_CONNECTION_ID_LEN { + let connection_id = d(dec.decode_vec(1))?; + if connection_id.len() > MAX_CONNECTION_ID_LEN { return Err(Error::DecodingFrame); } let srt = d(dec.decode(16))?; - let mut srtv: [u8; 16] = [0; 16]; - srtv.copy_from_slice(&srt); + let stateless_reset_token = <&[_; 16]>::try_from(srt).unwrap(); Ok(Self::NewConnectionId { - sequence_number: s, + sequence_number, retire_prior, - connection_id: cid, - stateless_reset_token: srtv, + connection_id, + stateless_reset_token, }) } FRAME_TYPE_RETIRE_CONNECTION_ID => Ok(Self::RetireConnectionId { @@ -710,7 +516,8 @@ impl Frame { } else { 0 }; - let reason_phrase = d(dec.decode_vvec())?.to_vec(); // TODO(mt) unnecessary copy + // We can tolerate this copy for now. + let reason_phrase = d(dec.decode_vvec())?.to_vec(); Ok(Self::ConnectionClose { error_code, frame_type, @@ -726,6 +533,7 @@ impl Frame { #[cfg(test)] mod tests { use super::*; + use neqo_common::{Decoder, Encoder}; fn just_dec(f: &Frame, s: &str) { let encoded = Encoder::from_hex(s); @@ -733,27 +541,16 @@ mod tests { assert_eq!(*f, decoded); } - fn enc_dec(f: &Frame, s: &str) { - let mut enc = Encoder::default(); - let expected = Encoder::from_hex(s); - - f.marshal(&mut enc); - assert_eq!(enc, expected); - - let decoded = Frame::decode(&mut expected.as_decoder()).unwrap(); - assert_eq!(*f, decoded); - } - #[test] - fn test_padding_frame() { + fn padding() { let f = Frame::Padding; - enc_dec(&f, "00"); + just_dec(&f, "00"); } #[test] - fn test_ping_frame() { + fn ping() { let f = Frame::Ping; - enc_dec(&f, "01"); + just_dec(&f, "01"); } #[test] @@ -781,43 +578,43 @@ mod tests { } #[test] - fn test_reset_stream() { + fn reset_stream() { let f = Frame::ResetStream { - stream_id: 0x1234.into(), + stream_id: StreamId::from(0x1234), application_error_code: 0x77, final_size: 0x3456, }; - enc_dec(&f, "04523440777456"); + just_dec(&f, "04523440777456"); } #[test] - fn test_stop_sending() { + fn stop_sending() { let f = Frame::StopSending { - stream_id: 63.into(), + stream_id: StreamId::from(63), application_error_code: 0x77, }; - enc_dec(&f, "053F4077") + just_dec(&f, "053F4077") } #[test] - fn test_crypto() { + fn crypto() { let f = Frame::Crypto { offset: 1, - data: vec![1, 2, 3], + data: &[1, 2, 3], }; - enc_dec(&f, "060103010203"); + just_dec(&f, "060103010203"); } #[test] fn new_token() { let f = Frame::NewToken { - token: vec![0x12, 0x34, 0x56], + token: &[0x12, 0x34, 0x56], }; - enc_dec(&f, "0703123456"); + just_dec(&f, "0703123456"); } #[test] @@ -830,119 +627,119 @@ mod tests { } #[test] - fn test_stream() { + fn stream() { // First, just set the length bit. let f = Frame::Stream { fin: false, - stream_id: 5.into(), + stream_id: StreamId::from(5), offset: 0, - data: vec![1, 2, 3], + data: &[1, 2, 3], fill: false, }; - enc_dec(&f, "0a0503010203"); + just_dec(&f, "0a0503010203"); // Now with offset != 0 and FIN let f = Frame::Stream { fin: true, - stream_id: 5.into(), + stream_id: StreamId::from(5), offset: 1, - data: vec![1, 2, 3], + data: &[1, 2, 3], fill: false, }; - enc_dec(&f, "0f050103010203"); + just_dec(&f, "0f050103010203"); // Now to fill the packet. let f = Frame::Stream { fin: true, - stream_id: 5.into(), + stream_id: StreamId::from(5), offset: 0, - data: vec![1, 2, 3], + data: &[1, 2, 3], fill: true, }; - enc_dec(&f, "0905010203"); + just_dec(&f, "0905010203"); } #[test] - fn test_max_data() { + fn max_data() { let f = Frame::MaxData { maximum_data: 0x1234, }; - enc_dec(&f, "105234"); + just_dec(&f, "105234"); } #[test] - fn test_max_stream_data() { + fn max_stream_data() { let f = Frame::MaxStreamData { - stream_id: 5.into(), + stream_id: StreamId::from(5), maximum_stream_data: 0x1234, }; - enc_dec(&f, "11055234"); + just_dec(&f, "11055234"); } #[test] - fn test_max_streams() { + fn max_streams() { let mut f = Frame::MaxStreams { stream_type: StreamType::BiDi, maximum_streams: StreamIndex::new(0x1234), }; - enc_dec(&f, "125234"); + just_dec(&f, "125234"); f = Frame::MaxStreams { stream_type: StreamType::UniDi, maximum_streams: StreamIndex::new(0x1234), }; - enc_dec(&f, "135234"); + just_dec(&f, "135234"); } #[test] - fn test_data_blocked() { + fn data_blocked() { let f = Frame::DataBlocked { data_limit: 0x1234 }; - enc_dec(&f, "145234"); + just_dec(&f, "145234"); } #[test] - fn test_stream_data_blocked() { + fn stream_data_blocked() { let f = Frame::StreamDataBlocked { - stream_id: 5.into(), + stream_id: StreamId::from(5), stream_data_limit: 0x1234, }; - enc_dec(&f, "15055234"); + just_dec(&f, "15055234"); } #[test] - fn test_streams_blocked() { + fn streams_blocked() { let mut f = Frame::StreamsBlocked { stream_type: StreamType::BiDi, stream_limit: StreamIndex::new(0x1234), }; - enc_dec(&f, "165234"); + just_dec(&f, "165234"); f = Frame::StreamsBlocked { stream_type: StreamType::UniDi, stream_limit: StreamIndex::new(0x1234), }; - enc_dec(&f, "175234"); + just_dec(&f, "175234"); } #[test] - fn test_new_connection_id() { + fn new_connection_id() { let f = Frame::NewConnectionId { sequence_number: 0x1234, retire_prior: 0, - connection_id: vec![0x01, 0x02], - stateless_reset_token: [9; 16], + connection_id: &[0x01, 0x02], + stateless_reset_token: &[9; 16], }; - enc_dec(&f, "1852340002010209090909090909090909090909090909"); + just_dec(&f, "1852340002010209090909090909090909090909090909"); } #[test] @@ -957,26 +754,26 @@ mod tests { } #[test] - fn test_retire_connection_id() { + fn retire_connection_id() { let f = Frame::RetireConnectionId { sequence_number: 0x1234, }; - enc_dec(&f, "195234"); + just_dec(&f, "195234"); } #[test] - fn test_path_challenge() { + fn path_challenge() { let f = Frame::PathChallenge { data: [9; 8] }; - enc_dec(&f, "1a0909090909090909"); + just_dec(&f, "1a0909090909090909"); } #[test] - fn test_path_response() { + fn path_response() { let f = Frame::PathResponse { data: [9; 8] }; - enc_dec(&f, "1b0909090909090909"); + just_dec(&f, "1b0909090909090909"); } #[test] @@ -987,7 +784,7 @@ mod tests { reason_phrase: vec![0x01, 0x02, 0x03], }; - enc_dec(&f, "1c80005678523403010203"); + just_dec(&f, "1c80005678523403010203"); } #[test] @@ -998,7 +795,7 @@ mod tests { reason_phrase: vec![0x01, 0x02, 0x03], }; - enc_dec(&f, "1d8000567803010203"); + just_dec(&f, "1d8000567803010203"); } #[test] @@ -1007,19 +804,19 @@ mod tests { let f2 = Frame::Padding; let f3 = Frame::Crypto { offset: 0, - data: vec![1, 2, 3], + data: &[1, 2, 3], }; let f4 = Frame::Crypto { offset: 0, - data: vec![1, 2, 3], + data: &[1, 2, 3], }; let f5 = Frame::Crypto { offset: 1, - data: vec![1, 2, 3], + data: &[1, 2, 3], }; let f6 = Frame::Crypto { offset: 0, - data: vec![1, 2, 4], + data: &[1, 2, 4], }; assert_eq!(f1, f2); @@ -1030,150 +827,9 @@ mod tests { } #[test] - fn test_decode_ack_frame() { + fn decode_ack_frame() { let res = Frame::decode_ack_frame(7, 2, &[AckRange { gap: 0, range: 3 }]); assert!(res.is_ok()); assert_eq!(res.unwrap(), vec![5..=7, 0..=3]); } - - #[test] - fn new_stream_empty() { - // Stream frames with empty data and no fin never work. - assert!(Frame::new_stream(0, 10, &[], false, 2).is_none()); - assert!(Frame::new_stream(0, 10, &[], false, 3).is_none()); - assert!(Frame::new_stream(0, 10, &[], false, 4).is_none()); - assert!(Frame::new_stream(0, 10, &[], false, 5).is_none()); - assert!(Frame::new_stream(0, 10, &[], false, 100).is_none()); - - // Empty data with fin is only a problem if there is no space. - assert!(Frame::new_stream(0, 0, &[], true, 1).is_none()); - assert!(Frame::new_stream(0, 0, &[], true, 2).is_some()); - assert!(Frame::new_stream(0, 10, &[], true, 2).is_none()); - assert!(Frame::new_stream(0, 10, &[], true, 3).is_some()); - assert!(Frame::new_stream(0, 10, &[], true, 4).is_some()); - assert!(Frame::new_stream(0, 10, &[], true, 5).is_some()); - assert!(Frame::new_stream(0, 10, &[], true, 100).is_some()); - } - - #[test] - fn new_stream_minimum() { - // Add minimum data - assert!(Frame::new_stream(0, 10, &[0x42; 1], false, 3).is_none()); - assert!(Frame::new_stream(0, 10, &[0x42; 1], true, 3).is_none()); - assert!(Frame::new_stream(0, 10, &[0x42; 1], false, 4).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 1], true, 4).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 1], false, 5).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 1], true, 5).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 1], false, 100).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 1], true, 100).is_some()); - } - - #[test] - fn new_stream_more() { - // Try more data - assert!(Frame::new_stream(0, 10, &[0x42; 100], false, 3).is_none()); - assert!(Frame::new_stream(0, 10, &[0x42; 100], true, 3).is_none()); - assert!(Frame::new_stream(0, 10, &[0x42; 100], false, 4).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 100], true, 4).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 100], false, 5).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 100], true, 5).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 100], false, 100).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 100], true, 100).is_some()); - - assert!(Frame::new_stream(0, 10, &[0x42; 100], false, 1000).is_some()); - assert!(Frame::new_stream(0, 10, &[0x42; 100], true, 1000).is_some()); - } - - #[test] - fn new_stream_big_id() { - // A value that encodes to the largest varint. - const BIG: u64 = 1 << 30; - - assert!(Frame::new_stream(BIG, BIG, &[], false, 16).is_none()); - assert!(Frame::new_stream(BIG, BIG, &[], true, 16).is_none()); - assert!(Frame::new_stream(BIG, BIG, &[], false, 17).is_none()); - assert!(Frame::new_stream(BIG, BIG, &[], true, 17).is_some()); - assert!(Frame::new_stream(BIG, BIG, &[], false, 18).is_none()); - assert!(Frame::new_stream(BIG, BIG, &[], true, 18).is_some()); - - assert!(Frame::new_stream(BIG, BIG, &[0x42; 1], false, 17).is_none()); - assert!(Frame::new_stream(BIG, BIG, &[0x42; 1], true, 17).is_none()); - assert!(Frame::new_stream(BIG, BIG, &[0x42; 1], false, 18).is_some()); - assert!(Frame::new_stream(BIG, BIG, &[0x42; 1], true, 18).is_some()); - assert!(Frame::new_stream(BIG, BIG, &[0x42; 1], false, 19).is_some()); - assert!(Frame::new_stream(BIG, BIG, &[0x42; 1], true, 19).is_some()); - assert!(Frame::new_stream(BIG, BIG, &[0x42; 1], false, 100).is_some()); - assert!(Frame::new_stream(BIG, BIG, &[0x42; 1], true, 100).is_some()); - } - - #[test] - fn new_stream_16384() { - // 16383/16384 is an odd boundary in STREAM frame construction. - // That is the boundary where a length goes from 2 bytes to 4 bytes. - // If the data fits in the available space, then it is simple: - let r = Frame::new_stream(0, 0, &[0x43; 16384], true, 16386); - let (f, used) = r.expect("Fit frame"); - assert_eq!(used, 16384); - if let Frame::Stream { - fin, fill, data, .. - } = f - { - assert_eq!(data.len(), 16384); - assert!(fin); - assert!(fill); - } else { - panic!("Wrong frame type"); - } - - // However, if there is one extra byte of space, we will try to add a length. - // That length will then make the frame to be too large and the data will be - // truncated. The frame could carry one more byte of data, but it's a corner - // case we don't want to address as it should be rare (if not impossible). - let r = Frame::new_stream(0, 0, &[0x43; 16384], true, 16387); - let (f, used) = r.expect("a frame"); - assert_eq!(used, 16381); - if let Frame::Stream { - fin, fill, data, .. - } = f - { - assert_eq!(data.len(), 16381); - assert!(!fin); - assert!(!fill); - } else { - panic!("Wrong frame type"); - } - } - - #[test] - fn new_stream_64() { - // Unlike 16383/16384, the boundary at 63/64 is easy because the difference - // is just one byte. We lose just the last byte when there is more space. - let r = Frame::new_stream(0, 0, &[0x43; 64], true, 66); - let (f, used) = r.expect("Fit frame"); - assert_eq!(used, 64); - if let Frame::Stream { - fin, fill, data, .. - } = f - { - assert_eq!(data.len(), 64); - assert!(fin); - assert!(fill); - } else { - panic!("Wrong frame type"); - } - - let r = Frame::new_stream(0, 0, &[0x43; 64], true, 67); - let (f, used) = r.expect("a frame"); - assert_eq!(used, 63); - if let Frame::Stream { - fin, fill, data, .. - } = f - { - assert_eq!(data.len(), 63); - assert!(!fin); - assert!(!fill); - } else { - panic!("Wrong frame type"); - } - } } diff --git a/third_party/rust/neqo-transport/src/packet/mod.rs b/third_party/rust/neqo-transport/src/packet/mod.rs index 8001e56de9b2..216e60009a09 100644 --- a/third_party/rust/neqo-transport/src/packet/mod.rs +++ b/third_party/rust/neqo-transport/src/packet/mod.rs @@ -315,7 +315,7 @@ impl PacketBuilder { /// Work out if nothing was added after the header. #[must_use] - pub fn is_empty(&self) -> bool { + pub fn packet_empty(&self) -> bool { self.encoder.len() == self.header.end } diff --git a/third_party/rust/neqo-transport/src/recv_stream.rs b/third_party/rust/neqo-transport/src/recv_stream.rs index 53272523b880..e82e08d2ecee 100644 --- a/third_party/rust/neqo-transport/src/recv_stream.rs +++ b/third_party/rust/neqo-transport/src/recv_stream.rs @@ -46,137 +46,118 @@ impl RxStreamOrderer { /// Process an incoming stream frame off the wire. This may result in data /// being available to upper layers if frame is not out of order (ooo) or /// if the frame fills a gap. - pub fn inbound_frame(&mut self, new_start: u64, mut new_data: Vec) -> Res<()> { + pub fn inbound_frame(&mut self, mut new_start: u64, mut new_data: &[u8]) { qtrace!("Inbound data offset={} len={}", new_start, new_data.len()); // Get entry before where new entry would go, so we can see if we already // have the new bytes. // Avoid copies and duplicated data. - let new_end = new_start + new_data.len() as u64; + let new_end = new_start + u64::try_from(new_data.len()).unwrap(); if new_end <= self.retired { // Range already read by application, this frame is very late and unneeded. - return Ok(()); + return; + } + + if new_start < self.retired { + new_data = &new_data[usize::try_from(self.retired - new_start).unwrap()..]; + new_start = self.retired; } if new_data.is_empty() { // No data to insert - return Ok(()); + return; } - let (insert_new, remove_prev) = if let Some((&prev_start, prev_vec)) = self + let extend = if let Some((&prev_start, prev_vec)) = self .data_ranges .range_mut((Unbounded, Included(new_start))) .next_back() { - let prev_end = prev_start + prev_vec.len() as u64; - - match (new_start > prev_start, new_end > prev_end) { - (true, true) => { - // PPPPPP -> PPPPPP - // NNNNNN NN - // Add a range containing only new data - // (In-order frames will take this path, with no overlap) - let overlap = prev_end.saturating_sub(new_start); - qtrace!( - "New frame {}-{} received, overlap: {}", - new_start, - new_end, - overlap - ); - if overlap != 0 { - new_data.drain(..overlap as usize); - return self.inbound_frame(prev_end, new_data); - } - (true, None) - } - (true, false) => { - // PPPPPP -> PPPPPP - // NNNN - // Do nothing - qtrace!( - "Dropping frame with already-received range {}-{}", - new_start, - new_end - ); - (false, None) - } - (false, true) => { - // PPPP -> PPPP - // NNNNNN NN - qtrace!( - "New frame with {}-{} overlaps with existing {}-{}", - new_start, - new_end, - prev_start, - prev_end - ); - let overlap = prev_end.saturating_sub(new_start); - new_data.drain(..overlap as usize); - return self.inbound_frame(prev_end, new_data); - } - (false, false) => { - // PPPPPP -> PPPPPP - // NNNN - // Do nothing - qtrace!( - "Dropping frame with already-received range {}-{}", - new_start, - new_end - ); - (false, None) - } + let prev_end = prev_start + u64::try_from(prev_vec.len()).unwrap(); + if new_end > prev_end { + // PPPPPP -> PPPPPP + // NNNNNN NN + // NNNNNNNN NN + // Add a range containing only new data + // (In-order frames will take this path, with no overlap) + let overlap = prev_end.saturating_sub(new_start); + qtrace!( + "New frame {}-{} received, overlap: {}", + new_start, + new_end, + overlap + ); + new_start += overlap; + new_data = &new_data[usize::try_from(overlap).unwrap()..]; + // If it is small enough, extend the previous buffer. + // This can't always extend, because otherwise the buffer could end up + // growing indefinitely without being released. + prev_vec.len() < 4096 && prev_end == new_start + } else { + // PPPPPP -> PPPPPP + // NNNN + // NNNN + // Do nothing + qtrace!( + "Dropping frame with already-received range {}-{}", + new_start, + new_end + ); + return; } } else { qtrace!("New frame {}-{} received", new_start, new_end); - (true, None) // Nothing previous + false }; - if let Some(remove_prev) = &remove_prev { - self.data_ranges.remove(remove_prev); + // Now handle possible overlap with next entries + let mut to_remove = SmallVec::<[_; 8]>::new(); + let mut to_add = new_data; + + for (&next_start, next_data) in self.data_ranges.range_mut(new_start..) { + let next_end = next_start + u64::try_from(next_data.len()).unwrap(); + let overlap = new_end.saturating_sub(next_start); + if overlap == 0 { + break; + } else if next_end >= new_end { + qtrace!( + "New frame {}-{} overlaps with next frame by {}, truncating", + new_start, + new_end, + overlap + ); + let truncate_to = new_data.len() - usize::try_from(overlap).unwrap(); + to_add = &new_data[..truncate_to]; + break; + } else { + qtrace!( + "New frame {}-{} spans entire next frame {}-{}, replacing", + new_start, + new_end, + next_start, + next_end + ); + to_remove.push(next_start); + } } - if insert_new { - // Now handle possible overlap with next entries - let mut to_remove = SmallVec::<[_; 8]>::new(); + for start in to_remove { + self.data_ranges.remove(&start); + } - for (&next_start, next_data) in self.data_ranges.range_mut(new_start..) { - let next_end = next_start + next_data.len() as u64; - let overlap = new_end.saturating_sub(next_start); - if overlap == 0 { - break; - } else if next_end > new_end { - qtrace!( - "New frame {}-{} overlaps with next frame by {}, truncating", - new_start, - new_end, - overlap - ); - let truncate_to = new_data.len() - overlap as usize; - new_data.truncate(truncate_to); - break; - } else { - qtrace!( - "New frame {}-{} spans entire next frame {}-{}, replacing", - new_start, - new_end, - next_start, - next_end - ); - to_remove.push(next_start); - } + if !to_add.is_empty() { + if extend { + let (_, buf) = self + .data_ranges + .range_mut((Unbounded, Included(new_start))) + .next_back() + .unwrap(); + buf.extend_from_slice(to_add); + } else { + self.data_ranges.insert(new_start, to_add.to_vec()); } - - for start in to_remove { - self.data_ranges.remove(&start); - } - - if !new_data.is_empty() { - self.data_ranges.insert(new_start, new_data); - } - }; - - Ok(()) + } } /// Are any bytes readable? @@ -399,12 +380,11 @@ impl RecvStream { self.state = new_state; } - pub fn inbound_stream_frame(&mut self, fin: bool, offset: u64, data: Vec) -> Res<()> { + pub fn inbound_stream_frame(&mut self, fin: bool, offset: u64, data: &[u8]) -> Res<()> { // We should post a DataReadable event only once when we change from no-data-ready to // data-ready. Therefore remember the state before processing a new frame. let already_data_ready = self.data_ready(); - - let new_end = offset + data.len() as u64; + let new_end = offset + u64::try_from(data.len()).unwrap(); // Send final size errors even if stream is closed if let Some(final_size) = self.state.final_size() { @@ -429,7 +409,7 @@ impl RecvStream { if final_size < recv_buf.highest_seen_offset() { return Err(Error::FinalSizeError); } - recv_buf.inbound_frame(offset, data)?; + recv_buf.inbound_frame(offset, data); let buf = mem::replace(recv_buf, RxStreamOrderer::new()); if final_size == buf.retired() + buf.bytes_ready() as u64 { @@ -441,14 +421,14 @@ impl RecvStream { }); } } else { - recv_buf.inbound_frame(offset, data)?; + recv_buf.inbound_frame(offset, data); } } RecvStreamState::SizeKnown { recv_buf, final_size, } => { - recv_buf.inbound_frame(offset, data)?; + recv_buf.inbound_frame(offset, data); if *final_size == recv_buf.retired() + recv_buf.bytes_ready() as u64 { let buf = mem::replace(recv_buf, RxStreamOrderer::new()); self.set_state(RecvStreamState::DataRecvd { recv_buf: buf }); @@ -570,17 +550,19 @@ mod tests { fn recv_ranges(ranges: &[Range], available: usize) { const ZEROES: &[u8] = &[0; 100]; + qtrace!("recv_ranges {:?}", ranges); let mut s = RxStreamOrderer::default(); for r in ranges { - let data = ZEROES[..usize::try_from(r.end - r.start).unwrap()].to_vec(); - s.inbound_frame(r.start, data).unwrap(); + let data = &ZEROES[..usize::try_from(r.end - r.start).unwrap()]; + s.inbound_frame(r.start, data); } - let mut buf = vec![0xff; 100]; + let mut buf = [0xff; 100]; let mut total_recvd = 0; loop { - let recvd = s.read(&mut buf); + let recvd = s.read(&mut buf[..]); + qtrace!("recv_ranges read {}", recvd); total_recvd += recvd; if recvd == 0 { assert_eq!(total_recvd, available); @@ -701,11 +683,11 @@ mod tests { let mut s = RxStreamOrderer::new(); // Add three chunks. - s.inbound_frame(0, vec![0; CHUNK_SIZE]).unwrap(); + s.inbound_frame(0, &[0; CHUNK_SIZE]); let offset = u64::try_from(CHUNK_SIZE).unwrap(); - s.inbound_frame(offset, vec![0; EXTRA_SIZE]).unwrap(); + s.inbound_frame(offset, &[0; EXTRA_SIZE]); let offset = u64::try_from(CHUNK_SIZE + EXTRA_SIZE).unwrap(); - s.inbound_frame(offset, vec![0; EXTRA_SIZE]).unwrap(); + s.inbound_frame(offset, &[0; EXTRA_SIZE]); // Read, providing only enough space for the first. let mut buf = vec![0; 100]; @@ -720,22 +702,21 @@ mod tests { let mut s = RxStreamOrderer::new(); // Add a chunk - s.inbound_frame(0, vec![0; 150]).unwrap(); + s.inbound_frame(0, &[0; 150]); assert_eq!(s.data_ranges.get(&0).unwrap().len(), 150); - // Read, providing only enough space for the first. - let mut buf = vec![0; 100]; - let count = s.read(&mut buf); + // Read, providing only enough space for the first 100. + let mut buf = [0; 100]; + let count = s.read(&mut buf[..]); assert_eq!(count, 100); assert_eq!(s.retired, 100); // Add a second frame that overlaps. // This shouldn't truncate the first frame, as we're already // Reading from it. - s.inbound_frame(120, vec![0; 60]).unwrap(); - assert_eq!(s.data_ranges.get(&0).unwrap().len(), 150); - assert_eq!(s.data_ranges.get(&150).unwrap().len(), 30); + s.inbound_frame(120, &[0; 60]); + assert_eq!(s.data_ranges.get(&0).unwrap().len(), 180); // Read second part of first frame and all of the second frame - let count = s.read(&mut buf); + let count = s.read(&mut buf[..]); assert_eq!(count, 80); } @@ -747,18 +728,18 @@ mod tests { let mut s = RxStreamOrderer::new(); // Add three chunks. - s.inbound_frame(0, vec![0; CHUNK_SIZE]).unwrap(); + s.inbound_frame(0, &[0; CHUNK_SIZE]); let offset = u64::try_from(CHUNK_SIZE + EXTRA_SIZE).unwrap(); - s.inbound_frame(offset, vec![0; EXTRA_SIZE]).unwrap(); + s.inbound_frame(offset, &[0; EXTRA_SIZE]); // Read, providing only enough space for the first chunk. - let mut buf = vec![0; 100]; + let mut buf = [0; 100]; let count = s.read(&mut buf[..CHUNK_SIZE]); assert_eq!(count, CHUNK_SIZE); // Now fill the gap and ensure that everything can be read. let offset = u64::try_from(CHUNK_SIZE).unwrap(); - s.inbound_frame(offset, vec![0; EXTRA_SIZE]).unwrap(); + s.inbound_frame(offset, &[0; EXTRA_SIZE]); let count = s.read(&mut buf[..]); assert_eq!(count, EXTRA_SIZE * 2); } @@ -771,12 +752,12 @@ mod tests { let mut s = RxStreamOrderer::new(); // Add two chunks. - s.inbound_frame(0, vec![0; CHUNK_SIZE]).unwrap(); + s.inbound_frame(0, &[0; CHUNK_SIZE]); let offset = u64::try_from(CHUNK_SIZE).unwrap(); - s.inbound_frame(offset, vec![0; EXTRA_SIZE]).unwrap(); + s.inbound_frame(offset, &[0; EXTRA_SIZE]); // Read, providing only enough space for some of the first chunk. - let mut buf = vec![0; 100]; + let mut buf = [0; 100]; let count = s.read(&mut buf[..CHUNK_SIZE - EXTRA_SIZE]); assert_eq!(count, CHUNK_SIZE - EXTRA_SIZE); @@ -792,11 +773,11 @@ mod tests { let mut s = RxStreamOrderer::new(); // Add two chunks. - s.inbound_frame(0, vec![0; CHUNK_SIZE]).unwrap(); + s.inbound_frame(0, &[0; CHUNK_SIZE]); let offset = u64::try_from(CHUNK_SIZE).unwrap(); - s.inbound_frame(offset, vec![0; EXTRA_SIZE]).unwrap(); + s.inbound_frame(offset, &[0; EXTRA_SIZE]); - let mut buf = vec![0; 1]; + let mut buf = [0; 1]; for _ in 0..CHUNK_SIZE + EXTRA_SIZE { let count = s.read(&mut buf[..]); assert_eq!(count, 1); @@ -805,14 +786,14 @@ mod tests { } #[test] - fn test_stream_rx() { + fn stream_rx() { let flow_mgr = Rc::new(RefCell::new(FlowMgr::default())); let conn_events = ConnectionEvents::default(); - let mut s = RecvStream::new(567.into(), 1024, Rc::clone(&flow_mgr), conn_events); + let mut s = RecvStream::new(StreamId::from(567), 1024, Rc::clone(&flow_mgr), conn_events); // test receiving a contig frame and reading it works - s.inbound_stream_frame(false, 0, vec![1; 10]).unwrap(); + s.inbound_stream_frame(false, 0, &[1; 10]).unwrap(); assert_eq!(s.data_ready(), true); let mut buf = vec![0u8; 100]; assert_eq!(s.read(&mut buf).unwrap(), (10, false)); @@ -820,33 +801,33 @@ mod tests { assert_eq!(s.state.recv_buf().unwrap().buffered(), 0); // test receiving a noncontig frame - s.inbound_stream_frame(false, 12, vec![2; 12]).unwrap(); + s.inbound_stream_frame(false, 12, &[2; 12]).unwrap(); assert_eq!(s.data_ready(), false); assert_eq!(s.read(&mut buf).unwrap(), (0, false)); assert_eq!(s.state.recv_buf().unwrap().retired(), 10); assert_eq!(s.state.recv_buf().unwrap().buffered(), 12); // another frame that overlaps the first - s.inbound_stream_frame(false, 14, vec![3; 8]).unwrap(); + s.inbound_stream_frame(false, 14, &[3; 8]).unwrap(); assert_eq!(s.data_ready(), false); assert_eq!(s.state.recv_buf().unwrap().retired(), 10); assert_eq!(s.state.recv_buf().unwrap().buffered(), 12); // fill in the gap, but with a FIN - s.inbound_stream_frame(true, 10, vec![4; 6]).unwrap_err(); + s.inbound_stream_frame(true, 10, &[4; 6]).unwrap_err(); assert_eq!(s.data_ready(), false); assert_eq!(s.read(&mut buf).unwrap(), (0, false)); assert_eq!(s.state.recv_buf().unwrap().retired(), 10); assert_eq!(s.state.recv_buf().unwrap().buffered(), 12); // fill in the gap - s.inbound_stream_frame(false, 10, vec![5; 10]).unwrap(); + s.inbound_stream_frame(false, 10, &[5; 10]).unwrap(); assert_eq!(s.data_ready(), true); assert_eq!(s.state.recv_buf().unwrap().retired(), 10); assert_eq!(s.state.recv_buf().unwrap().buffered(), 14); // a legit FIN - s.inbound_stream_frame(true, 24, vec![6; 18]).unwrap(); + s.inbound_stream_frame(true, 24, &[6; 18]).unwrap(); assert_eq!(s.state.recv_buf().unwrap().retired(), 10); assert_eq!(s.state.recv_buf().unwrap().buffered(), 32); assert_eq!(s.data_ready(), true); @@ -856,121 +837,154 @@ mod tests { s.read(&mut buf).unwrap_err(); } - #[test] - #[allow(clippy::cognitive_complexity)] - fn test_stream_rx_dedupe() { - let flow_mgr = Rc::new(RefCell::new(FlowMgr::default())); - let conn_events = ConnectionEvents::default(); - - let mut s = RecvStream::new(3.into(), 1024, Rc::clone(&flow_mgr), conn_events); - - let mut buf = vec![0u8; 100]; - - // test receiving a contig frame and reading it works - s.inbound_stream_frame(false, 0, vec![1; 6]).unwrap(); - - // See inbound_frame(). Test (true, true) case - s.inbound_stream_frame(false, 2, vec![2; 6]).unwrap(); - { - let mut i = s.state.recv_buf().unwrap().data_ranges.iter(); - let item = i.next().unwrap(); - assert_eq!(*item.0, 0); - assert_eq!(item.1.len(), 6); - let item = i.next().unwrap(); - assert_eq!(*item.0, 6); - assert_eq!(item.1.len(), 2); - } - - // Test (true, false) case - s.inbound_stream_frame(false, 4, vec![3; 4]).unwrap(); - { - let mut i = s.state.recv_buf().unwrap().data_ranges.iter(); - let item = i.next().unwrap(); - assert_eq!(*item.0, 0); - assert_eq!(item.1.len(), 6); - let item = i.next().unwrap(); - assert_eq!(*item.0, 6); - assert_eq!(item.1.len(), 2); - } - - // Test (false, true) case - s.inbound_stream_frame(false, 2, vec![4; 8]).unwrap(); - { - let mut i = s.state.recv_buf().unwrap().data_ranges.iter(); - let item = i.next().unwrap(); - assert_eq!(*item.0, 0); - assert_eq!(item.1.len(), 6); - let item = i.next().unwrap(); - assert_eq!(*item.0, 6); - assert_eq!(item.1.len(), 2); - } - - // Test (false, false) case - s.inbound_stream_frame(false, 2, vec![5; 2]).unwrap(); - { - let mut i = s.state.recv_buf().unwrap().data_ranges.iter(); - let item = i.next().unwrap(); - assert_eq!(*item.0, 0); - assert_eq!(item.1.len(), 6); - let item = i.next().unwrap(); - assert_eq!(*item.0, 6); - assert_eq!(item.1.len(), 2); - } - - assert_eq!(s.read(&mut buf).unwrap(), (10, false)); - assert_eq!(buf[..10], [1, 1, 1, 1, 1, 1, 2, 2, 4, 4]); - - // Test truncation/span-drop on insert - s.inbound_stream_frame(false, 100, vec![6; 6]).unwrap(); - // a. insert where new frame gets truncated - s.inbound_stream_frame(false, 99, vec![7; 6]).unwrap(); - { - let mut i = s.state.recv_buf().unwrap().data_ranges.iter(); - let item = i.next().unwrap(); - assert_eq!(*item.0, 99); - assert_eq!(item.1.len(), 1); - let item = i.next().unwrap(); - assert_eq!(*item.0, 100); - assert_eq!(item.1.len(), 6); - assert_eq!(i.next(), None); - } - - // b. insert where new frame spans next frame - s.inbound_stream_frame(false, 98, vec![8; 10]).unwrap(); - { - let mut i = s.state.recv_buf().unwrap().data_ranges.iter(); - let item = i.next().unwrap(); - assert_eq!(*item.0, 98); - assert_eq!(item.1.len(), 10); - assert_eq!(i.next(), None); + fn check_chunks(s: &mut RxStreamOrderer, expected: &[(u64, usize)]) { + assert_eq!(s.data_ranges.len(), expected.len()); + for ((start, buf), (expected_start, expected_len)) in s.data_ranges.iter().zip(expected) { + assert_eq!((*start, buf.len()), (*expected_start, *expected_len)); } } + // Test deduplication when the new data is at the end. #[test] - fn test_stream_flowc_update() { + fn stream_rx_dedupe_tail() { + let mut s = RxStreamOrderer::new(); + + s.inbound_frame(0, &[1; 6]); + check_chunks(&mut s, &[(0, 6)]); + + // New data that overlaps entirely (starting from the head), is ignored. + s.inbound_frame(0, &[2; 3]); + check_chunks(&mut s, &[(0, 6)]); + + // New data that overlaps at the tail has any new data appended. + s.inbound_frame(2, &[3; 6]); + check_chunks(&mut s, &[(0, 8)]); + + // New data that overlaps entirely (up to the tail), is ignored. + s.inbound_frame(4, &[4; 4]); + check_chunks(&mut s, &[(0, 8)]); + + // New data that overlaps, starting from the beginning is appended too. + s.inbound_frame(0, &[5; 10]); + check_chunks(&mut s, &[(0, 10)]); + + // New data that is entirely subsumed is ignored. + s.inbound_frame(2, &[6; 2]); + check_chunks(&mut s, &[(0, 10)]); + + let mut buf = [0; 16]; + assert_eq!(s.read(&mut buf[..]), 10); + assert_eq!(buf[..10], [1, 1, 1, 1, 1, 1, 3, 3, 5, 5]); + } + + /// When chunks are added before existing data, they aren't merged. + #[test] + fn stream_rx_dedupe_head() { + let mut s = RxStreamOrderer::new(); + + s.inbound_frame(1, &[6; 6]); + check_chunks(&mut s, &[(1, 6)]); + + // Insertion before an existing chunk causes truncation of the new chunk. + s.inbound_frame(0, &[7; 6]); + check_chunks(&mut s, &[(0, 1), (1, 6)]); + + // Perfect overlap with existing slices has no effect. + s.inbound_frame(0, &[8; 7]); + check_chunks(&mut s, &[(0, 1), (1, 6)]); + + let mut buf = [0; 16]; + assert_eq!(s.read(&mut buf[..]), 7); + assert_eq!(buf[..7], [7, 6, 6, 6, 6, 6, 6]); + } + + #[test] + fn stream_rx_dedupe_new_tail() { + let mut s = RxStreamOrderer::new(); + + s.inbound_frame(1, &[6; 6]); + check_chunks(&mut s, &[(1, 6)]); + + // Insertion before an existing chunk causes truncation of the new chunk. + s.inbound_frame(0, &[7; 6]); + check_chunks(&mut s, &[(0, 1), (1, 6)]); + + // New data at the end causes the tail to be added to the first chunk, + // replacing later chunks entirely. + s.inbound_frame(0, &[9; 8]); + check_chunks(&mut s, &[(0, 8)]); + + let mut buf = [0; 16]; + assert_eq!(s.read(&mut buf[..]), 8); + assert_eq!(buf[..8], [7, 9, 9, 9, 9, 9, 9, 9]); + } + + #[test] + fn stream_rx_dedupe_replace() { + let mut s = RxStreamOrderer::new(); + + s.inbound_frame(2, &[6; 6]); + check_chunks(&mut s, &[(2, 6)]); + + // Insertion before an existing chunk causes truncation of the new chunk. + s.inbound_frame(1, &[7; 6]); + check_chunks(&mut s, &[(1, 1), (2, 6)]); + + // New data at the start and end replaces all the slices. + s.inbound_frame(0, &[9; 10]); + check_chunks(&mut s, &[(0, 10)]); + + let mut buf = [0; 16]; + assert_eq!(s.read(&mut buf[..]), 10); + assert_eq!(buf[..10], [9; 10]); + } + + #[test] + fn trim_retired() { + let mut s = RxStreamOrderer::new(); + + let mut buf = [0; 18]; + s.inbound_frame(0, &[1; 10]); + + // Partially read slices are retained. + assert_eq!(s.read(&mut buf[..6]), 6); + check_chunks(&mut s, &[(0, 10)]); + + // Partially read slices are kept and so are added to. + s.inbound_frame(3, &buf[..10]); + check_chunks(&mut s, &[(0, 13)]); + + // Wholly read pieces are dropped. + assert_eq!(s.read(&mut buf[..]), 7); + assert!(s.data_ranges.is_empty()); + + // New data that overlaps with retired data is trimmed. + s.inbound_frame(0, &buf[..]); + check_chunks(&mut s, &[(13, 5)]); + } + + #[test] + fn stream_flowc_update() { let flow_mgr = Rc::default(); let conn_events = ConnectionEvents::default(); - let frame1 = vec![0; RX_STREAM_DATA_WINDOW as usize]; + let frame1 = vec![0; RECV_BUFFER_SIZE]; let mut s = RecvStream::new( - 4.into(), + StreamId::from(4), RX_STREAM_DATA_WINDOW, Rc::clone(&flow_mgr), conn_events, ); - let mut buf = vec![0u8; RX_STREAM_DATA_WINDOW as usize * 4]; // Make it overlarge + let mut buf = vec![0u8; RECV_BUFFER_SIZE + 100]; // Make it overlarge s.maybe_send_flowc_update(); assert_eq!(s.flow_mgr.borrow().peek(), None); - s.inbound_stream_frame(false, 0, frame1).unwrap(); + s.inbound_stream_frame(false, 0, &frame1).unwrap(); s.maybe_send_flowc_update(); assert_eq!(s.flow_mgr.borrow().peek(), None); - assert_eq!( - s.read(&mut buf).unwrap(), - (RX_STREAM_DATA_WINDOW as usize, false) - ); + assert_eq!(s.read(&mut buf).unwrap(), (RECV_BUFFER_SIZE, false)); assert_eq!(s.data_ready(), false); s.maybe_send_flowc_update(); @@ -986,14 +1000,13 @@ mod tests { } #[test] - fn test_stream_max_stream_data() { + fn stream_max_stream_data() { let flow_mgr = Rc::new(RefCell::new(FlowMgr::default())); let conn_events = ConnectionEvents::default(); - let frame1 = vec![0; RX_STREAM_DATA_WINDOW as usize]; - + let frame1 = vec![0; RECV_BUFFER_SIZE]; let mut s = RecvStream::new( - 67.into(), + StreamId::from(67), RX_STREAM_DATA_WINDOW, Rc::clone(&flow_mgr), conn_events, @@ -1001,38 +1014,41 @@ mod tests { s.maybe_send_flowc_update(); assert_eq!(s.flow_mgr.borrow().peek(), None); - s.inbound_stream_frame(false, 0, frame1).unwrap(); - s.inbound_stream_frame(false, RX_STREAM_DATA_WINDOW, vec![1; 1]) + s.inbound_stream_frame(false, 0, &frame1).unwrap(); + s.inbound_stream_frame(false, RX_STREAM_DATA_WINDOW, &[1; 1]) .unwrap_err(); } #[test] - fn test_stream_orderer_bytes_ready() { + fn stream_orderer_bytes_ready() { let mut rx_ord = RxStreamOrderer::new(); - let mut buf = vec![0u8; 100]; - - rx_ord.inbound_frame(0, vec![1; 6]).unwrap(); + rx_ord.inbound_frame(0, &[1; 6]); assert_eq!(rx_ord.bytes_ready(), 6); assert_eq!(rx_ord.buffered(), 6); assert_eq!(rx_ord.retired(), 0); + // read some so there's an offset into the first frame + let mut buf = [0u8; 10]; rx_ord.read(&mut buf[..2]); assert_eq!(rx_ord.bytes_ready(), 4); assert_eq!(rx_ord.buffered(), 4); assert_eq!(rx_ord.retired(), 2); + // an overlapping frame - rx_ord.inbound_frame(5, vec![2; 6]).unwrap(); + rx_ord.inbound_frame(5, &[2; 6]); assert_eq!(rx_ord.bytes_ready(), 9); assert_eq!(rx_ord.buffered(), 9); assert_eq!(rx_ord.retired(), 2); + // a noncontig frame - rx_ord.inbound_frame(20, vec![3; 6]).unwrap(); + rx_ord.inbound_frame(20, &[3; 6]); assert_eq!(rx_ord.bytes_ready(), 9); assert_eq!(rx_ord.buffered(), 15); assert_eq!(rx_ord.retired(), 2); + // an old frame - rx_ord.inbound_frame(0, vec![4; 2]).unwrap(); + rx_ord.inbound_frame(0, &[4; 2]); assert_eq!(rx_ord.bytes_ready(), 9); assert_eq!(rx_ord.buffered(), 15); assert_eq!(rx_ord.retired(), 2); @@ -1043,19 +1059,19 @@ mod tests { let flow_mgr = Rc::new(RefCell::new(FlowMgr::default())); let conn_events = ConnectionEvents::default(); - let frame1 = vec![0; RX_STREAM_DATA_WINDOW as usize]; - + let frame1 = vec![0; RECV_BUFFER_SIZE]; + let stream_id = StreamId::from(67); let mut s = RecvStream::new( - 67.into(), + stream_id, RX_STREAM_DATA_WINDOW, Rc::clone(&flow_mgr), conn_events, ); - s.inbound_stream_frame(false, 0, frame1).unwrap(); - flow_mgr.borrow_mut().max_stream_data(67.into(), 100); + s.inbound_stream_frame(false, 0, &frame1).unwrap(); + flow_mgr.borrow_mut().max_stream_data(stream_id, 100); assert!(matches!(s.flow_mgr.borrow().peek().unwrap(), Frame::MaxStreamData{..})); - s.inbound_stream_frame(true, RX_STREAM_DATA_WINDOW, vec![]) + s.inbound_stream_frame(true, RX_STREAM_DATA_WINDOW, &[]) .unwrap(); assert!(matches!(s.flow_mgr.borrow().peek(), None)); } @@ -1065,10 +1081,10 @@ mod tests { let flow_mgr = Rc::new(RefCell::new(FlowMgr::default())); let conn_events = ConnectionEvents::default(); - let frame1 = vec![0; RX_STREAM_DATA_WINDOW as usize]; - + let frame1 = &[0; RECV_BUFFER_SIZE]; + let stream_id = StreamId::from(67); let mut s = RecvStream::new( - 67.into(), + stream_id, RX_STREAM_DATA_WINDOW, Rc::clone(&flow_mgr), conn_events, @@ -1076,7 +1092,7 @@ mod tests { // A flow control update is queued s.inbound_stream_frame(false, 0, frame1).unwrap(); - flow_mgr.borrow_mut().max_stream_data(67.into(), 100); + flow_mgr.borrow_mut().max_stream_data(stream_id, 100); // Generates frame assert!(matches!( s.flow_mgr.borrow_mut().next().unwrap(), @@ -1088,7 +1104,7 @@ mod tests { s.maybe_send_flowc_update(); assert!(matches!(s.flow_mgr.borrow().peek(), None)); // But if lost, another frame is generated - flow_mgr.borrow_mut().max_stream_data(67.into(), 100); + flow_mgr.borrow_mut().max_stream_data(stream_id, 100); assert!(matches!(s.flow_mgr.borrow_mut().next().unwrap(), Frame::MaxStreamData{..})); } } diff --git a/third_party/rust/neqo-transport/src/send_stream.rs b/third_party/rust/neqo-transport/src/send_stream.rs index 62c84434e721..48d3d63c7f98 100644 --- a/third_party/rust/neqo-transport/src/send_stream.rs +++ b/third_party/rust/neqo-transport/src/send_stream.rs @@ -16,14 +16,15 @@ use std::rc::Rc; use indexmap::IndexMap; use smallvec::SmallVec; -use neqo_common::{qdebug, qerror, qinfo, qtrace}; +use neqo_common::{qdebug, qerror, qinfo, qtrace, Encoder}; use crate::events::ConnectionEvents; use crate::flow_mgr::FlowMgr; use crate::frame::Frame; +use crate::packet::PacketBuilder; use crate::recovery::RecoveryToken; +use crate::stats::FrameStats; use crate::stream_id::StreamId; -use crate::tracking::PNSpace; use crate::{AppError, Error, Res}; pub const SEND_BUFFER_SIZE: usize = 0x10_0000; // 1 MiB @@ -289,10 +290,7 @@ pub struct TxBuffer { impl TxBuffer { pub fn new() -> Self { - Self { - send_buf: VecDeque::with_capacity(SEND_BUFFER_SIZE), - ..Self::default() - } + Self::default() } /// Attempt to add some or all of the passed-in buffer to the TxBuffer. @@ -511,6 +509,80 @@ impl SendStream { } } + /// Calculate how many bytes (length) can fit into available space and whether + /// the remainder of the space can be filled (or if a length field is needed). + fn length_and_fill(data_len: usize, space: usize) -> (usize, bool) { + if data_len >= space { + // Either more data than space allows, or an exact fit. + qtrace!("SendStream::length_and_fill fill {}", space); + return (space, true); + } + + // Estimate size of the length field based on the available space, + // less 1, which is the worst case. + let length = min(space.saturating_sub(1), data_len); + let length_len = Encoder::varint_len(u64::try_from(length).unwrap()); + if length_len > space { + qtrace!( + "SendStream::length_and_fill no room for length of {} in {}", + length, + space + ); + return (0, false); + } + + let length = min(data_len, space - length_len); + qtrace!("SendStream::length_and_fill {} in {}", length, space); + (length, false) + } + + pub fn write_frame(&mut self, builder: &mut PacketBuilder) -> Option { + let id = self.stream_id; + let final_size = self.final_size(); + if let Some((offset, data)) = self.next_bytes() { + let overhead = 1 // Frame type + + Encoder::varint_len(id.as_u64()) + + if offset > 0 { + Encoder::varint_len(offset) + } else { + 0 + }; + if overhead > builder.remaining() { + qtrace!("SendStream::write_frame no space for header"); + return None; + } + + let (length, fill) = Self::length_and_fill(data.len(), builder.remaining() - overhead); + let fin = final_size.map_or(false, |fs| fs == offset + u64::try_from(length).unwrap()); + if length == 0 && !fin { + qtrace!("SendStream::write_frame no data, no fin"); + return None; + } + + // Write the stream out. + builder.encode_varint(Frame::stream_type(fin, offset > 0, fill)); + builder.encode_varint(id.as_u64()); + if offset > 0 { + builder.encode_varint(offset); + } + if fill { + builder.encode(&data[..length]); + } else { + builder.encode_vvec(&data[..length]); + } + + self.mark_as_sent(offset, length, fin); + Some(RecoveryToken::Stream(StreamRecoveryToken { + id, + offset, + length, + fin, + })) + } else { + None + } + } + pub fn mark_as_sent(&mut self, offset: u64, len: usize, fin: bool) { if let Some(buf) = self.state.tx_buf_mut() { buf.mark_as_sent(offset, len); @@ -791,49 +863,18 @@ impl SendStreams { self.0.retain(|_, stream| !stream.is_terminal()) } - pub(crate) fn get_frame( + pub(crate) fn write_frames( &mut self, - space: PNSpace, - remaining: usize, - ) -> Option<(Frame, Option)> { - if space != PNSpace::ApplicationData { - return None; - } - - for (stream_id, stream) in self { - let final_size = stream.final_size(); - if let Some((offset, data)) = stream.next_bytes() { - let data_len = u64::try_from(data.len()).unwrap(); - let range_has_fin = final_size - .map(|fs| fs == offset + data_len) - .unwrap_or(false); - if let Some((frame, length)) = - Frame::new_stream(stream_id.as_u64(), offset, data, range_has_fin, remaining) - { - qdebug!( - "Stream {} sending bytes {}-{}, space {:?}", - stream_id.as_u64(), - offset, - offset + length as u64, - space, - ); - let fin = range_has_fin && length == data.len(); - debug_assert!(!fin || matches!(frame, Frame::Stream{fin: true, .. })); - stream.mark_as_sent(offset, length, fin); - - return Some(( - frame, - Some(RecoveryToken::Stream(StreamRecoveryToken { - id: *stream_id, - offset, - length, - fin, - })), - )); - } + builder: &mut PacketBuilder, + tokens: &mut Vec, + stats: &mut FrameStats, + ) { + for (_, stream) in self { + if let Some(t) = stream.write_frame(builder) { + tokens.push(t); + stats.stream += 1; } } - None } } @@ -859,7 +900,7 @@ mod tests { use super::*; use crate::events::ConnectionEvent; - use neqo_common::event::Provider; + use neqo_common::{event::Provider, hex_with_len, qtrace}; #[test] fn test_mark_range() { @@ -1266,39 +1307,65 @@ mod tests { s.close(); let mut ss = SendStreams::default(); - ss.insert(0.into(), s); + ss.insert(StreamId::from(0), s); - let (_f1, f1_token) = ss.get_frame(PNSpace::ApplicationData, 6).unwrap(); - assert!(matches!(&f1_token, Some(RecoveryToken::Stream(x)) if !x.fin)); - let (_f2, f2_token) = ss.get_frame(PNSpace::ApplicationData, 100).unwrap(); - assert!(matches!(&f2_token, Some(RecoveryToken::Stream(x)) if x.fin)); + let mut tokens = Vec::new(); + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); - // Should be no more data to frame - let f3 = ss.get_frame(PNSpace::ApplicationData, 100); - assert!(matches!(f3, None)); + // Write a small frame: no fin. + let written = builder.len(); + builder.set_limit(written + 6); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + assert_eq!(builder.len(), written + 6); + assert_eq!(tokens.len(), 1); + let f1_token = tokens.remove(0); + assert!(matches!(&f1_token, RecoveryToken::Stream(x) if !x.fin)); + + // Write the rest: fin. + let written = builder.len(); + builder.set_limit(written + 200); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + assert_eq!(builder.len(), written + 10); + assert_eq!(tokens.len(), 1); + let f2_token = tokens.remove(0); + assert!(matches!(&f2_token, RecoveryToken::Stream(x) if x.fin)); + + // Should be no more data to frame. + let written = builder.len(); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + assert_eq!(builder.len(), written); + assert!(tokens.is_empty()); // Mark frame 1 as lost - let f1_token = match f1_token { - Some(RecoveryToken::Stream(rt)) => rt, - _ => panic!(), - }; - ss.lost(&f1_token); + if let RecoveryToken::Stream(rt) = f1_token { + ss.lost(&rt); + } else { + panic!(); + } // Next frame should not set fin even though stream has fin but frame // does not include end of stream - let (_f4, f4_token) = ss.get_frame(PNSpace::ApplicationData, 100).unwrap(); - assert!(matches!(f4_token, Some(RecoveryToken::Stream(x)) if !x.fin)); + let written = builder.len(); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + assert_eq!(builder.len(), written + 7); // Needs a length this time. + assert_eq!(tokens.len(), 1); + let f4_token = tokens.remove(0); + assert!(matches!(&f4_token, RecoveryToken::Stream(x) if !x.fin)); // Mark frame 2 as lost - let f2_token = match f2_token { - Some(RecoveryToken::Stream(rt)) => rt, - _ => panic!(), - }; - ss.lost(&f2_token); + if let RecoveryToken::Stream(rt) = f2_token { + ss.lost(&rt); + } else { + panic!(); + } // Next frame should set fin because it includes end of stream - let (_f5, f5_token) = ss.get_frame(PNSpace::ApplicationData, 100).unwrap(); - assert!(matches!(f5_token, Some(RecoveryToken::Stream(x)) if x.fin)); + let written = builder.len(); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + assert_eq!(builder.len(), written + 10); + assert_eq!(tokens.len(), 1); + let f5_token = tokens.remove(0); + assert!(matches!(&f5_token, RecoveryToken::Stream(x) if x.fin)); } #[test] @@ -1313,49 +1380,55 @@ mod tests { s.send(&[0; 10]).unwrap(); let mut ss = SendStreams::default(); - ss.insert(0.into(), s); + ss.insert(StreamId::from(0), s); - let (_f1, f1_token) = ss.get_frame(PNSpace::ApplicationData, 100).unwrap(); - assert!(matches!(&f1_token, Some(RecoveryToken::Stream(x)) if x.offset == 0)); - assert!(matches!(&f1_token, Some(RecoveryToken::Stream(x)) if x.length == 10)); - assert!(matches!(&f1_token, Some(RecoveryToken::Stream(x)) if !x.fin)); + let mut tokens = Vec::new(); + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + let f1_token = tokens.remove(0); + assert!(matches!(&f1_token, RecoveryToken::Stream(x) if x.offset == 0)); + assert!(matches!(&f1_token, RecoveryToken::Stream(x) if x.length == 10)); + assert!(matches!(&f1_token, RecoveryToken::Stream(x) if !x.fin)); // Should be no more data to frame - let f2 = ss.get_frame(PNSpace::ApplicationData, 100); - assert!(matches!(f2, None)); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + assert!(tokens.is_empty()); - ss.get_mut(0.into()).unwrap().close(); + ss.get_mut(StreamId::from(0)).unwrap().close(); - let (_f2, f2_token) = ss.get_frame(PNSpace::ApplicationData, 100).unwrap(); - assert!(matches!(&f2_token, Some(RecoveryToken::Stream(x)) if x.offset == 10)); - assert!(matches!(&f2_token, Some(RecoveryToken::Stream(x)) if x.length == 0)); - assert!(matches!(&f2_token, Some(RecoveryToken::Stream(x)) if x.fin)); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + let f2_token = tokens.remove(0); + assert!(matches!(&f2_token, RecoveryToken::Stream(x) if x.offset == 10)); + assert!(matches!(&f2_token, RecoveryToken::Stream(x) if x.length == 0)); + assert!(matches!(&f2_token, RecoveryToken::Stream(x) if x.fin)); // Mark frame 2 as lost - let f2_token = match f2_token { - Some(RecoveryToken::Stream(rt)) => rt, - _ => panic!(), - }; - ss.lost(&f2_token); + if let RecoveryToken::Stream(rt) = f2_token { + ss.lost(&rt); + } else { + panic!(); + } // Next frame should set fin - let (_f3, f3_token) = ss.get_frame(PNSpace::ApplicationData, 100).unwrap(); - assert!(matches!(&f3_token, Some(RecoveryToken::Stream(x)) if x.offset == 10)); - assert!(matches!(&f3_token, Some(RecoveryToken::Stream(x)) if x.length == 0)); - assert!(matches!(&f3_token, Some(RecoveryToken::Stream(x)) if x.fin)); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + let f3_token = tokens.remove(0); + assert!(matches!(&f3_token, RecoveryToken::Stream(x) if x.offset == 10)); + assert!(matches!(&f3_token, RecoveryToken::Stream(x) if x.length == 0)); + assert!(matches!(&f3_token, RecoveryToken::Stream(x) if x.fin)); // Mark frame 1 as lost - let f1_token = match f1_token { - Some(RecoveryToken::Stream(rt)) => rt, - _ => panic!(), - }; - ss.lost(&f1_token); + if let RecoveryToken::Stream(rt) = f1_token { + ss.lost(&rt); + } else { + panic!(); + } // Next frame should set fin and include all data - let (_f4, f4_token) = ss.get_frame(PNSpace::ApplicationData, 100).unwrap(); - assert!(matches!(&f4_token, Some(RecoveryToken::Stream(x)) if x.offset == 0)); - assert!(matches!(&f4_token, Some(RecoveryToken::Stream(x)) if x.length == 10)); - assert!(matches!(&f4_token, Some(RecoveryToken::Stream(x)) if x.fin)); + ss.write_frames(&mut builder, &mut tokens, &mut FrameStats::default()); + let f4_token = tokens.remove(0); + assert!(matches!(&f4_token, RecoveryToken::Stream(x) if x.offset == 0)); + assert!(matches!(&f4_token, RecoveryToken::Stream(x) if x.length == 10)); + assert!(matches!(&f4_token, RecoveryToken::Stream(x) if x.fin)); } #[test] @@ -1364,7 +1437,8 @@ mod tests { flow_mgr.borrow_mut().conn_increase_max_credit(5); let conn_events = ConnectionEvents::default(); - let mut s = SendStream::new(4.into(), 0, Rc::clone(&flow_mgr), conn_events); + let stream_id = StreamId::from(4); + let mut s = SendStream::new(stream_id, 0, Rc::clone(&flow_mgr), conn_events); s.set_max_stream_data(2); // Stream is initially blocked (conn:5, stream:2) @@ -1375,7 +1449,7 @@ mod tests { assert_eq!( flow_mgr.borrow_mut().next().unwrap(), Frame::StreamDataBlocked { - stream_id: 4.into(), + stream_id, stream_data_limit: 0x2 } ); @@ -1391,7 +1465,7 @@ mod tests { assert_eq!( flow_mgr.borrow_mut().next().unwrap(), Frame::StreamDataBlocked { - stream_id: 4.into(), + stream_id, stream_data_limit: 0x2 } ); @@ -1457,12 +1531,12 @@ mod tests { const MESSAGE: &[u8] = b"hello"; let len_u64 = u64::try_from(MESSAGE.len()).unwrap(); - let flow_mgr = Rc::new(RefCell::new(FlowMgr::default())); - flow_mgr.borrow_mut().conn_increase_max_credit(len_u64); + let mut flow_mgr = FlowMgr::default(); + flow_mgr.conn_increase_max_credit(len_u64); let conn_events = ConnectionEvents::default(); let id = StreamId::new(100); - let mut s = SendStream::new(id, 0, Rc::clone(&flow_mgr), conn_events); + let mut s = SendStream::new(id, 0, Rc::new(RefCell::new(flow_mgr)), conn_events); s.set_max_stream_data(len_u64); // Send all the data, then the fin. @@ -1476,8 +1550,197 @@ mod tests { s.mark_as_lost(len_u64, 0, true); // No frame should be sent here. - let mut builder = SendStreams(IndexMap::default()); - builder.insert(id, s); - assert!(builder.get_frame(PNSpace::ApplicationData, 1000).is_none()); + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); + assert!(s.write_frame(&mut builder).is_none()); + } + + /// Create a `SendStream` and force it into a state where it believes that + /// `offset` bytes have already been sent and acknowledged. + fn stream_with_sent(stream: u64, offset: usize) -> SendStream { + const MAX_VARINT: u64 = (1 << 62) - 1; + + let mut flow_mgr = FlowMgr::default(); + flow_mgr.conn_increase_max_credit(MAX_VARINT); + + let mut s = SendStream::new( + StreamId::from(stream), + MAX_VARINT, + Rc::new(RefCell::new(flow_mgr)), + ConnectionEvents::default(), + ); + + let mut send_buf = TxBuffer::new(); + send_buf.retired = u64::try_from(offset).unwrap(); + send_buf.ranges.mark_range(0, offset, RangeState::Acked); + s.state = SendStreamState::Send { send_buf }; + s + } + + fn frame_sent_sid(stream: u64, offset: usize, len: usize, fin: bool, space: usize) -> bool { + const BUF: &[u8] = &[0x42; 128]; + let mut s = stream_with_sent(stream, offset); + + // Now write out the proscribed data and maybe close. + if len > 0 { + s.send(&BUF[..len]).unwrap(); + } + if fin { + s.close(); + } + + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); + let header_len = builder.len(); + builder.set_limit(header_len + space); + let token = s.write_frame(&mut builder); + qtrace!("STREAM frame: {}", hex_with_len(&builder[header_len..])); + token.is_some() + } + + fn frame_sent(offset: usize, len: usize, fin: bool, space: usize) -> bool { + frame_sent_sid(0, offset, len, fin, space) + } + + #[test] + fn stream_frame_empty() { + // Stream frames with empty data and no fin never work. + assert!(!frame_sent(10, 0, false, 2)); + assert!(!frame_sent(10, 0, false, 3)); + assert!(!frame_sent(10, 0, false, 4)); + assert!(!frame_sent(10, 0, false, 5)); + assert!(!frame_sent(10, 0, false, 100)); + + // Empty data with fin is only a problem if there is no space. + assert!(!frame_sent(0, 0, true, 1)); + assert!(frame_sent(0, 0, true, 2)); + assert!(!frame_sent(10, 0, true, 2)); + assert!(frame_sent(10, 0, true, 3)); + assert!(frame_sent(10, 0, true, 4)); + assert!(frame_sent(10, 0, true, 5)); + assert!(frame_sent(10, 0, true, 100)); + } + + #[test] + fn stream_frame_minimum() { + // Add minimum data + assert!(!frame_sent(10, 1, false, 3)); + assert!(!frame_sent(10, 1, true, 3)); + assert!(frame_sent(10, 1, false, 4)); + assert!(frame_sent(10, 1, true, 4)); + assert!(frame_sent(10, 1, false, 5)); + assert!(frame_sent(10, 1, true, 5)); + assert!(frame_sent(10, 1, false, 100)); + assert!(frame_sent(10, 1, true, 100)); + } + + #[test] + fn stream_frame_more() { + // Try more data + assert!(!frame_sent(10, 100, false, 3)); + assert!(!frame_sent(10, 100, true, 3)); + assert!(frame_sent(10, 100, false, 4)); + assert!(frame_sent(10, 100, true, 4)); + assert!(frame_sent(10, 100, false, 5)); + assert!(frame_sent(10, 100, true, 5)); + assert!(frame_sent(10, 100, false, 100)); + assert!(frame_sent(10, 100, true, 100)); + + assert!(frame_sent(10, 100, false, 1000)); + assert!(frame_sent(10, 100, true, 1000)); + } + + #[test] + fn stream_frame_big_id() { + // A value that encodes to the largest varint. + const BIG: u64 = 1 << 30; + const BIGSZ: usize = 1 << 30; + + assert!(!frame_sent_sid(BIG, BIGSZ, 0, false, 16)); + assert!(!frame_sent_sid(BIG, BIGSZ, 0, true, 16)); + assert!(!frame_sent_sid(BIG, BIGSZ, 0, false, 17)); + assert!(frame_sent_sid(BIG, BIGSZ, 0, true, 17)); + assert!(!frame_sent_sid(BIG, BIGSZ, 0, false, 18)); + assert!(frame_sent_sid(BIG, BIGSZ, 0, true, 18)); + + assert!(!frame_sent_sid(BIG, BIGSZ, 1, false, 17)); + assert!(!frame_sent_sid(BIG, BIGSZ, 1, true, 17)); + assert!(frame_sent_sid(BIG, BIGSZ, 1, false, 18)); + assert!(frame_sent_sid(BIG, BIGSZ, 1, true, 18)); + assert!(frame_sent_sid(BIG, BIGSZ, 1, false, 19)); + assert!(frame_sent_sid(BIG, BIGSZ, 1, true, 19)); + assert!(frame_sent_sid(BIG, BIGSZ, 1, false, 100)); + assert!(frame_sent_sid(BIG, BIGSZ, 1, true, 100)); + } + + #[test] + fn stream_frame_16384() { + const DATA16384: &[u8] = &[0x43; 16384]; + + // 16383/16384 is an odd boundary in STREAM frame construction. + // That is the boundary where a length goes from 2 bytes to 4 bytes. + // If the data fits in the available space, then it is simple: + let mut s = stream_with_sent(0, 0); + s.send(DATA16384).unwrap(); + s.close(); + + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); + let header_len = builder.len(); + builder.set_limit(header_len + DATA16384.len() + 2); + let token = s.write_frame(&mut builder); + assert!(token.is_some()); + // Expect STREAM + FIN only. + assert_eq!(&builder[header_len..header_len + 2], &[0b1001, 0]); + assert_eq!(&builder[header_len + 2..], DATA16384); + + s.mark_as_lost(0, DATA16384.len(), true); + + // However, if there is one extra byte of space, we will try to add a length. + // That length will then make the frame to be too large and the data will be + // truncated. The frame could carry one more byte of data, but it's a corner + // case we don't want to address as it should be rare (if not impossible). + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); + let header_len = builder.len(); + builder.set_limit(header_len + DATA16384.len() + 3); + let token = s.write_frame(&mut builder); + assert!(token.is_some()); + // Expect STREAM + LEN + FIN. + assert_eq!( + &builder[header_len..header_len + 4], + &[0b1010, 0, 0x7f, 0xfd] + ); + assert_eq!( + &builder[header_len + 4..], + &DATA16384[..DATA16384.len() - 3] + ); + } + + #[test] + fn stream_frame_64() { + const DATA64: &[u8] = &[0x43; 64]; + + // Unlike 16383/16384, the boundary at 63/64 is easy because the difference + // is just one byte. We lose just the last byte when there is more space. + let mut s = stream_with_sent(0, 0); + s.send(DATA64).unwrap(); + s.close(); + + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); + let header_len = builder.len(); + builder.set_limit(header_len + 66); + let token = s.write_frame(&mut builder); + assert!(token.is_some()); + // Expect STREAM + FIN only. + assert_eq!(&builder[header_len..header_len + 2], &[0b1001, 0]); + assert_eq!(&builder[header_len + 2..], DATA64); + + s.mark_as_lost(0, DATA64.len(), true); + + let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); + let header_len = builder.len(); + builder.set_limit(header_len + 67); + let token = s.write_frame(&mut builder); + assert!(token.is_some()); + // Expect STREAM + LEN, not FIN. + assert_eq!(&builder[header_len..header_len + 3], &[0b1010, 0, 63]); + assert_eq!(&builder[header_len + 3..], &DATA64[..63]); } } diff --git a/third_party/rust/neqo-transport/src/stats.rs b/third_party/rust/neqo-transport/src/stats.rs index 73f0e4f3fdef..853a514f1024 100644 --- a/third_party/rust/neqo-transport/src/stats.rs +++ b/third_party/rust/neqo-transport/src/stats.rs @@ -7,6 +7,7 @@ // Tracking of some useful statistics. #![deny(clippy::pedantic)] +use crate::packet::PacketNumber; use neqo_common::qinfo; use std::cell::RefCell; use std::fmt::{self, Debug}; @@ -15,6 +16,39 @@ use std::rc::Rc; pub(crate) const MAX_PTO_COUNTS: usize = 16; +#[derive(Default, Clone)] +#[allow(clippy::module_name_repetitions)] +pub struct FrameStats { + pub all: usize, + pub ack: usize, + pub largest_acknowledged: PacketNumber, + + pub crypto: usize, + pub stream: usize, + pub reset_stream: usize, + pub stop_sending: usize, + + pub ping: usize, + pub padding: usize, + + pub max_streams: usize, + pub streams_blocked: usize, + pub max_data: usize, + pub data_blocked: usize, + pub max_stream_data: usize, + pub stream_data_blocked: usize, + + pub new_connection_id: usize, + pub retire_connection_id: usize, + + pub path_challenge: usize, + pub path_response: usize, + + pub connection_close: usize, + pub handshake_done: usize, + pub new_token: usize, +} + /// Connection statistics #[derive(Default, Clone)] #[allow(clippy::module_name_repetitions)] @@ -46,6 +80,11 @@ pub struct Stats { /// Count PTOs. Single PTOs, 2 PTOs in a row, 3 PTOs in row, etc. are counted /// separately. pub pto_counts: [usize; MAX_PTO_COUNTS], + + /// Count frames received. + pub frame_rx: FrameStats, + /// Count frames sent. + pub frame_tx: FrameStats, } impl Stats { diff --git a/third_party/rust/neqo-transport/src/tracking.rs b/third_party/rust/neqo-transport/src/tracking.rs index 02c8103922d5..0b14fe8ad514 100644 --- a/third_party/rust/neqo-transport/src/tracking.rs +++ b/third_party/rust/neqo-transport/src/tracking.rs @@ -20,6 +20,7 @@ use neqo_crypto::{Epoch, TLS_EPOCH_HANDSHAKE, TLS_EPOCH_INITIAL}; use crate::packet::{PacketBuilder, PacketNumber, PacketType}; use crate::recovery::RecoveryToken; +use crate::stats::FrameStats; use smallvec::{smallvec, SmallVec}; @@ -495,7 +496,12 @@ impl RecvdPackets { /// /// We don't send ranges that have been acknowledged, but they still need /// to be tracked so that duplicates can be detected. - fn write_frame(&mut self, now: Instant, builder: &mut PacketBuilder) -> Option { + fn write_frame( + &mut self, + now: Instant, + builder: &mut PacketBuilder, + stats: &mut FrameStats, + ) -> Option { // The worst possible ACK frame, assuming only one range. // Note that this assumes one byte for the type and count of extra ranges. const LONGEST_ACK_HEADER: usize = 1 + 8 + 8 + 1 + 8; @@ -533,6 +539,8 @@ impl RecvdPackets { None => return None, // Nothing to send. }; builder.encode_varint(first.largest); + stats.largest_acknowledged = first.largest; + stats.ack += 1; let elapsed = now.duration_since(self.largest_pn_time.unwrap()); // We use the default exponent, so delay is in multiples of 8 microseconds. @@ -626,9 +634,10 @@ impl AckTracker { pn_space: PNSpace, now: Instant, builder: &mut PacketBuilder, + stats: &mut FrameStats, ) -> Option { self.get_mut(pn_space) - .and_then(|space| space.write_frame(now, builder)) + .and_then(|space| space.write_frame(now, builder, stats)) } } @@ -652,6 +661,7 @@ mod tests { }; use crate::frame::Frame; use crate::packet::PacketBuilder; + use crate::stats::FrameStats; use lazy_static::lazy_static; use neqo_common::Encoder; use std::collections::HashSet; @@ -837,7 +847,12 @@ mod tests { .set_received(*NOW, 0, true); // The reference time for `ack_time` has to be in the past or we filter out the timer. assert!(tracker.ack_time(*NOW - Duration::from_millis(1)).is_some()); - let token = tracker.write_frame(PNSpace::Initial, *NOW, &mut builder); + let token = tracker.write_frame( + PNSpace::Initial, + *NOW, + &mut builder, + &mut FrameStats::default(), + ); assert!(token.is_some()); // Mark another packet as received so we have cause to send another ACK in that space. @@ -853,7 +868,12 @@ mod tests { assert!(tracker.get_mut(PNSpace::Initial).is_none()); assert!(tracker.ack_time(*NOW - Duration::from_millis(1)).is_none()); assert!(tracker - .write_frame(PNSpace::Initial, *NOW, &mut builder) + .write_frame( + PNSpace::Initial, + *NOW, + &mut builder, + &mut FrameStats::default() + ) .is_none()); if let RecoveryToken::Ack(tok) = token.unwrap() { tracker.acked(&tok); // Should be a noop. @@ -874,7 +894,12 @@ mod tests { let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); builder.set_limit(10); - let token = tracker.write_frame(PNSpace::Initial, *NOW, &mut builder); + let token = tracker.write_frame( + PNSpace::Initial, + *NOW, + &mut builder, + &mut FrameStats::default(), + ); assert!(token.is_none()); assert_eq!(builder.len(), 1); // Only the short packet header has been added. } @@ -895,7 +920,12 @@ mod tests { let mut builder = PacketBuilder::short(Encoder::new(), false, &[]); builder.set_limit(32); - let token = tracker.write_frame(PNSpace::Initial, *NOW, &mut builder); + let token = tracker.write_frame( + PNSpace::Initial, + *NOW, + &mut builder, + &mut FrameStats::default(), + ); assert!(token.is_some()); let mut dec = builder.as_decoder(); diff --git a/third_party/rust/neqo-transport/tests/connection.rs b/third_party/rust/neqo-transport/tests/connection.rs index 61d60689d6a6..aca7c3ce9b2f 100644 --- a/third_party/rust/neqo-transport/tests/connection.rs +++ b/third_party/rust/neqo-transport/tests/connection.rs @@ -27,7 +27,13 @@ fn truncate_long_packet() { // This will truncate the Handshake packet from the server. let dupe = dgram.as_ref().unwrap().clone(); - let truncated = Datagram::new(dupe.source(), dupe.destination(), &dupe[..(dupe.len() - 1)]); + // Count the padding in the packet, plus 1. + let tail = dupe.iter().rev().take_while(|b| **b == 0).count() + 1; + let truncated = Datagram::new( + dupe.source(), + dupe.destination(), + &dupe[..(dupe.len() - tail)], + ); let dupe_ack = client.process(Some(truncated), now()).dgram(); assert!(dupe_ack.is_some()); diff --git a/third_party/rust/neqo-transport/tests/server.rs b/third_party/rust/neqo-transport/tests/server.rs index fb1b13d6ed96..e332b54ac56f 100644 --- a/third_party/rust/neqo-transport/tests/server.rs +++ b/third_party/rust/neqo-transport/tests/server.rs @@ -20,7 +20,7 @@ use neqo_transport::{ Connection, ConnectionError, ConnectionEvent, Error, FixedConnectionIdManager, Output, QuicVersion, State, StreamType, }; -use test_fixture::{self, assertions, default_client, loopback, now}; +use test_fixture::{self, assertions, default_client, loopback, now, split_datagram}; use std::cell::RefCell; use std::convert::TryFrom; @@ -472,11 +472,8 @@ fn retry_after_initial() { assert!(server_flight.is_some()); // We need to have the client just process the Initial. - // Rather than try to find the Initial, we can just truncate the Handshake that follows. - let si = server_flight.as_ref().unwrap(); - let truncated = &si[..(si.len() - 1)]; - let just_initial = Datagram::new(si.source(), si.destination(), truncated); - let dgram = client.process(Some(just_initial), now()).dgram(); + let (server_initial, _other) = split_datagram(server_flight.as_ref().unwrap()); + let dgram = client.process(Some(server_initial), now()).dgram(); assert!(dgram.is_some()); assert!(*client.state() != State::Connected);