Skip to content

Commit

Permalink
Return detailed C* error for USE query. More test comments
Browse files Browse the repository at this point in the history
  • Loading branch information
lukasz-antoniak committed Jul 29, 2024
1 parent bd60d74 commit cd418fe
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 6 deletions.
14 changes: 9 additions & 5 deletions parser/lexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,15 @@ func TestLexerIdentifiers(t *testing.T) {
{`"system"`, tkIdentifier, "system"},
{`"system"`, tkIdentifier, "system"},
{`"System"`, tkIdentifier, "System"},
{`""""`, tkIdentifier, "\""},
{`""""""`, tkIdentifier, "\"\""},
{`"A"""""`, tkIdentifier, "A\"\""},
{`"""A"""`, tkIdentifier, "\"A\""},
{`"""""A"`, tkIdentifier, "\"\"A"},
// below test verify correct escaping double quote character as per CQL definition:
// identifier ::= unquoted_identifier | quoted_identifier
// unquoted_identifier ::= re('[a-zA-Z][link:[a-zA-Z0-9]]*')
// quoted_identifier ::= '"' (any character where " can appear if doubled)+ '"'
{`""""`, tkIdentifier, "\""}, // outermost quotes indicate quoted string, inner two double quotes shall be treated as single quote
{`""""""`, tkIdentifier, "\"\""}, // same as above, but 4 inner quotes result in 2 quotes
{`"A"""""`, tkIdentifier, "A\"\""}, // outermost quotes indicate quoted string, 4 quotes after A result in 2 quotes
{`"""A"""`, tkIdentifier, "\"A\""}, // outermost quotes indicate quoted string, 2 quotes before and after A result in single quotes
{`"""""A"`, tkIdentifier, "\"\"A"}, // analogical to previous tests
{`";`, tkInvalid, ""},
{`"""`, tkIdentifier, ""},
}
Expand Down
9 changes: 8 additions & 1 deletion proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,14 @@ func (c *client) interceptSystemQuery(hdr *frame.Header, stmt interface{}) {
}
case *parser.UseStatement:
if _, err := c.proxy.maybeCreateSession(hdr.Version, s.Keyspace); err != nil {
c.send(hdr, &message.ServerError{ErrorMessage: "Proxy unable to create new session for keyspace"})
var cqlError *proxycore.CqlError
switch {
case errors.As(err, &cqlError):
errMsg := cqlError.Message
c.send(hdr, errMsg)
default:
c.send(hdr, &message.ServerError{ErrorMessage: "Proxy unable to create new session for keyspace"})
}
} else {
c.keyspace = s.Keyspace
// We might have received a quoted keyspace name in the UseStatement so remove any
Expand Down
31 changes: 31 additions & 0 deletions proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,37 @@ func TestProxy_UseKeyspace(t *testing.T) {
}
}

func TestProxy_UseKeyspace_Error(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
tester, proxyContactPoint, err := setupProxyTest(ctx, 1, proxycore.MockRequestHandlers{
primitive.OpCodeQuery: func(cl *proxycore.MockClient, frm *frame.Frame) message.Message {
qry := frm.Body.Message.(*message.Query)
if qry.Query == "USE non_existing" {
return &message.ServerError{
ErrorMessage: "Keyspace 'non_existing' does not exist",
}
}
return cl.InterceptQuery(frm.Header, frm.Body.Message.(*message.Query))
}})
defer func() {
cancel()
tester.shutdown()
}()
require.NoError(t, err)

cl := connectTestClient(t, ctx, proxyContactPoint)

resp, err := cl.SendAndReceive(ctx, frame.NewFrame(primitive.ProtocolVersion4, 0, &message.Query{Query: "USE non_existing"}))
require.NoError(t, err)

assert.Equal(t, primitive.OpCodeError, resp.Header.OpCode)
res, ok := resp.Body.Message.(*message.ServerError)
require.True(t, ok)
// make sure that CQL Proxy returns the same error of 'USE keyspace' command
// as backend C* cluster has and does not wrap it inside a custom one
assert.Equal(t, "Keyspace 'non_existing' does not exist", res.ErrorMessage)
}

func TestProxy_NegotiateProtocolV5(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
tester, proxyContactPoint, err := setupProxyTest(ctx, 1, nil)
Expand Down

0 comments on commit cd418fe

Please sign in to comment.