Skip to content

Conversation

@TingDaoK
Copy link
Contributor

@TingDaoK TingDaoK commented Jan 28, 2026

Issue #, if available:

  1. When the initial settings set for h2 stream manager, the copy of the initial settings has a bug that didn't set the length of array correctly after the data copied. And end with the initial settings was not respected.
  2. When converting http/1.1 header to h2, forget to skip connection header that is also not allowed
  3. An endpoint MUST NOT generate an HTTP/2 message containing connection-specific header fields. so that let's check the headers we send out, and error out if those headers are found

Description of changes:

  • Add tests. Also, issues warnings when the flow control window is 0 and stops the flow so that we can track this easier
  • Fix the headers related bug
  • Log the headers so that we can debug easier
  • Skip the authorization headers in case of sensitive information.

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 Jan 28, 2026

Codecov Report

❌ Patch coverage is 72.41379% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.63%. Comparing base (acf3139) to head (c737053).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
source/h2_stream.c 60.00% 6 Missing ⚠️
source/h1_stream.c 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #544      +/-   ##
==========================================
- Coverage   79.70%   79.63%   -0.07%     
==========================================
  Files          28       28              
  Lines       11869    11894      +25     
==========================================
+ Hits         9460     9472      +12     
- Misses       2409     2422      +13     

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

@TingDaoK TingDaoK changed the title [fix] h2 stream manager initial settings not passed correctly [fix] h2 stream manager initial settings not passed correctly & Log the headers Jan 29, 2026
aws_http_headers_get_index(headers, i, &header);
enum aws_http_header_name name_enum = aws_http_str_to_header_name(header.name);
switch (name_enum) {
case AWS_HTTP_HEADER_AUTHORIZATION:
Copy link
Contributor

Choose a reason for hiding this comment

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

can we also skip session/security token?

@TingDaoK TingDaoK merged commit a9745ea into main Jan 30, 2026
45 checks passed
@TingDaoK TingDaoK deleted the h2-fixes branch January 30, 2026 21:42
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