lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-11-24 04:01:07 +03:00
Code Activity
8,691 Commits 17 Branches 122 Tags
56c0231b9ded6e167e57a0eea616be646debf043
Commit Graph

6 Commits

Author SHA1 Message Date
Aman Sharma
56c0231b9d Implement direct encap transport parameter negotiation + Fix build errors 
Summary:
This diff implements the transport parameter negotiation logic for direct encapsulation support on top of D77604174, addresses reviewer feedback by changing the connection state pointer to a reference, and **fixes critical build errors** caused by the constructor signature changes.

**Changes Made:**

1. **Client-side logic**: The client sends the `client_direct_encap` transport parameter with no value if `supportDirectEncap` is true.

2. **Server-side logic**: The server sends the `server_direct_encap` transport parameter if `directEncapAddress` is not null AND the client sent the `client_direct_encap` parameter. The value is the IP address bytes in network byte order.

3. **Pointer to Reference Change**: Changed `const QuicConnectionStateBase* conn_` to `const QuicConnectionStateBase& conn_` in ServerTransportParametersExtension as requested by reviewer feedback, since nullability is not possible (non-null is an invariant).

4. **🔧 Build Error Fixes**: Fixed multiple test files that were broken by the constructor signature changes:

**Build Fixes Applied:**

- **Fixed 3 critical build failures** that prevented compilation:
  - `fbcode//quic/facebook/mbed/test:mbed_client_handshake`
  - `fbcode//quic/fizz/client/handshake/test:fizz_client_handshake_test`
  - `fbcode//quic/server/handshake/test:ServerHandshakeTest`

- **Updated constructor calls** in test files to include the new `const QuicConnectionStateBase& conn` parameter
- **Fixed helper functions** like `constructServerTp()` to accept and pass connection state
- **Updated test classes** like `MalformedServerTransportParamsExt` to handle the new parameter

**Files Fixed:**
- `fbcode/quic/facebook/mbed/test/MbedClientHandshake.cpp` - Fixed 4 constructor calls and helper functions
- `fbcode/quic/fizz/client/handshake/test/FizzClientHandshakeTest.cpp` - Fixed constructor call
- `fbcode/quic/server/handshake/test/ServerHandshakeTest.cpp` - Fixed constructor call

**Test Results:**
- ✅ `buck test fbcode//quic/facebook/mbed/test:mbed_client_handshake` → Pass 7, Fail 0
- ✅ `buck test fbcode//quic/fizz/client/handshake/test:fizz_client_handshake_test` → Pass 12, Fail 0
- ✅ All previously failing builds now compile successfully

**Implementation Details:**

- Added `encodeIPAddressParameter()` function to handle IP address encoding (supports both IPv4 and IPv6)
- Modified `getSupportedExtTransportParams()` to include client-side direct encap logic
- Created new `getClientDependentExtTransportParams()` function that specifically handles server-side direct encap logic based on client parameters
- Updated `ServerTransportParametersExtension` to use the new function for adding client-dependent parameters
- Updated `ServerStateMachine` to pass connection state to the extension
- **Changed constructor parameter order**: `conn` parameter now comes before `customTransportParameters` to maintain C++ default parameter rules
- **Updated member initialization order**: Fixed to match class declaration order
- **Fixed all test constructors**: Updated test cases to provide connection state parameter

**Architecture:**

Instead of overloading `getSupportedExtTransportParams()` with two parameters, the solution now uses a dedicated `getClientDependentExtTransportParams()` function that:
- Only handles parameters that depend on client capabilities (currently `server_direct_encap`)
- Returns a clean list of parameters without duplicating base transport parameters
- Provides better separation of concerns and clearer function naming

**Unit Tests Added:**

- Comprehensive test suite in `fbcode/quic/handshake/test/TransportParametersTest.cpp`
- 8 test cases covering all client/server scenarios with IPv4/IPv6 support
- Tests verify parameter presence/absence and correct IP address byte encoding
- All tests pass successfully
- **Updated test infrastructure**: Fixed ServerTransportParametersTest.cpp to work with reference-based connection state

**Requirements Fulfilled:**
✅ Client sends `client_direct_encap` parameter with no value if `supportDirectEncap` is true
✅ Server sends `server_direct_encap` parameter with IP address bytes if conditions are met
✅ Changed connection state from pointer to reference as requested by reviewer
✅ **Fixed all build errors caused by constructor signature changes**
 ---
