s2n-quic icon indicating copy to clipboard operation
s2n-quic copied to clipboard

fix(s2n-quic-transport): error during handshake on 32-bit platforms

Open chayleaf opened this issue 3 years ago • 2 comments
trafficstars

I'm not too knowledgeable on the internals of the library, but during handshake, acquire_flow_control_window returns 0x3fffffffffffffff, at least in my application. This doesn't fit in a 32-bit usize, so it caused an error in the previous implementation

Resolved issues:

resolves #1476

Description of changes:

Allow window size over u32::MAX on 32-bit platforms

Call-outs:

Testing:

Ideally, a 32-bit CI pipeline needs to be added. Personally, I haven't ran cargo test on x86, so there may be further issues; however, everything appears to work fine after this fix.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

chayleaf avatar Sep 03 '22 02:09 chayleaf

Can you add an x86 matrix entry in the ci as part of this change? It should be pretty straightforward to do:

https://github.com/aws/s2n-quic/blob/e5ae19ff115a09d29a8b041728393df85f0b69ea/.github/workflows/ci.yml#L221-L223

I'm fine with the all of the tests not passing; just as long as we start tracking it.

camshaft avatar Sep 03 '22 04:09 camshaft

I made the tests pass which required some changes to test RNG to make sure it doesn't depend on pointer size, and also fixed a bug in zerocopy_value_codec's DecoderValue::encoding_size implementation for references (which did cause issues for me as well, not sure if it can cause any issues on 64-bit platforms).

I also had to add a separate version of size_of snapshots for some structs containing pointers/usize, this causes CI to think some snapshots are unused, not sure what to do about it.

chayleaf avatar Sep 12 '22 11:09 chayleaf