Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

proposal: x/net/http2: enable extension frame handling for HTTP/2 #40359

Open
soya3129 opened this issue Jul 23, 2020 · 10 comments
Open

proposal: x/net/http2: enable extension frame handling for HTTP/2 #40359

soya3129 opened this issue Jul 23, 2020 · 10 comments

Comments

@soya3129
Copy link

soya3129 commented Jul 23, 2020

HTTP/2 library currently ignores HTTP/2 frames with an undefined type. The proposal is to enable the library to handle the extension frames. The change will enable HTTP/2 Go's feature parity to nghttp2 (a popular HTTP/2 library in c).

Requirement:

  1. The change needs to include a feature flag to turn on the extension frame handling when needed.
  2. The change needs to support extension frames associated with a stream. Connection level extension frames are not required in this change, but may be needed in the future.
  3. Extension frames can't be used to open or close a stream, and should not be counted against flow control.
@soya3129
Copy link
Author

I would like to take the issue. Thanks!

@soya3129 soya3129 changed the title Enable extension frame handling for HTTP/2 Proposal: x/net/http2: Enable extension frame handling for HTTP/2 Jul 23, 2020
@gopherbot gopherbot added this to the Proposal milestone Jul 23, 2020
@soya3129 soya3129 changed the title Proposal: x/net/http2: Enable extension frame handling for HTTP/2 Proposal: net/http2: Enable extension frame handling for HTTP/2 Jul 23, 2020
@soya3129 soya3129 changed the title Proposal: net/http2: Enable extension frame handling for HTTP/2 Proposal: x/net/http2: Enable extension frame handling for HTTP/2 Jul 23, 2020
@networkimprov
Copy link

cc @fraenkel

@gopherbot
Copy link

Change https://golang.org/cl/244478 mentions this issue: http2:Enable HTTP/2 server to receive and send unknown frames.

@gopherbot
Copy link

Change https://golang.org/cl/244800 mentions this issue: http2:Enable HTTP/2 CLIENTs to receive and send unknown frames

@ianlancetaylor
Copy link
Contributor

Can someone write down the additional API that is being proposed? That is, all the new exported names that would be added to the x/net/http2 package. Thanks.

@ianlancetaylor ianlancetaylor changed the title Proposal: x/net/http2: Enable extension frame handling for HTTP/2 proposal: x/net/http2: enable extension frame handling for HTTP/2 Aug 7, 2020
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Aug 7, 2020
@soya3129
Copy link
Author

soya3129 commented Aug 8, 2020

The new API include a reader interface which can be called by both servers and clients, a write interface called by servers and a write function called by clients. In addition, the PR also proposes an unknown frame info struct.

// Server side usage:
// func ServeHTTP(rw http.RequestWriter, r *http.Request) {
// ...
// ufr := r.Context().Value(http2.UnknownFrameReaderKey)
// reader, ok := ufr.(UnknownFrameReader)
// f, err := reader.ReadUnknownFrame(context.Background())
// ...
// }
//
// Client side usage:
// response, err := transport.RoundTrip(request)
// rwuf, _ := response.Body.(UnknownFrameReader)
// f, err := rwuf.ReadUnknownFrame(context.Background())
type UnknownFrameReader interface {
// If no new unknown frame has been received, the function will return
// error "haven't received unknown frames". If there will be no more unknown frames, the
// function will return error "no more unknown frame".
ReadUnknownFrame(ctx context.Context) (*UnknownFrame, error)
}

// Servers call WriteUnknownFrame writes unknown frames.
// WriteUnknownFrame writes unknown frames.
// When EnableUnknownFrames is true, http.ResponseWriter passed to the handler function can be cast to
// WriteUnknownFrame and write unknown frames. For example:
// func ServeHTTP(rw http.RequestWriter, r *http.Request) {
// ...
// ufw, ok := rw.(WriteUnknownFrame)
// ufw.WriteUnknownFrame(frameType, frameFlags, unknownFrameBody)
// ...
// }
type WriteUnknownFrame interface {
// Call WriteUnknownFrame to write an unknown frame.
WriteUnknownFrame(t FrameType, flags Flags, payload []byte) error
}

