Skip to content

Conversation

samuel40791765
Copy link
Contributor

@samuel40791765 samuel40791765 commented Sep 9, 2025

Issues:

Addresses #2650

Description of changes:

Curl is setting a buffer size larger than the maximum buffer length that AWS-LC allows. 0x401e * 4 is inherently 65656, which is larger than the allowed 65535 and AWS-LC directly uses the largest size available (65535) if the consuming application sets a value larger than that. This limitation's due to a buffer size we inherited from upstream:

aws-lc/ssl/ssl_buffer.cc

Lines 32 to 34 in 5a834ec

// BIO uses int instead of size_t. No lengths will exceed SSLBUFFER_MAX_CAPACITY
// (uint16_t), so this will not overflow.
static_assert(SSLBUFFER_MAX_CAPACITY <= INT_MAX,
.

I've tested against the test that was failing for curl and the test seems to pass now (for all instances I've ran it). Running pytest -n auto -v ./http/test_10_proxy.py::TestProxy::test_10_08_upload_seq_large -rA had been failing 5/10 times prior to this change and it passes every time that I run it now.

Call-outs:

N/A

Testing:

New serde tests and buffer limit test.

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

skmcgrail
skmcgrail previously approved these changes Sep 9, 2025
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 88.57143% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.81%. Comparing base (3bf53ce) to head (0fd4dc8).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
ssl/ssl_buffer.cc 75.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2673      +/-   ##
==========================================
+ Coverage   78.79%   78.81%   +0.02%     
==========================================
  Files         667      667              
  Lines      113982   114078      +96     
  Branches    16030    16046      +16     
==========================================
+ Hits        89810    89914     +104     
+ Misses      23397    23389       -8     
  Partials      775      775              

☔ View full report in Codecov by Sentry.
📢 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.

@samuel40791765 samuel40791765 enabled auto-merge (squash) September 9, 2025 22:54
@samuel40791765 samuel40791765 merged commit 78ada21 into aws:main Sep 9, 2025
128 of 139 checks passed
skmcgrail pushed a commit to skmcgrail/aws-lc that referenced this pull request Sep 10, 2025
Curl is setting a buffer size larger than the maximum buffer length that
AWS-LC allows. `0x401e * 4` is inherently `65656`, which is larger than
the allowed `65535` and AWS-LC directly uses the largest size available
(`65535`) if the consuming application sets a value larger than that.
This limitation's due to a buffer size we inherited from upstream.

I've tested against the test that was failing for curl and the test
seems to pass now (for all instances I've ran it). Running `pytest -n
auto -v ./http/test_10_proxy.py::TestProxy::test_10_08_upload_seq_large
-rA` had been failing 5/10 times prior to this change and it passes
every time that I run it now.

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

(cherry picked from commit 78ada21)
@justsmth justsmth mentioned this pull request Sep 12, 2025
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.

4 participants