> Generated by [RACER](https://www.internalfb.com/wiki/RACER_(Risk-Aware_Code_Editing_and_Refactoring)/), powered by [Confucius](https://www.internalfb.com/wiki/Confucius/Analect/Shared_Analects/Confucius_Code_Assist_(CCA)/)
[Session](https://www.internalfb.com/confucius?entry_name=RACER&mode=Focused&namespace[0]=agentrix&session_id=8c84b14a-56a5-11f0-8e69-214e73924e50&tab=Chat), [Trace](https://www.internalfb.com/confucius?entry_name=RACER&mode=Focused&namespace[0]=agentrix&session_id=8c84b14a-56a5-11f0-8e69-214e73924e50&tab=Trace)
[Session](https://www.internalfb.com/confucius?entry_name=RACER&mode=Focused&namespace[0]=agentrix&session_id=439da8ee-5798-11f0-ace1b7dae9e7575d&tab=Chat), [Trace](https://www.internalfb.com/confucius?entry_name=RACER&mode=Focused&namespace[0]=agentrix&session_id=439da8ee-5798-11f0-ace1-b7dae9e7575d&tab=Trace)
[Session](https://www.internalfb.com/confucius?session_id=7ed2dc86-5847-11f0-8055-b73b775dc61a&tab=Chat), [Trace](https://www.internalfb.com/confucius?session_id=7ed2dc86-5847-11f0-8055-b73b775dc61a&tab=Trace)
[Session](https://www.internalfb.com/confucius?session_id=8bdc0a0c-584b-11f0-9977-35e1e0d6200a&tab=Chat), [Trace](https://www.internalfb.com/confucius?session_id=8bdc0a0c-584b-1f0-9977-35e1e0d6200a&tab=Trace)
**[Current Session](https://www.internalfb.com/confucius?session_id={{ session_id }}&tab=Chat), [Trace](https://www.internalfb.com/confucius?session_id={{ session_id }}&tab=Trace)**
[Session](https://www.internalfb.com/confucius?entry_name=RACER&mode=Focused&namespace[0]=agentrix&session_id=08290174-5b4d-11f0-ac9d-93447239bce3&tab=Chat), [Trace](https://www.internalfb.com/confucius?entry_name=RACER&mode=Focused&namespace[0]=agentrix&session_id=08290174-5b4d-11f0-ac9d-93447239bce3&tab=Trace)
[Session](https://www.internalfb.com/confucius?entry_name=RACER&mode=Focused&namespace[0]=agentrix&session_id=ded2f5f2-5b6d-11f0-b259-5db72d7f2f63&tab=Chat), [Trace](https://www.internalfb.com/confucius?entry_name=RACER&mode=Focused&namespace[0]=agentrix&session_id=ded2f5f2-5b6d-11f0-b259-5db72d7f2f63&tab=Trace)

Reviewed By: hanidamlaj

Differential Revision: D77605298

fbshipit-source-id: 22d3faffaa93f1aa57e05c984339ab3b2e817ac1
 2025-07-07 20:04:24 -07:00
Amaury Séchet
f8a545e32b Split out fizz specific transport parameters (#55) 
Summary:
As per title.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/55

Reviewed By: yangchi

Differential Revision: D17898087

Pulled By: mjoras

fbshipit-source-id: b7c2a7c8d0e9950344a0f9796a047c25d0abf87e
 2019-10-22 10:18:39 -07:00
Amaury Séchet
c567c867f5 Move fizz specific ciphers in their own file (#51) 
Summary:
Technically, there are not fizz specific but ssl specific. The goal being to package all of the ssl machinery together so it can be swapped in/out for something else.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/51

Test Plan:
Imported from GitHub, without a `Test Plan:` line.

 ---
## Proxygen Canary
Traffic Canary: https://our.intern.facebook.com/intern/traffic/canary?fbid=138091837536628
* elb.prod.lax3c01 - binary - 2019-10-10 15:19 - https://fburl.com/dyndash/htdojejn
* olb.prod.rprn0c01 - binary - 2019-10-10 15:19 - https://fburl.com/dyndash/u15l8f8q
* slb.prod_regional.rftw0c00 - binary - 2019-10-10 15:19 - https://fburl.com/dyndash/bxdm1w7r
* slb.regional.rpnb0c01 - binary - 2019-10-10 15:19 - https://fburl.com/dyndash/1nnpuznl
 ---

Reviewed By: mjoras

Differential Revision: D17831892

Pulled By: mjoras

fbshipit-source-id: 16402be805ba0b3cb4d2944317d7e19f85e531bc
 2019-10-16 12:11:58 -07:00
Matt Joras
2f528e68c0 Draft-22 invariants change 
Summary:
Draft-22 onwards uses two one byte length fields to encode connection ID length instead of one one byte length field.

To support this without disrupting existing clients we need to make our parsing version dependent. This diff accomplishes it by special casing the existing Facebook client QUIC version (0xfaceb000), and changing the default Facebook client version going forward to 0xfaceb001.

Note that this diff also changes the behavior of the ticket transport parameters. When we changed from draft-18 to draft-19 transport parameters I apparently forgot to update the ticket transport parameters to the new format.

Reviewed By: yangchi

Differential Revision: D16205090

fbshipit-source-id: e74a92fa959d308f4bb43bad76e58d4b58d07322
 2019-07-16 17:59:58 -07:00
Matt Joras
b2e1eedfd3 Mostly remove version negotiation 
Summary:
Draft-19 onwards effectively punted version negotiation to QUICv2. Now receiving version negotiation on clients is treated as an immediate termination of the connection.

The transport parameter format has also changed to no longer include any reference to the QUIC version. To avoid us (Facebook) having to turn off QUIC traffic in production, our server needs to be able to parse these transport parameters from our older clients. To achieve this when parsing the transport parameters we will, as a temporary measure, check for the Facebook QUIC version to determine which transport parameter format we are parsing. Luckily for us the version we chose maps nicely to an implausible length for the transport parameters (0xface).

Note that this diff still has the client send the old transport parameter format, so that the rollout can be staged.

Reviewed By: yangchi

Differential Revision: D15203481

fbshipit-source-id: dfaaddc3acc76434461b04430b82a0902138c060
 2019-05-10 12:55:28 -07:00
udippant
50d4939e9e Initial commit of mvfst 2019-04-22 23:42:46 -07:00