Update to quic-go v0.15.5, adapt usage #141
Conversation
Adapt usage of various APIs: - pass context (`context.Background()`, for now) in various Accept, Listen etc. - replace `session.Close()` with `session.CloseWithError` - module `h2quic` is now `http3` - `http3` now uses `EarlySession` instead of Session; add corresponding `DialEarly` API to `appquic` - set now required `NextProtos` where needed At the same time, slightly reorganize the usage of the `tls.Config` parameter for the `appquic.Dial` functions. As `NextProtos` is now required, pass this explicitly for every usage. As a related cleanup, don't automatically set `InsecureSkipVerify` and self-signed server certificates in the `appquic` library code anymore; moved to the applications, to make this hack a bit more apparent.
New format allows to not include brackets.
There was a problem hiding this comment.
Reviewed 4 of 13 files at r1.
Reviewable status: 4 of 13 files reviewed, all discussions resolved
There was a problem hiding this comment.
Reviewed 3 of 13 files at r1.
Reviewable status: 7 of 14 files reviewed, 1 unresolved discussion (waiting on @matzf)
netcat/modes/quic-modes.go, line 50 at r1 (raw file):
quic.ErrorCode(0)
In scionproto rpc.go, you used errorNoError quic.ErrorCode = 0x100
Do we want to use a different No error error code here?
It could make sense to use the same No error error code for at least all scion applications that use appquic. Makes debugging / reading the logs across different apps easier.
There was a problem hiding this comment.
Reviewed 1 of 13 files at r1, 1 of 1 files at r2.
Reviewable status: 9 of 14 files reviewed, 2 unresolved discussions (waiting on @matzf)
pkg/appnet/hosts.go, line 32 at r1 (raw file):
regexp.MustCompile(`^((?:[-.\da-zA-Z]+)|(?:\d+-[\d:A-Fa-f]+,(\[[^\]]+\]|[^\]]+))):(\d+)$`)
The regex seems wrong to me.
TestSplitHostPort should fail for a test case like {"1-ff00:0:0,2a00:1450:400a:801::200", "", "", true},, but it does not. With the old one, it obviously would have.
There was a problem hiding this comment.
Reviewed 2 of 13 files at r1.
Reviewable status: 11 of 14 files reviewed, 2 unresolved discussions (waiting on @matzf)
There was a problem hiding this comment.
Reviewable status: 9 of 14 files reviewed, 2 unresolved discussions (waiting on @FR4NK-W and @matzf)
pkg/appnet/hosts.go, line 32 at r1 (raw file):
Previously, FR4NK-W wrote…
regexp.MustCompile(`^((?:[-.\da-zA-Z]+)|(?:\d+-[\d:A-Fa-f]+,(\[[^\]]+\]|[^\]]+))):(\d+)$`)The regex seems wrong to me.
TestSplitHostPort should fail for a test case like{"1-ff00:0:0,2a00:1450:400a:801::200", "", "", true},, but it does not. With the old one, it obviously would have.
Done. I actually didn't care too much, false positives seem acceptable here. But it was too easy to fix ;)
There was a problem hiding this comment.
Reviewed 3 of 13 files at r1, 2 of 2 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @matzf)
pkg/appnet/hosts_test.go, line 165 at r3 (raw file):
{"1-ff00:0:0,0:0:0:80", "", "", true},
And {"1-ff00:0:0,1.1.1.1", "", "", true},, then all simple cases should be covered
pkg/appnet/appquic/appquic.go, line 50 at r3 (raw file):
analogoues
Typo
Adapt usage of various APIs:
context.Background(), for now) in various Accept, Listen etc.session.Close()withsession.CloseWithErrorh2quicis nowhttp3http3now usesEarlySessioninstead of Session; add correspondingDialEarlyAPI toappquicNextProtoswhere neededAt the same time, slightly reorganize the usage of the
tls.Configparameter for the
appquic.Dialfunctions. AsNextProtosis now required,pass this explicitly for every usage.
As a related cleanup, don't automatically set
InsecureSkipVerifyandself-signed server certificates in the
appquiclibrary code anymore;moved to the applications, to make this hack a bit more apparent.
This change is