http2: avoid API changes when built with go1.27 The wrapping implementation accidentally dropped some symbols. Preserve them. Move errors into common locations: - ErrNoCachedConn - ErrPushLimitReached - ErrRecursivePush - GoAwayError Move one constant into a common location: - TrailerPrefix Preserve methods of FrameWriteRequest as no-ops. These are not a user-serviceable part and there is no way for users to construct FrameWriteRequests. - FrameWriteRequest.Consume - FrameWriteRequest.DataSize - FrameWriteRequest.StreamID - FrameWriteRequest.String Preserve functions which create write schedulers, returning a no-op implementation of WriteScheduler: - NewPriorityWriteScheduler - NewRandomWriteScheduler - PriorityWriteSchedulerConfig For golang/go#78508 Change-Id: I327603137cf69d93bb405d9a95e038886a6a6964 Reviewed-on: https://go-review.googlesource.com/c/net/+/776180 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
diff --git a/http2/server.go b/http2/server.go index fbb1451..a7d2053 100644 --- a/http2/server.go +++ b/http2/server.go
@@ -2657,21 +2657,6 @@ return len(p), nil } -// TrailerPrefix is a magic prefix for ResponseWriter.Header map keys -// that, if present, signals that the map entry is actually for -// the response trailers, and not the response headers. The prefix -// is stripped after the ServeHTTP call finishes and the values are -// sent in the trailers. -// -// This mechanism is intended only for trailers that are not known -// prior to the headers being written. If the set of trailers is fixed -// or known before the header is written, the normal Go trailers mechanism -// is preferred: -// -// https://golang.org/pkg/net/http/#ResponseWriter -// https://golang.org/pkg/net/http/#example_ResponseWriter_trailers -const TrailerPrefix = "Trailer:" - // promoteUndeclaredTrailers permits http.Handlers to set trailers // after the header has already been flushed. Because the Go // ResponseWriter interface has no way to set Trailers (only the @@ -2948,12 +2933,6 @@ responseWriterStatePool.Put(rws) } -// Push errors. -var ( - ErrRecursivePush = errors.New("http2: recursive push not allowed") - ErrPushLimitReached = errors.New("http2: push would exceed peer's SETTINGS_MAX_CONCURRENT_STREAMS") -) - var _ http.Pusher = (*responseWriter)(nil) func (w *responseWriter) Push(target string, opts *http.PushOptions) error {
diff --git a/http2/server_common.go b/http2/server_common.go index e2faeb9..449538c 100644 --- a/http2/server_common.go +++ b/http2/server_common.go
@@ -6,11 +6,33 @@ import ( "context" + "errors" "net" "net/http" "time" ) +// TrailerPrefix is a magic prefix for ResponseWriter.Header map keys +// that, if present, signals that the map entry is actually for +// the response trailers, and not the response headers. The prefix +// is stripped after the ServeHTTP call finishes and the values are +// sent in the trailers. +// +// This mechanism is intended only for trailers that are not known +// prior to the headers being written. If the set of trailers is fixed +// or known before the header is written, the normal Go trailers mechanism +// is preferred: +// +// https://golang.org/pkg/net/http/#ResponseWriter +// https://golang.org/pkg/net/http/#example_ResponseWriter_trailers +const TrailerPrefix = "Trailer:" + +// Push errors. +var ( + ErrRecursivePush = errors.New("http2: recursive push not allowed") + ErrPushLimitReached = errors.New("http2: push would exceed peer's SETTINGS_MAX_CONCURRENT_STREAMS") +) + // ConfigureServer adds HTTP/2 support to a net/http Server. // // The configuration conf may be nil.
diff --git a/http2/server_wrap.go b/http2/server_wrap.go index 9e6003b..a7a0955 100644 --- a/http2/server_wrap.go +++ b/http2/server_wrap.go
@@ -159,3 +159,43 @@ // to avoid duplicating an exported symbol across two files, // but the changes required to make this work are fairly large. } + +func (wr FrameWriteRequest) StreamID() uint32 { + return 0 +} + +func (wr FrameWriteRequest) DataSize() int { + return 0 +} + +func (wr FrameWriteRequest) Consume(n int32) (FrameWriteRequest, FrameWriteRequest, int) { + return FrameWriteRequest{}, FrameWriteRequest{}, 0 +} + +func (wr FrameWriteRequest) String() string { + return "" +} + +// NewPriorityWriteScheduler is deprecated. +// +// Deprecated: User-provided write schedulers are deprecated. +func NewPriorityWriteScheduler(cfg *PriorityWriteSchedulerConfig) WriteScheduler { + return unsupportedWriteScheduler{} +} + +// NewRandomWriteScheduler is deprecated. +// +// Deprecated: User-provided write schedulers are deprecated. +func NewRandomWriteScheduler() WriteScheduler { + return unsupportedWriteScheduler{} +} + +type unsupportedWriteScheduler struct{} + +func (unsupportedWriteScheduler) OpenStream(streamID uint32, options OpenStreamOptions) {} +func (unsupportedWriteScheduler) CloseStream(streamID uint32) {} +func (unsupportedWriteScheduler) AdjustStream(streamID uint32, priority PriorityParam) {} +func (unsupportedWriteScheduler) Push(wr FrameWriteRequest) {} +func (unsupportedWriteScheduler) Pop() (wr FrameWriteRequest, ok bool) { + return FrameWriteRequest{}, false +}
diff --git a/http2/transport.go b/http2/transport.go index 882a926..08ac409 100644 --- a/http2/transport.go +++ b/http2/transport.go
@@ -399,27 +399,6 @@ return n, err } -// noCachedConnError is the concrete type of ErrNoCachedConn, which -// needs to be detected by net/http regardless of whether it's its -// bundled version (in h2_bundle.go with a rewritten type name) or -// from a user's x/net/http2. As such, as it has a unique method name -// (IsHTTP2NoCachedConnError) that net/http sniffs for via func -// isNoCachedConnError. -type noCachedConnError struct{} - -func (noCachedConnError) IsHTTP2NoCachedConnError() {} -func (noCachedConnError) Error() string { return "http2: no cached connection was available" } - -// isNoCachedConnError reports whether err is of type noCachedConnError -// or its equivalent renamed type in net/http2's h2_bundle.go. Both types -// may coexist in the same running program. -func isNoCachedConnError(err error) bool { - _, ok := err.(interface{ IsHTTP2NoCachedConnError() }) - return ok -} - -var ErrNoCachedConn error = noCachedConnError{} - func (t *Transport) roundTripOpt(req *http.Request, opt RoundTripOpt) (*http.Response, error) { switch req.URL.Scheme { case "https": @@ -1786,19 +1765,6 @@ } } -// GoAwayError is returned by the Transport when the server closes the -// TCP connection after sending a GOAWAY frame. -type GoAwayError struct { - LastStreamID uint32 - ErrCode ErrCode - DebugData string -} - -func (e GoAwayError) Error() string { - return fmt.Sprintf("http2: server sent GOAWAY and closed the connection; LastStreamID=%v, ErrCode=%v, debug=%q", - e.LastStreamID, e.ErrCode, e.DebugData) -} - func isEOFOrNetReadError(err error) bool { if err == io.EOF { return true
diff --git a/http2/transport_common.go b/http2/transport_common.go index f7f85b3..b9f5293 100644 --- a/http2/transport_common.go +++ b/http2/transport_common.go
@@ -411,3 +411,37 @@ tlsCn := cn.(*tls.Conn) // DialContext comment promises this will always succeed return tlsCn, nil } + +// GoAwayError is returned by the Transport when the server closes the +// TCP connection after sending a GOAWAY frame. +type GoAwayError struct { + LastStreamID uint32 + ErrCode ErrCode + DebugData string +} + +func (e GoAwayError) Error() string { + return fmt.Sprintf("http2: server sent GOAWAY and closed the connection; LastStreamID=%v, ErrCode=%v, debug=%q", + e.LastStreamID, e.ErrCode, e.DebugData) +} + +// noCachedConnError is the concrete type of ErrNoCachedConn, which +// needs to be detected by net/http regardless of whether it's its +// bundled version (in h2_bundle.go with a rewritten type name) or +// from a user's x/net/http2. As such, as it has a unique method name +// (IsHTTP2NoCachedConnError) that net/http sniffs for via func +// isNoCachedConnError. +type noCachedConnError struct{} + +func (noCachedConnError) IsHTTP2NoCachedConnError() {} +func (noCachedConnError) Error() string { return "http2: no cached connection was available" } + +// isNoCachedConnError reports whether err is of type noCachedConnError +// or its equivalent renamed type in net/http2's h2_bundle.go. Both types +// may coexist in the same running program. +func isNoCachedConnError(err error) bool { + _, ok := err.(interface{ IsHTTP2NoCachedConnError() }) + return ok +} + +var ErrNoCachedConn error = noCachedConnError{}
diff --git a/http2/writesched_common.go b/http2/writesched_common.go index 957bc65..75354c1 100644 --- a/http2/writesched_common.go +++ b/http2/writesched_common.go
@@ -47,3 +47,44 @@ // priority is used to set the priority of the newly opened stream. priority PriorityParam } + +// PriorityWriteSchedulerConfig configures a priorityWriteScheduler. +// +// Deprecated: User-provided write schedulers are deprecated. +type PriorityWriteSchedulerConfig struct { + // MaxClosedNodesInTree controls the maximum number of closed streams to + // retain in the priority tree. Setting this to zero saves a small amount + // of memory at the cost of performance. + // + // See RFC 7540, Section 5.3.4: + // "It is possible for a stream to become closed while prioritization + // information ... is in transit. ... This potentially creates suboptimal + // prioritization, since the stream could be given a priority that is + // different from what is intended. To avoid these problems, an endpoint + // SHOULD retain stream prioritization state for a period after streams + // become closed. The longer state is retained, the lower the chance that + // streams are assigned incorrect or default priority values." + MaxClosedNodesInTree int + + // MaxIdleNodesInTree controls the maximum number of idle streams to + // retain in the priority tree. Setting this to zero saves a small amount + // of memory at the cost of performance. + // + // See RFC 7540, Section 5.3.4: + // Similarly, streams that are in the "idle" state can be assigned + // priority or become a parent of other streams. This allows for the + // creation of a grouping node in the dependency tree, which enables + // more flexible expressions of priority. Idle streams begin with a + // default priority (Section 5.3.5). + MaxIdleNodesInTree int + + // ThrottleOutOfOrderWrites enables write throttling to help ensure that + // data is delivered in priority order. This works around a race where + // stream B depends on stream A and both streams are about to call Write + // to queue DATA frames. If B wins the race, a naive scheduler would eagerly + // write as much data from B as possible, but this is suboptimal because A + // is a higher-priority stream. With throttling enabled, we write a small + // amount of data from B to minimize the amount of bandwidth that B can + // steal from A. + ThrottleOutOfOrderWrites bool +}
diff --git a/http2/writesched_priority_rfc7540.go b/http2/writesched_priority_rfc7540.go index ccd1afe..10e67f7 100644 --- a/http2/writesched_priority_rfc7540.go +++ b/http2/writesched_priority_rfc7540.go
@@ -15,47 +15,6 @@ // RFC 7540, Section 5.3.5: the default weight is 16. const priorityDefaultWeightRFC7540 = 15 // 16 = 15 + 1 -// PriorityWriteSchedulerConfig configures a priorityWriteScheduler. -// -// Deprecated: User-provided write schedulers are deprecated. -type PriorityWriteSchedulerConfig struct { - // MaxClosedNodesInTree controls the maximum number of closed streams to - // retain in the priority tree. Setting this to zero saves a small amount - // of memory at the cost of performance. - // - // See RFC 7540, Section 5.3.4: - // "It is possible for a stream to become closed while prioritization - // information ... is in transit. ... This potentially creates suboptimal - // prioritization, since the stream could be given a priority that is - // different from what is intended. To avoid these problems, an endpoint - // SHOULD retain stream prioritization state for a period after streams - // become closed. The longer state is retained, the lower the chance that - // streams are assigned incorrect or default priority values." - MaxClosedNodesInTree int - - // MaxIdleNodesInTree controls the maximum number of idle streams to - // retain in the priority tree. Setting this to zero saves a small amount - // of memory at the cost of performance. - // - // See RFC 7540, Section 5.3.4: - // Similarly, streams that are in the "idle" state can be assigned - // priority or become a parent of other streams. This allows for the - // creation of a grouping node in the dependency tree, which enables - // more flexible expressions of priority. Idle streams begin with a - // default priority (Section 5.3.5). - MaxIdleNodesInTree int - - // ThrottleOutOfOrderWrites enables write throttling to help ensure that - // data is delivered in priority order. This works around a race where - // stream B depends on stream A and both streams are about to call Write - // to queue DATA frames. If B wins the race, a naive scheduler would eagerly - // write as much data from B as possible, but this is suboptimal because A - // is a higher-priority stream. With throttling enabled, we write a small - // amount of data from B to minimize the amount of bandwidth that B can - // steal from A. - ThrottleOutOfOrderWrites bool -} - // NewPriorityWriteScheduler constructs a WriteScheduler that schedules // frames by following HTTP/2 priorities as described in RFC 7540 Section 5.3. // If cfg is nil, default options are used.