Skip to content

Commit

Permalink
fix(client): modify config parsing behavior to match other clients (#28)
Browse files Browse the repository at this point in the history 
Co-authored-by: Andrey Pechkurov <[email protected]>
  • Loading branch information
@sklarsa @puzpuzpuz
sklarsa and puzpuzpuz authored Mar 22, 2024
1 parent 8a27774 commit 0f785ff
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 79 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Features:
* Optimized for batch writes.
* Supports TLS encryption and ILP authentication.
* Automatic write retries and connection reuse for ILP over HTTP.
* Tested against QuestDB 7.3.11 and newer versions.
* Tested against QuestDB 7.3.10 and newer versions.

New in v3:
* Supports ILP over HTTP using the same client semantics
Expand Down
19 changes: 10 additions & 9 deletions conf_parse.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func confFromStr(conf string) (*lineSenderConfig, error) {
}

for k, v := range data.KeyValuePairs {
switch strings.ToLower(k) {
switch k {
case "addr":
senderConf.address = v
case "username":
Expand All @@ -86,6 +86,10 @@ func confFromStr(conf string) (*lineSenderConfig, error) {
default:
panic("add a case for " + k)
}
case "token_x":
case "token_y":
// Some clients require public key.
// But since Go sender doesn't need it, we ignore the values.
case "auto_flush":
if v == "off" {
senderConf.autoFlushRows = 0
Expand Down Expand Up @@ -166,9 +170,8 @@ func parseConfigStr(conf string) (configData, error) {
KeyValuePairs: map[string]string{},
}

nextRune rune
isEscaping bool
hasTrailingSemicolon bool
nextRune rune
isEscaping bool
)

schemaStr, conf, found := strings.Cut(conf, "::")
Expand All @@ -182,10 +185,8 @@ func parseConfigStr(conf string) (configData, error) {
return result, NewInvalidConfigStrError("'addr' key not found")
}

if strings.HasSuffix(conf, ";") {
hasTrailingSemicolon = true
} else {
conf = conf + ";" // add trailing semicolon if it doesn't exist
if !strings.HasSuffix(conf, ";") {
return result, NewInvalidConfigStrError("trailing semicolon ';' required")
}

keyValueStr := []rune(conf)
Expand All @@ -198,7 +199,7 @@ func parseConfigStr(conf string) (configData, error) {
switch rune {
case ';':
if isKey {
if nextRune == 0 && !hasTrailingSemicolon {
if nextRune == 0 {
return result, NewInvalidConfigStrError("unexpected end of string")
}
return result, NewInvalidConfigStrError("invalid key character ';'")
Expand Down
93 changes: 48 additions & 45 deletions conf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestParserHappyCases(t *testing.T) {
testCases := []parseConfigTestCase{
{
name: "http and ipv4 address",
config: fmt.Sprintf("http::addr=%s", addr),
config: fmt.Sprintf("http::addr=%s;", addr),
expected: qdb.ConfigData{
Schema: "http",
KeyValuePairs: map[string]string{
Expand All @@ -74,7 +74,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "tcp and address",
config: fmt.Sprintf("tcp::addr=%s", addr),
config: fmt.Sprintf("tcp::addr=%s;", addr),
expected: qdb.ConfigData{
Schema: "tcp",
KeyValuePairs: map[string]string{
Expand All @@ -84,7 +84,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "http and username/password",
config: fmt.Sprintf("http::addr=%s;username=%s;password=%s", addr, user, pass),
config: fmt.Sprintf("http::addr=%s;username=%s;password=%s;", addr, user, pass),
expected: qdb.ConfigData{
Schema: "http",
KeyValuePairs: map[string]string{
Expand All @@ -107,7 +107,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "tcp with key and user",
config: fmt.Sprintf("tcp::addr=%s;token=%s;username=%s", addr, token, user),
config: fmt.Sprintf("tcp::addr=%s;token=%s;username=%s;", addr, token, user),
expected: qdb.ConfigData{
Schema: "tcp",
KeyValuePairs: map[string]string{
Expand All @@ -119,7 +119,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "https with min_throughput",
config: fmt.Sprintf("https::addr=%s;min_throughput=%d", addr, min_throughput),
config: fmt.Sprintf("https::addr=%s;min_throughput=%d;", addr, min_throughput),
expected: qdb.ConfigData{
Schema: "https",
KeyValuePairs: map[string]string{
Expand All @@ -130,7 +130,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "https with min_throughput, init_buf_size and tls_verify=unsafe_off",
config: fmt.Sprintf("https::addr=%s;min_throughput=%d;init_buf_size=%d;tls_verify=unsafe_off", addr, min_throughput, 1024),
config: fmt.Sprintf("https::addr=%s;min_throughput=%d;init_buf_size=%d;tls_verify=unsafe_off;", addr, min_throughput, 1024),
expected: qdb.ConfigData{
Schema: "https",
KeyValuePairs: map[string]string{
Expand All @@ -143,7 +143,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "tcps with tls_verify=unsafe_off",
config: fmt.Sprintf("tcps::addr=%s;tls_verify=unsafe_off", addr),
config: fmt.Sprintf("tcps::addr=%s;tls_verify=unsafe_off;", addr),
expected: qdb.ConfigData{
Schema: "tcps",
KeyValuePairs: map[string]string{
Expand All @@ -154,7 +154,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "http with min_throughput, request_timeout, and retry_timeout",
config: fmt.Sprintf("http::addr=%s;min_throughput=%d;request_timeout=%d;retry_timeout=%d",
config: fmt.Sprintf("http::addr=%s;min_throughput=%d;request_timeout=%d;retry_timeout=%d;",
addr, min_throughput, request_timeout.Milliseconds(), retry_timeout.Milliseconds()),
expected: qdb.ConfigData{
Schema: "http",
Expand All @@ -168,7 +168,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "tcp with tls_verify=on",
config: fmt.Sprintf("tcp::addr=%s;tls_verify=on", addr),
config: fmt.Sprintf("tcp::addr=%s;tls_verify=on;", addr),
expected: qdb.ConfigData{
Schema: "tcp",
KeyValuePairs: map[string]string{
Expand All @@ -177,18 +177,6 @@ func TestParserHappyCases(t *testing.T) {
},
},
},
{
name: "password with an escaped semicolon",
config: fmt.Sprintf("http::addr=%s;username=%s;password=pass;;word", addr, user),
expected: qdb.ConfigData{
Schema: "http",
KeyValuePairs: map[string]string{
"addr": addr,
"username": user,
"password": "pass;word",
},
},
},
{
name: "password with an escaped semicolon (ending with a ';')",
config: fmt.Sprintf("http::addr=%s;username=%s;password=pass;;word;", addr, user),
Expand All @@ -215,7 +203,7 @@ func TestParserHappyCases(t *testing.T) {
},
{
name: "equal sign in password",
config: fmt.Sprintf("http::addr=%s;username=%s;password=pass=word", addr, user),
config: fmt.Sprintf("http::addr=%s;username=%s;password=pass=word;", addr, user),
expected: qdb.ConfigData{
Schema: "http",
KeyValuePairs: map[string]string{
Expand Down Expand Up @@ -253,15 +241,10 @@ func TestParserPathologicalCases(t *testing.T) {
config: "http::",
expectedErrMsgContains: "'addr' key not found",
},
{
name: "unescaped semicolon in password leads to unexpected end of string",
config: "http::addr=localhost:9000;username=test;password=pass;word",
expectedErrMsgContains: "unexpected end of string",
},
{
name: "unescaped semicolon in password leads to invalid key character",
config: "http::addr=localhost:9000;username=test;password=pass;word;",
expectedErrMsgContains: "invalid key character ';'",
expectedErrMsgContains: "unexpected end of",
},
}

Expand Down Expand Up @@ -299,7 +282,17 @@ func TestHappyCasesFromConf(t *testing.T) {
testCases := []configTestCase{
{
name: "user and token",
config: fmt.Sprintf("tcp::addr=%s;username=%s;token=%s",
config: fmt.Sprintf("tcp::addr=%s;username=%s;token=%s;",
addr, user, token),
expectedOpts: []qdb.LineSenderOption{
qdb.WithTcp(),
qdb.WithAddress(addr),
qdb.WithAuth(user, token),
},
},
{
name: "token_x and token_y (ignored)",
config: fmt.Sprintf("tcp::addr=%s;username=%s;token=%s;token_x=xyz;token_y=xyz;",
addr, user, token),
expectedOpts: []qdb.LineSenderOption{
qdb.WithTcp(),
Expand All @@ -309,7 +302,7 @@ func TestHappyCasesFromConf(t *testing.T) {
},
{
name: "init_buf_size and max_buf_size",
config: fmt.Sprintf("tcp::addr=%s;init_buf_size=%d;max_buf_size=%d",
config: fmt.Sprintf("tcp::addr=%s;init_buf_size=%d;max_buf_size=%d;",
addr, initBufSize, maxBufSize),
expectedOpts: []qdb.LineSenderOption{
qdb.WithTcp(),
Expand All @@ -320,7 +313,7 @@ func TestHappyCasesFromConf(t *testing.T) {
},
{
name: "with tls",
config: fmt.Sprintf("tcp::addr=%s;tls_verify=on",
config: fmt.Sprintf("tcp::addr=%s;tls_verify=on;",
addr),
expectedOpts: []qdb.LineSenderOption{
qdb.WithTcp(),
Expand All @@ -330,7 +323,7 @@ func TestHappyCasesFromConf(t *testing.T) {
},
{
name: "with tls and unsafe_off",
config: fmt.Sprintf("tcp::addr=%s;tls_verify=unsafe_off",
config: fmt.Sprintf("tcp::addr=%s;tls_verify=unsafe_off;",
addr),
expectedOpts: []qdb.LineSenderOption{
qdb.WithTcp(),
Expand All @@ -340,7 +333,7 @@ func TestHappyCasesFromConf(t *testing.T) {
},
{
name: "request_timeout and retry_timeout milli conversion",
config: fmt.Sprintf("http::addr=%s;request_timeout=%d;retry_timeout=%d",
config: fmt.Sprintf("http::addr=%s;request_timeout=%d;retry_timeout=%d;",
addr, requestTimeout.Milliseconds(), retryTimeout.Milliseconds()),
expectedOpts: []qdb.LineSenderOption{
qdb.WithHttp(),
Expand All @@ -351,7 +344,7 @@ func TestHappyCasesFromConf(t *testing.T) {
},
{
name: "password before username",
config: fmt.Sprintf("http::addr=%s;password=%s;username=%s",
config: fmt.Sprintf("http::addr=%s;password=%s;username=%s;",
addr, pass, user),
expectedOpts: []qdb.LineSenderOption{
qdb.WithHttp(),
Expand All @@ -361,7 +354,7 @@ func TestHappyCasesFromConf(t *testing.T) {
},
{
name: "min_throughput",
config: fmt.Sprintf("http::addr=%s;min_throughput=%d",
config: fmt.Sprintf("http::addr=%s;min_throughput=%d;",
addr, minThroughput),
expectedOpts: []qdb.LineSenderOption{
qdb.WithHttp(),
Expand All @@ -371,7 +364,7 @@ func TestHappyCasesFromConf(t *testing.T) {
},
{
name: "bearer token",
config: fmt.Sprintf("http::addr=%s;token=%s",
config: fmt.Sprintf("http::addr=%s;token=%s;",
addr, token),
expectedOpts: []qdb.LineSenderOption{
qdb.WithHttp(),
Expand All @@ -381,7 +374,7 @@ func TestHappyCasesFromConf(t *testing.T) {
},
{
name: "auto flush",
config: fmt.Sprintf("http::addr=%s;auto_flush_rows=100;auto_flush_interval=1000",
config: fmt.Sprintf("http::addr=%s;auto_flush_rows=100;auto_flush_interval=1000;",
addr),
expectedOpts: []qdb.LineSenderOption{
qdb.WithHttp(),
Expand Down Expand Up @@ -416,44 +409,54 @@ func TestPathologicalCasesFromConf(t *testing.T) {
},
{
name: "invalid schema",
config: "foobar::addr=localhost:1111",
config: "foobar::addr=localhost:1111;",
expectedErrMsgContains: "invalid schema",
},
{
name: "invalid tls_verify 1",
config: "tcp::addr=localhost:1111;tls_verify=invalid",
config: "tcp::addr=localhost:1111;tls_verify=invalid;",
expectedErrMsgContains: "invalid tls_verify",
},
{
name: "invalid tls_verify 2",
config: "http::addr=localhost:1111;tls_verify=invalid",
config: "http::addr=localhost:1111;tls_verify=invalid;",
expectedErrMsgContains: "invalid tls_verify",
},
{
name: "unsupported option",
config: "tcp::addr=localhost:1111;unsupported_option=invalid",
config: "tcp::addr=localhost:1111;unsupported_option=invalid;",
expectedErrMsgContains: "unsupported option",
},
{
name: "invalid auto_flush",
config: "http::addr=localhost:1111;auto_flush=invalid",
config: "http::addr=localhost:1111;auto_flush=invalid;",
expectedErrMsgContains: "invalid auto_flush",
},
{
name: "invalid auto_flush_rows",
config: "http::addr=localhost:1111;auto_flush_rows=invalid",
config: "http::addr=localhost:1111;auto_flush_rows=invalid;",
expectedErrMsgContains: "invalid auto_flush_rows",
},
{
name: "invalid auto_flush_interval",
config: "http::addr=localhost:1111;auto_flush_interval=invalid",
config: "http::addr=localhost:1111;auto_flush_interval=invalid;",
expectedErrMsgContains: "invalid auto_flush_interval",
},
{
name: "unsupported option",
config: "http::addr=localhost:1111;unsupported_option=invalid",
config: "http::addr=localhost:1111;unsupported_option=invalid;",
expectedErrMsgContains: "unsupported option",
},
{
name: "case-sensitive values",
config: "http::aDdr=localhost:9000;",
expectedErrMsgContains: "unsupported option",
},
{
name: "trailing semicolon required",
config: "http::addr=localhost:9000",
expectedErrMsgContains: "trailing semicolon",
},
}

for _, tc := range testCases {
Expand Down
Loading

0 comments on commit 0f785ff

Please sign in to comment.