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 · 8 comments
Open

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

soya3129 opened this issue Jul 23, 2020 · 8 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.

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

5 participants