Skip to content
New issue

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

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

Already on GitHub? # to your account

HTTP/1: Support streaming requests of unknown length #506

Merged
merged 9 commits into from
Mar 14, 2025
Merged

Conversation

graebm
Copy link
Contributor

@graebm graebm commented Mar 11, 2025

Issue:

aws-sdk-cpp needs to send streaming requests of unknown content length. This isn't currently supported in our HTTP/1 client. (our HTTP/2 client does already support this)

Background:

The HTTP/1 client already supported these use cases:

  • Send streaming request body, but declare Content-Length up front
  • Send chunked request body, but user must declare length of each chunk via write_chunk() API.

But it wasn't possible to send a stream of unknown length without doing extra copies (read to intermediate buffer, then send that buffer via write_chunk() API).

Description of changes:

Support input streams of unknown length via chunked encoding.

When reading from the stream, leave a bit of space at the start and end of the aws_io_message buffer for the chunk prefix and suffix. Then go back and write in the prefix and suffix afterwards. No unnecessary copies are involved.

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

@codecov-commenter
Copy link

codecov-commenter commented Mar 11, 2025

Codecov Report

Attention: Patch coverage is 88.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 79.45%. Comparing base (60c43f8) to head (599c9b3).

Files with missing lines Patch % Lines
source/h1_encoder.c 88.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
+ Coverage   79.41%   79.45%   +0.03%     
==========================================
  Files          27       27              
  Lines       11621    11666      +45     
==========================================
+ Hits         9229     9269      +40     
- Misses       2392     2397       +5     

☔ 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.

@graebm graebm changed the title HTTP/1: Support input streams of unknown length via chunked encoding HTTP/1: Support streaming requests of unknown length Mar 11, 2025
Copy link
Contributor

@TingDaoK TingDaoK left a comment

Choose a reason for hiding this comment

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

minor comments

@graebm graebm merged commit e3a9cab into main Mar 14, 2025
40 checks passed
@graebm graebm deleted the chunked-stream branch March 14, 2025 20:12
graebm added a commit to awslabs/aws-crt-cpp that referenced this pull request Mar 17, 2025
**Issue:**

aws-sdk-cpp needs to send streaming requests of unknown content length. This isn't currently supported in our HTTP/1 client.

**Description of changes:**
- Update submodules:

        aws-c-auth         v0.8.5 -> v0.8.6
        aws-c-cal          v0.8.3 -> v0.8.7
        aws-c-common       v0.11.1 -> v0.12.0
        aws-c-event-stream v0.5.2 -> v0.5.4
        aws-c-http         v0.9.3 -> v0.9.5
        aws-c-io           v0.16.0 -> v0.17.0
        aws-c-s3           v0.7.11 -> v0.7.13
        aws-lc             v1.46.1 -> v1.48.4
        s2n                v1.5.13 -> v1.5.14

  - Bring in change that allows HTTP/1 streams of unknown length:
    - awslabs/aws-c-http#506
- Remove hack in Base64 decoded logic, which is no longer necessary due to this change:
  - awslabs/aws-c-common#1188
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants