Conversation
) Add 5 tests and extend the fixture builder for issue #52's 3 edge cases: Builder changes: - Add build_client_hello_with_typed_sni_list(&[(u8, &[u8])]) for explicit NameType per entry (non-zero NameType testing) - Refactor build_client_hello_with_sni_list to delegate to typed variant - Add build_client_hello_with_raw_sni_ext for hand-crafted SNI framing Tests added: - test_non_zero_name_type_sni_entry: NameType=1 is included by tls_parser and treated as hostname by extract_sni (pinned) - test_non_zero_name_type_with_valid_first_entry: mixed types, first entry (type 0) takes priority - test_large_sni_near_record_payload_limit: 16KB hostname parses OK - test_oversized_sni_exceeds_record_payload_limit: record rejected at 18,432-byte payload limit, parse error incremented - test_trailing_bytes_in_server_name_list: tls_parser accepts malformed framing with trailing garbage (pinned) Closes #52
Address code review: raw SNI extension builder now uses checked_mul(2) for cipher byte length, consistent with the typed builder. Also fix the self-correcting "name_len=11 (wait, 12 bytes)" comment.
There was a problem hiding this comment.
Pull request overview
Adds TLS SNI “pin” tests to cover edge cases called out in issue #52, ensuring the analyzer’s behavior is documented and future tls_parser upgrades that change parsing semantics are caught by CI.
Changes:
- Refactors
build_client_hello_with_sni_listto delegate to a new typed SNI builder that allows explicitNameType. - Adds a raw-SNI-extension ClientHello builder to craft malformed/non-standard SNI framing (e.g., trailing bytes).
- Introduces 5 new tests covering non-zero
NameType, large/oversized SNI vsMAX_RECORD_PAYLOAD, and trailing bytes inServerNameList.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review: replace `as u16` casts with u16::try_from in the trailing-bytes test for consistency with the builder functions.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot round 2 feedback: - Add explicit payload-length assertion in the oversized-SNI test so fixture drift (overhead estimate changes) fails loudly instead of passing vacuously. - Clarify in comment that the large-SNI test pins size-handling only and does not care about DNS label validity.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot round 3: - Add local const MAX_RECORD_PAYLOAD in the oversize test, mirroring the pattern used by test_non_utf8_sni_finding_fires_when_sni_counts_at_capacity for MAX_MAP_ENTRIES. - Clarify the typed SNI builder doc: bytes are a hostname only for NameType=host_name(0); for other types the bytes are opaque ServerName payload per the RFC 6066 enum.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot round 4: the builder now accepts non-host_name(0) NameTypes where bytes are opaque ServerName payload, so the panic message should reference ServerName rather than RFC 6066 HostName.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
build_client_hello_with_sni_listto delegate to a new typed variant.Edge cases covered
test_non_zero_name_type_sni_entry,test_non_zero_name_type_with_valid_first_entrytest_large_sni_near_record_payload_limit,test_oversized_sni_exceeds_record_payload_limittest_trailing_bytes_in_server_name_listNote on u16 boundary (65K SNI)
The issue mentioned testing at the u16 max (65,535 bytes). This is unreachable through the production code path —
MAX_RECORD_PAYLOAD(18,432 bytes) rejects the record before SNI parsing begins. Thetest_oversized_sni_exceeds_record_payload_limittest pins the actual effective boundary.Closes #52
Test plan
cargo clippy --all-targetscleancargo fmt --checkclean