Workaround the quic-go v0.53 API changes

[rthellend]

quic-go v0.53 changed its API significantly: https://github.com/quic-go/quic-go/releases/tag/v0.53.0

At first glance, it will break a lot of TLSPROXY functionality. Among other things, the instrumentation for ingress/egress metrics (and everything that depends on them, e.g. bw limits) will probably need to be removed. I don't think it will be possible to re-implement with the new API.

FYI @marten-seemann

[marten-seemann]

Among other things, the instrumentation for ingress/egress metrics (and everything that depends on them, e.g. bw limits) will probably need to be removed. I don't think it will be possible to re-implement with the new API.

Why?

[rthellend]

Among other things, the instrumentation for ingress/egress metrics (and everything that depends on them, e.g. bw limits) will probably need to be removed. I don't think it will be possible to re-implement with the new API.

Why?

I'm still assessing the impact of all the changes, but I know for sure that something like this won't work anymore, and very likely won't be possible to implement again.

func (s *QUICStream) Read(b []byte) (int, error) {
	if l := s.qc.ingressLimiter; l != nil {
		if err := l.WaitN(s.Context(), len(b)); err != nil {
			return 0, err
		}
	}
	n, err := s.Stream.Read(b)
	if err != nil {
		s.markReadDone()
	}
	s.qc.bytesReceived.Incr(int64(n))
	s.qc.upBytesReceived.Incr(int64(n))
	return n, err
}

(where QUICStream is a quic.Stream)

https://github.com/c2FmZQ/tlsproxy/blob/main/proxy/internal/netw/quic.go#L564

[marten-seemann]

You can define your own interface with the methods of quic.Stream that your application needs. I believe this is the idiomatic pattern anyway: the consumer, not the provider, should define the interface.

[rthellend]

I understand the idiomatic pattern. However, the consumer here is code that now expects a quic.Conn, for example https://pkg.go.dev/github.com/quic-go/quic-go@v0.53.0/http3#Server.ServeQUICConn

It is now impossible to send an instrumented quic.Conn to http3.Server.ServeQUICConn, because it only accepts a specific struct. Now, if that accepted an interface, I could still use an instrumented quic.Conn.

My implementation of quic.Connection has a lot of instrumentation for streams that are created on it.

Making quic.Conn a struct is not the problem. The problem is everything that used to accept a quic.Connection is now requiring a specific struct now.

[marten-seemann]

Good point. We should return structs, but accept interfaces. So http3.Server.ServeQUICConn should accept an interface.

But what should that interface be? The HTTP/3 package could define a QUICConn interface:

type QUICConn interface {
	OpenStream() (*quic.Stream, error)
	OpenStreamSync(context.Context) (*quic.Stream, error)
	OpenUniStream() (*quic.SendStream, error)
	OpenUniStreamSync(context.Context) (*quic.SendStream, error)
	AcceptStream(context.Context) (*quic.Stream, error)
	AcceptUniStream(context.Context) (*quic.ReceiveStream, error)
	Context() context.Context
	LocalAddr() net.Addr
	RemoteAddr() net.Addr
	CloseWithError(quic.ApplicationErrorCode, string) error
	ConnectionState() quic.ConnectionState
	HandshakeComplete() <-chan struct{}
	SendDatagram([]byte) error
	ReceiveDatagram(context.Context) ([]byte, error)
}

However, now you still can't pass in your wrapped streams. But if we define the interface with interfaces for the streams as well, the quic.Conn doesn't satisfy this interface anymore:

type QUICConn interface {
	OpenStream() (QUICStream, error)
	OpenStreamSync(context.Context) (QUICStream, error)
	OpenUniStream() (QUICSendStream, error)
	OpenUniStreamSync(context.Context) (QUICSendStream, error)
	AcceptStream(context.Context) (QUICStream, error)
	AcceptUniStream(context.Context) (QUICReceiveStream, error)
        // ...
}

What's the idiomatic solution?

[rthellend]

I think this is a case where the only solution that works is the right solution: make OpenStream() and friends return interfaces. I can't think of anything else that would work.

[marten-seemann]

I don't think that's the right way to go.

[rthellend]

