Skip to content

Commit 23ee2ef

Browse files
committed
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>
1 parent b138e06 commit 23ee2ef

7 files changed

Lines changed: 137 additions & 96 deletions

File tree

http2/server.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2657,21 +2657,6 @@ func (rws *responseWriterState) writeChunk(p []byte) (n int, err error) {
26572657
return len(p), nil
26582658
}
26592659

2660-
// TrailerPrefix is a magic prefix for ResponseWriter.Header map keys
2661-
// that, if present, signals that the map entry is actually for
2662-
// the response trailers, and not the response headers. The prefix
2663-
// is stripped after the ServeHTTP call finishes and the values are
2664-
// sent in the trailers.
2665-
//
2666-
// This mechanism is intended only for trailers that are not known
2667-
// prior to the headers being written. If the set of trailers is fixed
2668-
// or known before the header is written, the normal Go trailers mechanism
2669-
// is preferred:
2670-
//
2671-
// https://golang.org/pkg/net/http/#ResponseWriter
2672-
// https://golang.org/pkg/net/http/#example_ResponseWriter_trailers
2673-
const TrailerPrefix = "Trailer:"
2674-
26752660
// promoteUndeclaredTrailers permits http.Handlers to set trailers
26762661
// after the header has already been flushed. Because the Go
26772662
// ResponseWriter interface has no way to set Trailers (only the
@@ -2948,12 +2933,6 @@ func (w *responseWriter) handlerDone() {
29482933
responseWriterStatePool.Put(rws)
29492934
}
29502935

2951-
// Push errors.
2952-
var (
2953-
ErrRecursivePush = errors.New("http2: recursive push not allowed")
2954-
ErrPushLimitReached = errors.New("http2: push would exceed peer's SETTINGS_MAX_CONCURRENT_STREAMS")
2955-
)
2956-
29572936
var _ http.Pusher = (*responseWriter)(nil)
29582937

