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.

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

String envar incorrectly inferred as number #3431

Closed
2 tasks done
julian-dolce-form3 opened this issue Sep 1, 2022 · 1 comment · Fixed by #3434
Closed
2 tasks done

String envar incorrectly inferred as number #3431

julian-dolce-form3 opened this issue Sep 1, 2022 · 1 comment · Fixed by #3434
Assignees

Comments

@julian-dolce-form3
Copy link

julian-dolce-form3 commented Sep 1, 2022

Defect

A String envar that starts with a digit (0-9) followed by the second character being anyone of these...

return r == 'k' || r == 'K' || r == 'm' || r == 'M' || r == 'g' || r == 'G' || r == 't' || r == 'T' || r == 'p' || r == 'P' || r == 'e' || r == 'E'

and has a length > 2, is incorrectly inferred as a number and not a string.

After stepping through the code isNumberSuffix() gets called but I feel like it should only infer it's a number if the suffix is at the end of the string.

  • Included nats-server -DV output
  • Included a [Minimal, Complete, and Verifiable example]

Versions of nats-server and affected client libraries used: latest

OS/Container environment:

Steps or code to reproduce the issue:

Here is a unit test that shows the issue. It can go in conf/parse_test.go

func TestEnvVariableString(t *testing.T) {
	ex := map[string]interface{}{
		"foo": "2GP3JZCZ4G7JNWkzBhVYx9ZXtUiu",
	}
	evar := "__UNIQ22__"
	os.Setenv(evar, "2GP3JZCZ4G7JNWkzBhVYx9ZXtUiu")
	defer os.Unsetenv(evar)
	test(t, fmt.Sprintf("foo = $%s", evar), ex)
}

Expected result:

The test passes

Actual result:

Errors with

Received err: variable reference for '__UNIQ22__' on line 1 could not be parsed: Parse error on line 1: 'Expected a top-level value to end with a new line, comment or EOF, but got 'P' instead.'
@kozlovic
Copy link
Member

kozlovic commented Sep 2, 2022

@julian-dolce-form3 As you can see in parse_test.go, we have a test called TestEnvVariableStringStartingWithNumber that catches that we fail if a string (without quotes) starts with a number. To parse the string which starts with a number one would have to use quotes, like in test TestEnvVariableStringStartingWithNumberUsingQuotes.
Would that be possible for you to set the env variable surrounded with quotes '?

kozlovic added a commit that referenced this issue Sep 3, 2022
If a configuration variable starts with numbers and has a character
that such as K/k/G/g/etc.. it would assume that it was a number
(expressed in Kb, Gb, etc..).

This PR checks that if the special characters are not the suffix,
that is, the variable does not end after those characters, then
the parsing will treat the whole thing as a string.

Resolves #3431

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@bruth bruth removed the 🐞 bug label Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants