Skip to content

Commit eb3a566

Browse files
committed
fix protocol negotiation error handling
In CASSGO-97 and #1920 the handling was changed to better support non-zero stream id error responses. But because of hardcoding of version 5 in the test harness there was a regression where older protocol responses were not supported anymore. By adding an Unwrap method to ErrProtocol now the existing checking in checkProtocolRelatedError will correctly catch the protocolError. Patch by James Hartig; reviewed by Bohdan Siryk for CASSGO-131
1 parent 9bc3b5d commit eb3a566

4 files changed

Lines changed: 23 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- Improve host_source locking and ring refresh concurrency (CASSGO-121)
1414
- Add PreparedMetadata (Keyspace, Table) and IsPrepared fields to ObservedQuery, and parallel PreparedMetadata / IsPrepared slices to ObservedBatch, for statement-level observability without CQL parsing (CASSGO-119)
1515

16+
### Fixed
17+
- Correct protocol negotiation with non-Cassandra servers (CASSGO-131)
18+
1619
## [2.1.2]
1720

1821
### Fixed

conn_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1259,7 +1259,12 @@ func (srv *TestServer) process(conn net.Conn, reqFrame *framer, useProtoV5, star
12591259
srv.errorLocked("process frame with a nil header")
12601260
return
12611261
}
1262-
respFrame := newFramer(nil, byte(head.version), GlobalTypes)
1262+
// use the configured version unless it wasn't specified
1263+
version := srv.protocol
1264+
if version == 0 {
1265+
version = byte(head.version)
1266+
}
1267+
respFrame := newFramer(nil, version, GlobalTypes)
12631268

12641269
if srv.customRequestHandler != nil {
12651270
if err := srv.customRequestHandler(srv, reqFrame, respFrame); err != nil {

protocol_negotiation_test.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,18 @@ func TestProtocolNegotiation(t *testing.T) {
231231
forceZeroStreamID: tc.forceZeroStreamID,
232232
}
233233

234+
// use the maximum protocol supported
235+
protocol := uint8(0)
236+
for _, supportedVersion := range tc.supportedVersions {
237+
supportedProto := uint8(supportedVersion)
238+
if supportedProto > protocol {
239+
protocol = supportedProto
240+
}
241+
}
242+
234243
srv := newTestServerOpts{
235244
addr: "127.0.0.1:0",
236-
protocol: 5,
245+
protocol: protocol,
237246
customRequestHandler: handler.handle,
238247
dontFailOnProtocolMismatch: true,
239248
}.newServer(t, context.Background())

session.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2532,6 +2532,10 @@ var (
25322532
// ErrProtocol represents a protocol-level error.
25332533
type ErrProtocol struct{ error }
25342534

2535+
func (e ErrProtocol) Unwrap() error {
2536+
return e.error
2537+
}
2538+
25352539
// NewErrProtocol creates a new protocol error with the specified format and arguments.
25362540
func NewErrProtocol(format string, args ...interface{}) error {
25372541
return ErrProtocol{fmt.Errorf(format, args...)}

0 commit comments

Comments
 (0)