Skip to content

Commit

Permalink
Fix JSON compatibility in conf format (#4418)
Browse files Browse the repository at this point in the history 
Fixes a side effect from #4394 which was breaking support for JSON
configs.
  • Loading branch information
@wallyqs
wallyqs committed Aug 22, 2023
2 parents 8453676 + 62242a7 commit 9d11988
Show file tree
Hide file tree
Showing 4 changed files with 231 additions and 12 deletions.
1 change: 1 addition & 0 deletions conf/lex.go
View file
Expand Up @@ -78,6 +78,7 @@ const (
topOptTerm = '}'
blockStart = '('
blockEnd = ')'
mapEndString = string(mapEnd)
)

type stateFn func(lx *lexer) stateFn
Expand Down
40 changes: 40 additions & 0 deletions conf/lex_test.go
View file
Expand Up @@ -1471,6 +1471,8 @@ func TestJSONCompat(t *testing.T) {
expected: []item{
{itemKey, "http_port", 3, 28},
{itemInteger, "8223", 3, 40},
{itemKey, "}", 4, 25},
{itemEOF, "", 0, 0},
},
},
{
Expand All @@ -1486,6 +1488,8 @@ func TestJSONCompat(t *testing.T) {
{itemInteger, "8223", 3, 40},
{itemKey, "port", 4, 28},
{itemInteger, "4223", 4, 35},
{itemKey, "}", 5, 25},
{itemEOF, "", 0, 0},
},
},
{
Expand All @@ -1510,6 +1514,8 @@ func TestJSONCompat(t *testing.T) {
{itemBool, "true", 6, 36},
{itemKey, "max_control_line", 7, 28},
{itemInteger, "1024", 7, 47},
{itemKey, "}", 8, 25},
{itemEOF, "", 0, 0},
},
},
{
Expand All @@ -1521,6 +1527,7 @@ func TestJSONCompat(t *testing.T) {
{itemInteger, "8224", 1, 14},
{itemKey, "port", 1, 20},
{itemInteger, "4224", 1, 27},
{itemEOF, "", 0, 0},
},
},
{
Expand All @@ -1533,6 +1540,8 @@ func TestJSONCompat(t *testing.T) {
{itemInteger, "8225", 1, 14},
{itemKey, "port", 1, 20},
{itemInteger, "4225", 1, 27},
{itemKey, "}", 2, 25},
{itemEOF, "", 0, 0},
},
},
{
Expand All @@ -1557,6 +1566,8 @@ func TestJSONCompat(t *testing.T) {
{itemString, "nats://127.0.0.1:4224", 1, 140},
{itemArrayEnd, "", 1, 163},
{itemMapEnd, "", 1, 164},
{itemKey, "}", 14, 25},
{itemEOF, "", 0, 0},
},
},
{
Expand Down Expand Up @@ -1594,6 +1605,35 @@ func TestJSONCompat(t *testing.T) {
{itemString, "nats://127.0.0.1:4224", 11, 32},
{itemArrayEnd, "", 12, 30},
{itemMapEnd, "", 13, 28},
{itemKey, "}", 14, 25},
{itemEOF, "", 0, 0},
},
},
{
name: "should support JSON with blocks",
input: `{
"jetstream": {
"store_dir": "/tmp/nats"
"max_mem": 1000000,
},
"port": 4222,
"server_name": "nats1"
}
`,
expected: []item{
{itemKey, "jetstream", 2, 28},
{itemMapStart, "", 2, 41},
{itemKey, "store_dir", 3, 30},
{itemString, "/tmp/nats", 3, 43},
{itemKey, "max_mem", 4, 30},
{itemInteger, "1000000", 4, 40},
{itemMapEnd, "", 5, 28},
{itemKey, "port", 6, 28},
{itemInteger, "4222", 6, 35},
{itemKey, "server_name", 7, 28},
{itemString, "nats1", 7, 43},
{itemKey, "}", 8, 25},
{itemEOF, "", 0, 0},
},
},
} {
Expand Down
8 changes: 5 additions & 3 deletions conf/parse.go
View file
Expand Up @@ -137,16 +137,18 @@ func parse(data, fp string, pedantic bool) (p *parser, err error) {
}
p.pushContext(p.mapping)

var prevItem itemType
var prevItem item
for {
it := p.next()
if it.typ == itemEOF {
if prevItem == itemKey {
// Here we allow the final character to be a bracket '}'
// in order to support JSON like configurations.
if prevItem.typ == itemKey && prevItem.val != mapEndString {
return nil, fmt.Errorf("config is invalid (%s:%d:%d)", fp, it.line, it.pos)
}
break
}
prevItem = it.typ
prevItem = it
if err := p.processItem(it, fp); err != nil {
return nil, err
}
Expand Down
194 changes: 185 additions & 9 deletions conf/parse_test.go
View file
Expand Up @@ -442,15 +442,6 @@ func TestParseWithNoValuesAreInvalid(t *testing.T) {
`,
"config is invalid (:3:25)",
},
{
// trailing brackets accidentally can become keys, these are also invalid.
"trailing brackets after config",
`
accounts { users = [{}]}
}
`,
"config is invalid (:4:25)",
},
} {
t.Run(test.name, func(t *testing.T) {
if _, err := parse(test.conf, "", true); err == nil {
Expand Down Expand Up @@ -494,6 +485,48 @@ func TestParseWithNoValuesEmptyConfigsAreValid(t *testing.T) {
}
}

func TestParseWithTrailingBracketsAreValid(t *testing.T) {
for _, test := range []struct {
name string
conf string
}{
{
"empty conf",
"{}",
},
{
"just comments with no values",
`
{
# comments in the body
}
`,
},
{
// trailing brackets accidentally can become keys,
// this is valid since needed to support JSON like configs..
"trailing brackets after config",
`
accounts { users = [{}]}
}
`,
},
{
"wrapped in brackets",
`{
accounts { users = [{}]}
}
`,
},
} {
t.Run(test.name, func(t *testing.T) {
if _, err := parse(test.conf, "", true); err != nil {
t.Errorf("unexpected error: %v", err)
}
})
}
}

func TestParseWithNoValuesIncludes(t *testing.T) {
for _, test := range []struct {
input string
Expand Down Expand Up @@ -564,3 +597,146 @@ func TestParseWithNoValuesIncludes(t *testing.T) {
})
}
}

func TestJSONParseCompat(t *testing.T) {
for _, test := range []struct {
name string
input string
includes map[string]string
expected map[string]interface{}
}{
{
"JSON with nested blocks",
`
{
"http_port": 8227,
"port": 4227,
"write_deadline": "1h",
"cluster": {
"port": 6222,
"routes": [
"nats://127.0.0.1:4222",
"nats://127.0.0.1:4223",
"nats://127.0.0.1:4224"
]
}
}
`,
nil,
map[string]interface{}{
"http_port": int64(8227),
"port": int64(4227),
"write_deadline": "1h",
"cluster": map[string]interface{}{
"port": int64(6222),
"routes": []interface{}{
"nats://127.0.0.1:4222",
"nats://127.0.0.1:4223",
"nats://127.0.0.1:4224",
},
},
},
},
{
"JSON with nested blocks",
`{
"jetstream": {
"store_dir": "/tmp/nats"
"max_mem": 1000000,
},
"port": 4222,
"server_name": "nats1"
}
`,
nil,
map[string]interface{}{
"jetstream": map[string]interface{}{
"store_dir": "/tmp/nats",
"max_mem": int64(1_000_000),
},
"port": int64(4222),
"server_name": "nats1",
},
},
{
"JSON empty object in one line",
`{}`,
nil,
map[string]interface{}{},
},
{
"JSON empty object with line breaks",
`
{
}
`,
nil,
map[string]interface{}{},
},
{
"JSON includes",
`
accounts {
foo { include 'foo.json' }
bar { include 'bar.json' }
quux { include 'quux.json' }
}
`,
map[string]string{
"foo.json": `{ "users": [ {"user": "foo"} ] }`,
"bar.json": `{
"users": [ {"user": "bar"} ]
}`,
"quux.json": `{}`,
},
map[string]interface{}{
"accounts": map[string]interface{}{
"foo": map[string]interface{}{
"users": []interface{}{
map[string]interface{}{
"user": "foo",
},
},
},
"bar": map[string]interface{}{
"users": []interface{}{
map[string]interface{}{
"user": "bar",
},
},
},
"quux": map[string]interface{}{},
},
},
},
} {
t.Run(test.name, func(t *testing.T) {
sdir := t.TempDir()
f, err := os.CreateTemp(sdir, "nats.conf-")
if err != nil {
t.Fatal(err)
}
if err := os.WriteFile(f.Name(), []byte(test.input), 066); err != nil {
t.Error(err)
}
if test.includes != nil {
for includeFile, contents := range test.includes {
inf, err := os.Create(filepath.Join(sdir, includeFile))
if err != nil {
t.Fatal(err)
}
if err := os.WriteFile(inf.Name(), []byte(contents), 066); err != nil {
t.Error(err)
}
}
}
m, err := ParseFile(f.Name())
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if !reflect.DeepEqual(m, test.expected) {
t.Fatalf("Not Equal:\nReceived: '%+v'\nExpected: '%+v'\n", m, test.expected)
}
})
}
}

0 comments on commit 9d11988

Please sign in to comment.