Skip to content

🐛 Fix authority header handling for HTTP/2 - #2522

Merged
AlexV525 merged 8 commits into
mainfrom
fix/h2-authority
May 24, 2026
Merged

AlexV525 merged 8 commits into
mainfrom
fix/h2-authority

Conversation

@AlexV525

Copy link
Copy Markdown
Member

Fixes #2521

@AlexV525

Copy link
Copy Markdown
Member Author

@copilot Add authority tests in the dio_test package

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor

@copilot Add authority tests in the dio_test package

Implemented in commit 34dd6c1.

I added shared authority regression coverage in dio_test (http2AuthorityTests) and wired it into plugins/http2_adapter/test/test_suite_test.dart so the HTTP/2 adapter now validates :authority includes the non-default port via a local HTTP/2 test server. No UI changes in this PR, so there is no screenshot to provide.

@AlexV525

Copy link
Copy Markdown
Member Author

@copilot The test should not only targeting H2, but as a general test about header values

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor

@copilot The test should not only targeting H2, but as a general test about header values

Updated in bae1841.

I refactored this into a general shared header-value test under headers_tests (instead of a dedicated HTTP/2-only test hook). The new shared test checks non-default port preservation in host/authority header values across adapters, with VM conditional helpers for HTTP/1 and HTTP/2 paths. No UI changes here, so there’s no screenshot to provide.

@AlexV525

Copy link
Copy Markdown
Member Author

@copilot Can't the test check the value directly without determining the adapter? If not, convert the test from general to the H2 adapter package only (and move under its test folder)

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor

@copilot Can't the test check the value directly without determining the adapter? If not, convert the test from general to the H2 adapter package only (and move under its test folder)

Updated in f339797.

I moved this regression test to plugins/http2_adapter/test/authority_header_test.dart and removed the adapter-branching general header test from dio_test, so it now directly validates :authority for the H2 adapter only. No UI changes in this PR.

@AlexV525

Copy link
Copy Markdown
Member Author

@copilot

  1. Remove unnecessary dep adding in dio_test
  2. Use headers_test.dart as the filename and gracefully group tests for future addition.

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor

@copilot

  1. Remove unnecessary dep adding in dio_test
  2. Use headers_test.dart as the filename and gracefully group tests for future addition.

Addressed in 9cce64b.

I removed the unnecessary http2 dependency from dio_test/pubspec.yaml, renamed the H2 test file to plugins/http2_adapter/test/headers_test.dart, and wrapped it in a group('headers', ...) for future additions. No UI changes in this PR.

@AlexV525

Copy link
Copy Markdown
Member Author

@copilot Format the test file

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor

@copilot Format the test file

Done in f372ff7 — I formatted plugins/http2_adapter/test/headers_test.dart.

@AlexV525

Copy link
Copy Markdown
Member Author

@copilot Add changelog entry

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor

@copilot Add changelog entry

Done in b591053 — added the changelog entry under plugins/http2_adapter/CHANGELOG.md in ## Unreleased.

@AlexV525
AlexV525 marked this pull request as ready for review May 23, 2026 10:26
@AlexV525
AlexV525 requested a review from a team as a code owner May 23, 2026 10:26
@github-actions

Copy link
Copy Markdown
Contributor

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
plugins/http2_adapter/lib/src/connection_manager_imp.dart 🟢 82.14% 🟢 84.29% 🟢 2.15%
plugins/http2_adapter/lib/src/http2_adapter.dart 🟠 74.31% 🟢 75.69% 🟢 1.38%
Overall Coverage 🟢 88.61% 🟢 88.88% 🟢 0.27%

Minimum allowed coverage is 0%, this run produced 88.88%

@AlexV525
AlexV525 merged commit 589c099 into main May 24, 2026
5 checks passed
@AlexV525
AlexV525 deleted the fix/h2-authority branch May 24, 2026 03:16
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.

[dio_http2_adapter] Http2Adapter omits non-default port from :authority

3 participants