Skip to content

Commit

Permalink
pgwire: fix the handling of the cancel request
Browse files Browse the repository at this point in the history
This fixes a telemetry regression that was introduced in the "great
HBA refactor".

Release note: None
  • Loading branch information
knz committed Apr 21, 2020
1 parent 0315d68 commit 2419a37
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 32 deletions.
56 changes: 32 additions & 24 deletions pkg/sql/pgwire/pgwire_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1867,33 +1867,41 @@ var _ pgx.Logger = pgxTestLogger{}
func TestCancelRequest(t *testing.T) {
defer leaktest.AfterTest(t)()

params := base.TestServerArgs{Insecure: true}
s, _, _ := serverutils.StartServer(t, params)
testutils.RunTrueAndFalse(t, "insecure", func(t *testing.T, insecure bool) {
params := base.TestServerArgs{Insecure: insecure}
s, _, _ := serverutils.StartServer(t, params)

ctx := context.TODO()
defer s.Stopper().Stop(ctx)
ctx := context.TODO()
defer s.Stopper().Stop(ctx)

var d net.Dialer
conn, err := d.DialContext(ctx, "tcp", s.ServingSQLAddr())
if err != nil {
t.Fatal(err)
}
defer conn.Close()
var d net.Dialer
conn, err := d.DialContext(ctx, "tcp", s.ServingSQLAddr())
if err != nil {
t.Fatal(err)
}
defer conn.Close()

fe, err := pgproto3.NewFrontend(conn, conn)
if err != nil {
t.Fatal(err)
}
const versionCancel = 80877102
if err := fe.Send(&pgproto3.StartupMessage{ProtocolVersion: versionCancel}); err != nil {
t.Fatal(err)
}
if _, err := fe.Receive(); err != io.EOF {
t.Fatalf("unexpected: %v", err)
}
if count := telemetry.GetRawFeatureCounts()["pgwire.unimplemented.cancel_request"]; count != 1 {
t.Fatalf("expected 1 cancel request, got %d", count)
}
// Reset telemetry so we get a deterministic count below.
_ = telemetry.GetFeatureCounts(telemetry.Raw, telemetry.ResetCounts)

fe, err := pgproto3.NewFrontend(conn, conn)
if err != nil {
t.Fatal(err)
}
// versionCancel is the special code sent as header for cancel requests.
// See: https://www.postgresql.org/docs/current/protocol-message-formats.html
// and the explanation in server.go.
const versionCancel = 80877102
if err := fe.Send(&pgproto3.StartupMessage{ProtocolVersion: versionCancel}); err != nil {
t.Fatal(err)
}
if _, err := fe.Receive(); err != io.EOF {
t.Fatalf("unexpected: %v", err)
}
if count := telemetry.GetRawFeatureCounts()["pgwire.unimplemented.cancel_request"]; count != 1 {
t.Fatalf("expected 1 cancel request, got %d", count)
}
})
}

func TestFailPrepareFailsTxn(t *testing.T) {
Expand Down
19 changes: 11 additions & 8 deletions pkg/sql/pgwire/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,17 @@ func (s *Server) ServeConn(ctx context.Context, conn net.Conn, socketType Socket
return err
}

if version == versionCancel {
// The cancel message is rather peculiar: it is sent without
// authentication, always over an unencrypted channel.
//
// Since we don't support this, close the door in the client's
// face. Make a note of that use in telemetry.
telemetry.Inc(sqltelemetry.CancelRequestCounter)
_ = conn.Close()
return nil
}

// If the server is shutting down, terminate the connection early.
if draining {
return s.sendErr(ctx, conn, newAdminShutdownErr(ErrDrainingNewConn))
Expand All @@ -512,14 +523,6 @@ func (s *Server) ServeConn(ctx context.Context, conn net.Conn, socketType Socket

// What does the client want to do?
switch version {
case versionCancel:
// If the client is really issuing a cancel request, close the door
// in their face (we don't support it yet). Make a note of that use
// in telemetry.
telemetry.Inc(sqltelemetry.CancelRequestCounter)
_ = conn.Close()
return nil

case version30:
// Normal SQL connection. Proceed normally below.

Expand Down

0 comments on commit 2419a37

Please sign in to comment.