// Clients call AddUnknownFrame to add an unknown frame to http.Request on the client side. This function can
// be called multiple times, in case multiple unknown frames need to be added.
// This function must be called before RoundTrip().
func AddUnknownFrame(req *http.Request, uf *UnknownFrameInfo) (*http.Request)

// UnknownFrameInfo is a struct to store unknown frame fields.
type UnknownFrameInfo struct {
Type FrameType
Flags Flags
Body []byte
}

// Flag to enable unknown frame handling
EnableUnknownFrame bool

@soya3129
Copy link
Author

soya3129 commented Aug 8, 2020

cc @ianswett @birenroy

@neild
Copy link
Contributor

neild commented Oct 22, 2020

There are four new operations required to support sending and receiving extension frames: Send and receive for client and server.

Under this proposal:

  • Server sends a frame: Type assert the http.ResponseWriter to an type which permits sending frames.
  • Server receives a frame: Retrieve a value which permits reading frames from the request context.Context.
  • Client sends a frame: Pass a (http.Request).Body value that includes the unknown frames. This value is an unexported type; an exported function initializes the Body field.
  • Client receives a frame: Type assert the (http.Response).Body to a type which permits accessing frames.

There are some clear asymmetries in here. Clients pass frames via the request/response body, but the send API looks quite different from the receive one. Servers get an UnknownFrameReader from the context, while clients get one from the response body. I think we need a more consistent API across common operations.

Incoming frames are added to an infinite buffer. This seems hazardous. Since there is no flow control on frames (AFAIK, I may be missing something here), I think it would be better to deliver incoming frames immediately to a callback. This does open the question of how to dispatch extensions frames received before the response.

@lasradoVinod
Copy link

I wanted to pick this up. The main idea of this proposal relates to https://datatracker.ietf.org/doc/html/rfc9113#name-extending-http-2 allowing for adding new frame types but go http2 simply errors out https://cs.opensource.google/go/x/net/+/master:http2/transport.go;l=2322;bpv=1;bpt=1

This is how nghttp2 handles it as extensions https://nghttp2.org/documentation/programmers-guide.html#implement-user-defined-http-2-non-critical-extensions and it allows anyone to add a frame type. I will figure out a clean api and submit it for approval shortly. I just wanted to see what the community has to say about this proposal since it is almost a year old.

@neild
Copy link
Contributor

neild commented Nov 18, 2023

At a high level, I think supporting user access extension frames seems reasonable. The question is what the API should look like.

We have a medium-term project to fully merge golang.org/x/net/http2 into net/http, so any additions here should be something that we can support entirely in net/http. Ideally, the new API should not require any references to the http2 package.

We would like to support HTTP/3 in net/http at some point. HTTP/3 also supports extension frames (https://www.rfc-editor.org/rfc/rfc9114.html#name-extensions-to-http-3), so this proposal should be something that can be extended to cover HTTP/3 in the future.

Buffering received frames is problematic without some form of limit on buffer growth. Capping the buffer size leaves the question of what to do when an extension is received when the buffer is full. I think a callback-based approach is going to work better, since it adds a natural pushback mechanism.

A connection-level callback for unrecognized frames would be easy to implement. Associating frames with streams (requests) is harder. We could expose the stream id in requests somehow and leave it up to the user to figure out how to move the frame from the callback to the request handler; that's not the most ergonomic approach, but I don't have any better ideas at the moment.

Sending frames has the interesting question of where to put the send-frame method. If frames are a connection-level concept, then a method on http2.ClientConn would be logical, but we don't really expose ClientConn outside of the mostly-vestigial HTTP/2 connection pool. Also, ClientConn is only defined in the http2 package, which is a problem if we want this API to be in net/http. We could instead put a send-frame method on http.ResponseController, but that associates sending frames with a specific stream rather than the connection as a whole. I don't have any good ideas here at the moment, and I'd be interested to hear ideas on how to handle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

6 participants