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.

Sign up for GitHub

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

Jump to bottom

proxy: remove sequence check #410

Merged
merged 10 commits into from
Dec 22, 2023
Merged

proxy: remove sequence check #410

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
182ca58
proxy: remove sequence check
xhebox Dec 1, 2023
3fe459f
relax check
xhebox Dec 1, 2023
429cb29
remove unused
xhebox Dec 1, 2023
148f285
revert compress change
xhebox Dec 1, 2023
3299ca3
address comments
xhebox Dec 1, 2023
903f698
remove config change
xhebox Dec 1, 2023
cdf33ef
address comments
xhebox Dec 1, 2023
59d7ab3
address comments
xhebox Dec 2, 2023
92eb72a
address comments
xhebox Dec 22, 2023
85b3776
Merge branch 'main' into main
xhebox Dec 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/proxy/backend/authenticator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ loop:
if err != nil {
// tiproxy pp enabled, tidb pp disabled, tls disabled => invalid sequence
// tiproxy pp disabled, tidb pp enabled, tls disabled => invalid sequence
if pktIdx == 0 && errors.Is(err, pnet.ErrInvalidSequence) {
if pktIdx == 0 {
xhebox marked this conversation as resolved.
Show resolved Hide resolved
return errors.Wrap(ErrBackendPPV2, err)
}
return err
Expand Down
4 changes: 4 additions & 0 deletions pkg/proxy/backend/authenticator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,10 @@ func TestProxyProtocol(t *testing.T) {
cfgOverriders := getCfgCombinations(cfgs)
for _, cfgs := range cfgOverriders {
ts, clean := newTestSuite(t, tc, cfgs...)
// invalid sequence detection removed, backend will stuck if clients insists to send proxy header.
xhebox marked this conversation as resolved.
Show resolved Hide resolved
if !ts.mb.proxyProtocol && ts.mp.bcConfig.ProxyProtocol {
continue
}
ts.authenticateFirstTime(t, func(t *testing.T, ts *testSuite) {
// TiDB proxy-protocol can be set unfallbackable, but TiProxy proxy-protocol is always fallbackable.
// So when backend enables proxy-protocol and proxy disables it, it still works well.
Expand Down
12 changes: 2 additions & 10 deletions pkg/proxy/net/packetio.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,7 @@ func (p *PacketIO) readOnePacket() ([]byte, bool, error) {
if err := ReadFull(p.readWriter, p.header); err != nil {
return nil, false, errors.Wrap(ErrReadConn, err)
}
sequence, pktSequence := p.header[3], p.readWriter.Sequence()
if sequence != pktSequence {
return nil, false, ErrInvalidSequence.GenWithStack("invalid sequence, expected %d, actual %d", pktSequence, sequence)
}
p.readWriter.SetSequence(sequence + 1)
p.readWriter.SetSequence(p.header[3] + 1)

length := int(p.header[0]) | int(p.header[1])<<8 | int(p.header[2])<<16
data := make([]byte, length)
Expand Down Expand Up @@ -350,11 +346,7 @@ func (p *PacketIO) ForwardUntil(dest *PacketIO, isEnd func(firstByte byte, first
}
} else {
for {
sequence, pktSequence := header[3], p.readWriter.Sequence()
if sequence != pktSequence {
return p.wrapErr(ErrInvalidSequence.GenWithStack("invalid sequence, expected %d, actual %d", pktSequence, sequence))
}
p.readWriter.SetSequence(sequence + 1)
p.readWriter.SetSequence(header[3] + 1)
// Sequence may be different (e.g. with compression) so we can't just copy the data to the destination.
dest.readWriter.SetSequence(dest.readWriter.Sequence() + 1)
p.limitReader.N = int64(length + 4)
Expand Down
Loading