8000 Fix connect-tests for 1000-sans.badssl.com by ctz · Pull Request #1324 · rustls/rustls · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix connect-tests for 1000-sans.badssl.com #1324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2023
Merged

Conversation

ctz
Copy link
Member
@ctz ctz commented Jun 23, 2023

This regressed in ae4ca32. DNS names starting with and wholly numerals are in wide use.

RFC1035 actually doesn't really reflect reality, and RFC1123 updates it to allow labels that start with numerals or are wholly numeric. The only restriction is that the last label cannot be wholly numeric.

Also:

  • Limit the entire length of a DNS name.
  • Ensure a label that ends with hyphens respects the label length limit.

(Both of these were found by writing a fuzzer that compared behaviour between the old and new implementations.)

This regressed in ae4ca32.  DNS names starting with
and wholly numerals are in wide use.

RFC1035 actually doesn't really reflect reality, and RFC1123
updates it to allow labels that start with numerals or are
wholly numeric.  The only restriction is that the last label
cannot be wholly numeric.

Also:

- Limit the entire length of a DNS name.
- Ensure a label that ends with hyphens respects the label length limit.
@codecov
Copy link
codecov bot commented Jun 23, 2023

Codecov Report

Merging #1324 (86cac4f) into main (3a2c595) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1324   +/-   ##
=======================================
  Coverage   95.87%   95.87%           
=======================================
  Files          61       61           
  Lines       14556    14574   +18     
=======================================
+ Hits        13955    13973   +18     
  Misses        601      601           
Impacted Files Coverage Δ
rustls/src/dns_name.rs 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ctz ctz requested a review from cpu June 23, 2023 13:28
Copy link
Member
@cpu cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😓 the real world is complicated.

(Both of these were found by writing a fuzzer that compared behaviour between the old and new implementations.)

Really good idea 👍

@ctz ctz merged commit a591363 into main Jun 23, 2023
@ctz ctz deleted the jbp-fix-1000-sans-test branch June 23, 2023 14:30
@djc
Copy link
Member
djc commented Jun 23, 2023

Should we also add a link to the relevant section of the RFC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0