29592938
func (w *responseWriter) Push(target string, opts *http.PushOptions) error {

http2/server_common.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,33 @@ package http2
66

77
import (
88
"context"
9+
"errors"
910
"net"
1011
"net/http"
1112
"time"
1213
)
1314

15+
// TrailerPrefix is a magic prefix for ResponseWriter.Header map keys
16+
// that, if present, signals that the map entry is actually for
17+
// the response trailers, and not the response headers. The prefix
18+
// is stripped after the ServeHTTP call finishes and the values are
19+
// sent in the trailers.
20+
//
21+
// This mechanism is intended only for trailers that are not known
22+
// prior to the headers being written. If the set of trailers is fixed
23+
// or known before the header is written, the normal Go trailers mechanism
24+
// is preferred:
25+
//
26+
// https://golang.org/pkg/net/http/#ResponseWriter
27+
// https://golang.org/pkg/net/http/#example_ResponseWriter_trailers
28+
const TrailerPrefix = "Trailer:"
29+
30+
// Push errors.
31+
var (
32+
ErrRecursivePush = errors.New("http2: recursive push not allowed")
33+
ErrPushLimitReached = errors.New("http2: push would exceed peer's SETTINGS_MAX_CONCURRENT_STREAMS")
34+
)
35+
1436
// ConfigureServer adds HTTP/2 support to a net/http Server.
1537
//
1638
// The configuration conf may be nil.

http2/server_wrap.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,3 +159,43 @@ type FrameWriteRequest struct {
159159
// to avoid duplicating an exported symbol across two files,
160160
// but the changes required to make this work are fairly large.
161161
}
162+
163+
func (wr FrameWriteRequest) StreamID() uint32 {
164+
return 0
165+
}
166+
167+
func (wr FrameWriteRequest) DataSize() int {
168+
return 0
169+
}
170+
171+
func (wr FrameWriteRequest) Consume(n int32) (FrameWriteRequest, FrameWriteRequest, int) {
172+
return FrameWriteRequest{}, FrameWriteRequest{}, 0
173+
}
174+
175+
func (wr FrameWriteRequest) String() string {
176+
return ""
177+
}
178+
179+
// NewPriorityWriteScheduler is deprecated.
180+
//
181+
// Deprecated: User-provided write schedulers are deprecated.
182+
func NewPriorityWriteScheduler(cfg *PriorityWriteSchedulerConfig) WriteScheduler {
183+
return unsupportedWriteScheduler{}
184+
}
185+
186+
// NewRandomWriteScheduler is deprecated.
187+
//
188+
// Deprecated: User-provided write schedulers are deprecated.
189+
func NewRandomWriteScheduler() WriteScheduler {
190+
return unsupportedWriteScheduler{}
191+
}
192+
193+
type unsupportedWriteScheduler struct{}
194+
195+
func (unsupportedWriteScheduler) OpenStream(streamID uint32, options OpenStreamOptions) {}
196+
func (unsupportedWriteScheduler) CloseStream(streamID uint32) {}
197+
func (unsupportedWriteScheduler) AdjustStream(streamID uint32, priority PriorityParam) {}
198+
func (unsupportedWriteScheduler) Push(wr FrameWriteRequest) {}
199+
func (unsupportedWriteScheduler) Pop() (wr FrameWriteRequest, ok bool) {
200+
return FrameWriteRequest{}, false
201+
}

http2/transport.go

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -399,27 +399,6 @@ func (sew stickyErrWriter) Write(p []byte) (n int, err error) {
399399
return n, err
400400
}
401401

402-
// noCachedConnError is the concrete type of ErrNoCachedConn, which
403-
// needs to be detected by net/http regardless of whether it's its
404-
// bundled version (in h2_bundle.go with a rewritten type name) or
405-
// from a user's x/net/http2. As such, as it has a unique method name
406-
// (IsHTTP2NoCachedConnError) that net/http sniffs for via func
407-
// isNoCachedConnError.
408-
type noCachedConnError struct{}
409-
410-
func (noCachedConnError) IsHTTP2NoCachedConnError() {}
411-
func (noCachedConnError) Error() string { return "http2: no cached connection was available" }
412-
413-
// isNoCachedConnError reports whether err is of type noCachedConnError
414-
// or its equivalent renamed type in net/http2's h2_bundle.go. Both types
415-
// may coexist in the same running program.
416-
func isNoCachedConnError(err error) bool {
417-
_, ok := err.(interface{ IsHTTP2NoCachedConnError() })
418-
return ok
419-
}
420-
421-
var ErrNoCachedConn error = noCachedConnError{}
422-
423402
func (t *Transport) roundTripOpt(req *http.Request, opt RoundTripOpt) (*http.Response, error) {
424403
switch req.URL.Scheme {
425404
case "https":
@@ -1786,19 +1765,6 @@ func (cc *ClientConn) readLoop() {
17861765
}
17871766
}
17881767

1789-
// GoAwayError is returned by the Transport when the server closes the
1790-
// TCP connection after sending a GOAWAY frame.
1791-
type GoAwayError struct {
1792-
LastStreamID uint32
1793-
ErrCode ErrCode
1794-
DebugData string
1795-
}
1796-
1797-
func (e GoAwayError) Error() string {
1798-
return fmt.Sprintf("http2: server sent GOAWAY and closed the connection; LastStreamID=%v, ErrCode=%v, debug=%q",
1799-
e.LastStreamID, e.ErrCode, e.DebugData)
1800-
}
1801-
18021768
func isEOFOrNetReadError(err error) bool {
18031769
if err == io.EOF {
18041770
return true

http2/transport_common.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,3 +411,37 @@ func (t *Transport) dialTLSWithContext(ctx context.Context, network, addr string
411411
tlsCn := cn.(*tls.Conn) // DialContext comment promises this will always succeed
412412
return tlsCn, nil
413413
}
414+
415+
// GoAwayError is returned by the Transport when the server closes the
416+
// TCP connection after sending a GOAWAY frame.
417+
type GoAwayError struct {
418+
LastStreamID uint32
419+
ErrCode ErrCode
420+
DebugData string
421+
}
422+
423+
func (e GoAwayError) Error() string {
424+
return fmt.Sprintf("http2: server sent GOAWAY and closed the connection; LastStreamID=%v, ErrCode=%v, debug=%q",
425+
e.LastStreamID, e.ErrCode, e.DebugData)
426+
}
427+
428+
// noCachedConnError is the concrete type of ErrNoCachedConn, which
429+
// needs to be detected by net/http regardless of whether it's its
430+
// bundled version (in h2_bundle.go with a rewritten type name) or
431+
// from a user's x/net/http2. As such, as it has a unique method name
432+
// (IsHTTP2NoCachedConnError) that net/http sniffs for via func
433+
// isNoCachedConnError.
434+
type noCachedConnError struct{}
435+
436+
func (noCachedConnError) IsHTTP2NoCachedConnError() {}
437+
func (noCachedConnError) Error() string { return "http2: no cached connection was available" }
438+
439+
// isNoCachedConnError reports whether err is of type noCachedConnError
440+
// or its equivalent renamed type in net/http2's h2_bundle.go. Both types
441+
// may coexist in the same running program.
442+
func isNoCachedConnError(err error) bool {
443+
_, ok := err.(interface{ IsHTTP2NoCachedConnError() })
444+
return ok
445+
}
446+
447+
var ErrNoCachedConn error = noCachedConnError{}

http2/writesched_common.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,44 @@ type OpenStreamOptions struct {
4747
// priority is used to set the priority of the newly opened stream.
4848
priority PriorityParam
4949
}
50+
51+
// PriorityWriteSchedulerConfig configures a priorityWriteScheduler.
52+
//
53+
// Deprecated: User-provided write schedulers are deprecated.
54+
type PriorityWriteSchedulerConfig struct {
55+
// MaxClosedNodesInTree controls the maximum number of closed streams to
56+
// retain in the priority tree. Setting this to zero saves a small amount
57+
// of memory at the cost of performance.
58+
//
59+
// See RFC 7540, Section 5.3.4:
60+
// "It is possible for a stream to become closed while prioritization
61+
// information ... is in transit. ... This potentially creates suboptimal
62+
// prioritization, since the stream could be given a priority that is
63+
// different from what is intended. To avoid these problems, an endpoint
64+
// SHOULD retain stream prioritization state for a period after streams
65+
// become closed. The longer state is retained, the lower the chance that
66+
// streams are assigned incorrect or default priority values."
67+
MaxClosedNodesInTree int
68+
69+
// MaxIdleNodesInTree controls the maximum number of idle streams to
70+
// retain in the priority tree. Setting this to zero saves a small amount
71+
// of memory at the cost of performance.
72+
//
73+
// See RFC 7540, Section 5.3.4:
74+
// Similarly, streams that are in the "idle" state can be assigned
75+
// priority or become a parent of other streams. This allows for the
76+
// creation of a grouping node in the dependency tree, which enables
77+
// more flexible expressions of priority. Idle streams begin with a
78+
// default priority (Section 5.3.5).
79+
MaxIdleNodesInTree int
80+
81+
// ThrottleOutOfOrderWrites enables write throttling to help ensure that
82+
// data is delivered in priority order. This works around a race where
83+
// stream B depends on stream A and both streams are about to call Write
84+
// to queue DATA frames. If B wins the race, a naive scheduler would eagerly
85+
// write as much data from B as possible, but this is suboptimal because A
86+
// is a higher-priority stream. With throttling enabled, we write a small
87+
// amount of data from B to minimize the amount of bandwidth that B can
88+
// steal from A.
89+
ThrottleOutOfOrderWrites bool
90+
}

http2/writesched_priority_rfc7540.go

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,47 +15,6 @@ import (
1515
// RFC 7540, Section 5.3.5: the default weight is 16.
1616
const priorityDefaultWeightRFC7540 = 15 // 16 = 15 + 1
1717

18-
// PriorityWriteSchedulerConfig configures a priorityWriteScheduler.
19-
//
20-
// Deprecated: User-provided write schedulers are deprecated.
21-
type PriorityWriteSchedulerConfig struct {
22-
// MaxClosedNodesInTree controls the maximum number of closed streams to
23-
// retain in the priority tree. Setting this to zero saves a small amount
24-
// of memory at the cost of performance.
25-
//
26-
// See RFC 7540, Section 5.3.4:
27-
// "It is possible for a stream to become closed while prioritization
28-
// information ... is in transit. ... This potentially creates suboptimal
29-
// prioritization, since the stream could be given a priority that is
30-
// different from what is intended. To avoid these problems, an endpoint
31-
// SHOULD retain stream prioritization state for a period after streams
32-
// become closed. The longer state is retained, the lower the chance that
33-
// streams are assigned incorrect or default priority values."
34-
MaxClosedNodesInTree int
35-
36-
// MaxIdleNodesInTree controls the maximum number of idle streams to
37-
// retain in the priority tree. Setting this to zero saves a small amount
38-
// of memory at the cost of performance.
39-
//
40-
// See RFC 7540, Section 5.3.4:
41-
// Similarly, streams that are in the "idle" state can be assigned
42-
// priority or become a parent of other streams. This allows for the
43-
// creation of a grouping node in the dependency tree, which enables
44-
// more flexible expressions of priority. Idle streams begin with a
45-
// default priority (Section 5.3.5).
46-
MaxIdleNodesInTree int
47-
48-
// ThrottleOutOfOrderWrites enables write throttling to help ensure that
49-
// data is delivered in priority order. This works around a race where
50-
// stream B depends on stream A and both streams are about to call Write
51-
// to queue DATA frames. If B wins the race, a naive scheduler would eagerly
52-
// write as much data from B as possible, but this is suboptimal because A
53-
// is a higher-priority stream. With throttling enabled, we write a small
54-
// amount of data from B to minimize the amount of bandwidth that B can
55-
// steal from A.
56-
ThrottleOutOfOrderWrites bool
57-
}
58-
5918
// NewPriorityWriteScheduler constructs a WriteScheduler that schedules
6019
// frames by following HTTP/2 priorities as described in RFC 7540 Section 5.3.
6120
// If cfg is nil, default options are used.

0 commit comments

Comments
 (0)