Skip to content

Fix isreadable stealing data, key re-parse, failed re-seed, empty CRT info, empty ALPN - #324

Merged
quinnj merged 1 commit into
masterfrom
jq/round6-fixes
Oct 23, 2026
Merged

quinnj merged 1 commit into
masterfrom
jq/round6-fixes

Conversation

@quinnj

@quinnj quinnj commented Oct 23, 2026

Copy link
Copy Markdown
Member

Five low-impact fixes from a sixth review pass. None affects HTTP.jl 1.x's default use; two are regressions from this week's unreleased PRs.

  • isreadable(ctx) stole a waiting reader's data. It decrypted arriving records even while another task waited in eof(ctx) for them, leaving that task waiting on an empty socket with the data already decrypted. With renegotiation on (Don't let a write wait forever after a read finishes the renegotiation #323), it blocked behind the reader instead. It now processes records only when it can take waitlock; a waiting reader processes them itself.
  • Parsing a key into a filled PKContext destroyed the old key. mbedtls parses only into an empty context; given one that holds a key, it frees that key and fails with "Bad input parameters". Configs using the key then failed every handshake, and RSA(pk) views read freed memory. parse_key!, parse_public_key!, parse_keyfile! and parse_public_keyfile! now throw an ArgumentError first.
  • A failed re-seed broke a working generator (regression from Re-seed a CtrDrbg in a fresh context; keep raw add_source! objects alive #322). seed! freed the context before seeding, so bad pdata or a failing entropy source left the generator unusable, and every config using it failed handshakes. It now seeds a fresh context and swaps it in only on success.
  • show(MbedTLS.CRT()) ran out of memory (regression from Fix ALPN lifetime, get_peer_cert, long cert info, and byte reads #302). For a CRT with no certificate, mbedtls reports "buffer too small" about its own internal buffer, and the growth loop doubled forever. Growth now stops at 16 MiB and the error is thrown.
  • set_alpn!(conf, String[]) made mbedtls send an empty ALPN extension, which RFC 7301 forbids and OpenSSL servers reject. It now throws an ArgumentError.

Also: the ssl_unsafe_read docstring now says it can wait for another task's read or write when renegotiation is on.

Each has a test; they fail on master. Passes on Julia 1.12, 1.10 and nightly locally.

Found by a sixth independent review pass; reproduced and checked here.

🤖 Generated with Claude Code

… re-seed, empty CRT info, empty ALPN

- isreadable(ctx) decrypted arriving records even while another task waited in eof(ctx)
  for them, leaving that task waiting on an empty socket (or, with renegotiation on,
  blocked behind it). It now processes them only when it can take waitlock.
- mbedtls parses a key only into an empty PKContext: given one that holds a key, it frees
  that key and fails, breaking configs and RSA(pk) views that use it. The parse functions
  now throw an ArgumentError first.
- seed! (since #322) freed the context before seeding, so a failed re-seed (bad pdata, a
  failing source) left a working generator unusable. It now seeds a fresh context and swaps
  it in only on success.
- show(::CRT) on an empty CRT grew its buffer until out of memory (since #302): mbedtls
  reports "buffer too small" for its own internal buffer there. Growth stops at 16 MiB.
- set_alpn!(conf, String[]) made mbedtls send an empty ALPN extension, which servers
  reject; it now throws an ArgumentError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.96%. Comparing base (fa3aa4e) to head (8571392).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #324      +/-   ##
==========================================
+ Coverage   89.82%   89.96%   +0.14%     
==========================================
  Files          12       12              
  Lines         924      937      +13     
==========================================
+ Hits          830      843      +13     
  Misses         94       94              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit ea6a756 into master Oct 23, 2026
9 checks passed
@quinnj
quinnj deleted the jq/round6-fixes branch October 23, 2026 23:03
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.

1 participant