Skip to content

transport: reject malformed method paths before tap#9150

Open
sainiranjannallam wants to merge 1 commit into
grpc:masterfrom
sainiranjannallam:harden-malformed-method-tap
Open

transport: reject malformed method paths before tap#9150
sainiranjannallam wants to merge 1 commit into
grpc:masterfrom
sainiranjannallam:harden-malformed-method-tap

Conversation

@sainiranjannallam

Copy link
Copy Markdown

This rejects malformed HTTP/2 method paths in the server transport before invoking InTapHandle. The server already returns Unimplemented for malformed method names before application handlers; validating at transport time keeps tap hooks from observing non-canonical method strings.

The malformed-method regression test now installs an InTapHandle and asserts invalid method paths are rejected without invoking it.

Tests:

  • go test ./test -run Test/MalformedMethodPath -count=1
  • go test ./test -run 'Test/(MalformedMethodPath|Tap)$' -count=1
  • go test ./internal/transport -run 'Test/(HeadersHTTPStatusGRPCStatus|HeadersCausingStreamError)' -count=1

RELEASE NOTES:

  • transport: Reject malformed method names before invoking tap handlers.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Jun 1, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: sainiranjannallam / name: Sai Niranjan Nallam (6456c4d)

@sainiranjannallam sainiranjannallam marked this pull request as ready for review June 1, 2026 05:44
@sainiranjannallam

Copy link
Copy Markdown
Author

Ready for review. EasyCLA is passing now; the only completed failing check is PR validation waiting on a Type: label.

@sainiranjannallam sainiranjannallam force-pushed the harden-malformed-method-tap branch from 6456c4d to f9f665d Compare June 1, 2026 05:47
@codecov

codecov Bot commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.06%. Comparing base (b58f32d) to head (f9f665d).
⚠️ Report is 19 commits behind head on master.

Files with missing lines Patch % Lines
internal/transport/http2_server.go 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9150      +/-   ##
==========================================
- Coverage   83.18%   83.06%   -0.12%     
==========================================
  Files         418      418              
  Lines       33709    33716       +7     
==========================================
- Hits        28041    28007      -34     
- Misses       4248     4276      +28     
- Partials     1420     1433      +13     
Files with missing lines Coverage Δ
internal/transport/http2_server.go 90.35% <71.42%> (-0.16%) ⬇️

... and 30 files with indirect coverage changes

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

@easwars easwars self-requested a review June 3, 2026 03:29
@easwars easwars self-assigned this Jun 3, 2026
@easwars easwars added Type: Bug Area: Transport Includes HTTP/2 client/server and HTTP server handler transports and advanced transport features. labels Jun 3, 2026
@easwars easwars added this to the 1.82 Release milestone Jun 3, 2026
@mbissa mbissa modified the milestones: 1.82 Release, 1.83 Release Jun 5, 2026
@easwars

easwars commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

What is the reasoning behind this PR? Why should the tap handlers not see these malformed paths?

@easwars

easwars commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

FYI, I'm discussing with other gRPC maintainers on what the ideal approach would be to handle this.

@github-actions

Copy link
Copy Markdown

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions Bot added stale and removed stale labels Jun 11, 2026
@github-actions

Copy link
Copy Markdown

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions Bot added stale and removed stale labels Jun 18, 2026
@github-actions

Copy link
Copy Markdown

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions Bot added stale and removed stale labels Jun 24, 2026
@github-actions

Copy link
Copy Markdown

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions Bot added stale and removed stale labels Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Transport Includes HTTP/2 client/server and HTTP server handler transports and advanced transport features. Status: Requires Reporter Clarification Type: Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants