Remove HTTP2 details from generic types shared by HTTP/1.1#7482
Merged
achamayou merged 1 commit intomicrosoft:mainfrom Nov 25, 2025
Merged
Remove HTTP2 details from generic types shared by HTTP/1.1#7482achamayou merged 1 commit intomicrosoft:mainfrom
achamayou merged 1 commit intomicrosoft:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors HTTP/2-specific functionality out of generic HTTP types to simplify the codebase and better separate concerns between HTTP/1.1 and HTTP/2 implementations.
- Removes HTTP/2-specific streaming methods from the generic
HTTPResponderinterface - Moves per-stream responder storage from the global
RPCSessionsclass intoHTTP2ServerSession, where it belongs - Eliminates unnecessary cross-session synchronization by localizing responder lifetime to individual HTTP/2 sessions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/http/responder_lookup.h |
Deleted - global responder lookup no longer needed as responders are now session-local |
src/http/http_session.h |
Removed HTTP/2 stream methods that threw std::logic_error from HTTP/1.1 responder |
src/http/http2_types.h |
Removed dependency on http_responder.h, defined StreamCloseCB locally |
src/http/http2_session.h |
Moved responder storage into HTTP2ServerSession, removed override keywords from HTTP/2-specific methods, removed global lookup reference |
src/enclave/rpc_sessions.h |
Removed ResponderLookup inheritance and updated constructor calls to remove responder lookup parameter |
include/ccf/http_responder.h |
Removed HTTP/2-specific streaming virtual methods and callback typedef from generic interface |
achamayou
approved these changes
Nov 25, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trying to simplify these interfaces to make it easier to add blocking commit, noticed 2 redundancies:
HTTPRespondertype has stream-specific virtual methods, that are unused. I think these should remain in the HTTP2 inheritance stack, visible only to HTTP2-aware bits of code.RPCSessionsobject, even though they're keyed per-sessionID. That seems daft, they can be pushed down the stack intoHTTP2ServerSession, simplifying their lifetime and lookup.