To summarize:

The API change in quic-go and quic-go/http3 changed the quic.Connection and quic.Stream interfaces to concrete types. The motivation was to be idiomatic: return concrete types, accept interfaces.

The problem is that the http3 API now requires concrete types instead of interfaces, which makes it impossible to mock the input or use a different implementation.

The solution would be to change the http3 API to accept an interface wherever *quic.Conn is now required.

The question is what should the interface be.

If the interface itself requires concrete types, e.g.

type QUICConn interface {
	OpenStream() (*quic.Stream, error)
	OpenStreamSync(context.Context) (*quic.Stream, error)
	OpenUniStream() (*quic.SendStream, error)
	OpenUniStreamSync(context.Context) (*quic.SendStream, error)
// ...
}

we have the same problem. This QUICConn cannot be mocked or replaced with a different implementation because *quic.Stream is a concrete type that cannot be replaced.

My suggestion would be to use an interface for Stream, but that would require changing quic.Conn's OpenStream() and other functions to return an interface instead of a *quic.Stream concrete type. This breaks the return concrete types pattern, BUT it satisfies the accept interfaces pattern and restores functionality in http3.

This is not an isolated case. The same problem happens whenever concrete type functions return concrete types. The std library has many such examples where interface functions return io.Reader, io.Writer, ... There is no problem with a function that returns io.Reader instead of a concrete type if it helps with interoperability.

This model is completely valid and makes the go libraries so extendable. The quic-go library can use the same pattern for streams and other types to facilitate interoperability and extension.

[marten-seemann]

This is not an isolated case. The same problem happens whenever concrete type functions return concrete types. The std library has many such examples where interface functions return io.Reader, io.Writer, ... There is no problem with a function that returns io.Reader instead of a concrete type if it helps with interoperability.

Where does the standard library do that?

This model is completely valid and makes the go libraries so extendable. The quic-go library can use the same pattern for streams and other types to facilitate interoperability and extension.

With the same argument you could say that Conn should be an interface, since it should be possible to mock a quic.Listener. Where do we draw the line?

[rthellend]

Where does the standard library do that?

The standard library provides interfaces for interoperability everywhere. The io and net packages are prime examples. These interfaces are used in other structs and interfaces. Without io.Reader and net.Conn, it would be pretty hard to build anything in go. A quic stream is not much different.

With the same argument you could say that Conn should be an interface, since it should be possible to mock a quic.Listener. Where do we draw the line?

This is about interoperability. My personal opinion is that there was nothing wrong with the use of interfaces in v0.52. I have read your blog post about your motivation for getting rid of interfaces. I generally agree with the "return concrete types, accept interfaces" pattern, BUT the accept interfaces is the most important part. That's what makes interoperability possible.

To answer the question more directly, I do think that quic.Listener should return an interface so that it and Transport can be mocked. I don't think complex APIs can be designed properly in go without some thoughtful consideration of interfaces.

[marten-seemann]

Where does the standard library do that?

The standard library provides interfaces for interoperability everywhere. The io and net packages are prime examples. These interfaces are used in other structs and interfaces. Without io.Reader and net.Conn, it would be pretty hard to build anything in go. A quic stream is not much different.

Except it is... the io.Reader has 1 method, a QUIC stream has 10 (!), and with quic-go/quic-go#4139 even more. This makes an io.Reader widely applicable across different use cases. It is hard to imagine that the massive quic.Stream interface would ever be used for something that's not a QUIC stream (a wrapped one, at best).

My question was: where does the standard library return interfaces instead of structs. bytes.NewReader returns a bytes.Reader, for example, and net.DialUDP returns a net.UDPConn.

BUT the accept interfaces is the most important part.

I agree that the current HTTP/3 API is not ideal, and I'd be happy to change it. The question is how.

[rthellend]

io/fs comes to mind.

https://pkg.go.dev/io/fs

[marten-seemann]

That's a bad example. There are many different file systems: on disk, compressed archives, embedded, etc. This is not even remotely comparable to a QUIC stream. Please stop pushing on this, we will not change the API back to its messy prior state. Let's focus on finding a reasonable HTTP/3 API.