X Tutup
Skip to content

Commit 654bd29

Browse files
mislavmislav
committed
Disallow unsupported values for --json flag
1 parent 56cdbb6 commit 654bd29

File tree

2 files changed

+182
-0
lines changed

2 files changed

+182
-0
lines changed

‎pkg/cmdutil/json_flags.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/cli/cli/pkg/export"
1313
"github.com/cli/cli/pkg/jsoncolor"
14+
"github.com/cli/cli/pkg/set"
1415
"github.com/spf13/cobra"
1516
"github.com/spf13/pflag"
1617
)
@@ -36,6 +37,14 @@ func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) {
3637
if export == nil {
3738
*exportTarget = nil
3839
} else {
40+
allowedFields := set.NewStringSet()
41+
allowedFields.AddValues(fields)
42+
for _, f := range export.fields {
43+
if !allowedFields.Contains(f) {
44+
sort.Strings(fields)
45+
return JSONFlagError{fmt.Errorf("Unknown JSON field: %q\nAvailable fields:\n %s", f, strings.Join(fields, "\n "))}
46+
}
47+
}
3948
*exportTarget = export
4049
}
4150
} else {

‎pkg/cmdutil/json_flags_test.go‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
package cmdutil
2+
3+
import (
4+
"bytes"
5+
"io/ioutil"
6+
"testing"
7+
8+
"github.com/spf13/cobra"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestAddJSONFlags(t *testing.T) {
14+
tests := []struct {
15+
name string
16+
fields []string
17+
args []string
18+
wantsExport *exportFormat
19+
wantsError string
20+
}{
21+
{
22+
name: "no JSON flag",
23+
fields: []string{},
24+
args: []string{},
25+
wantsExport: nil,
26+
},
27+
{
28+
name: "empty JSON flag",
29+
fields: []string{"one", "two"},
30+
args: []string{"--json"},
31+
wantsExport: nil,
32+
wantsError: "Specify one or more comma-separated fields for `--json`:\n one\n two",
33+
},
34+
{
35+
name: "invalid JSON field",
36+
fields: []string{"id", "number"},
37+
args: []string{"--json", "idontexist"},
38+
wantsExport: nil,
39+
wantsError: "Unknown JSON field: \"idontexist\"\nAvailable fields:\n id\n number",
40+
},
41+
{
42+
name: "cannot combine --json with --web",
43+
fields: []string{"id", "number", "title"},
44+
args: []string{"--json", "id", "--web"},
45+
wantsExport: nil,
46+
wantsError: "cannot use `--web` with `--json`",
47+
},
48+
{
49+
name: "cannot use --jq without --json",
50+
fields: []string{},
51+
args: []string{"--jq", ".number"},
52+
wantsExport: nil,
53+
wantsError: "cannot use `--jq` without specifying `--json`",
54+
},
55+
{
56+
name: "cannot use --template without --json",
57+
fields: []string{},
58+
args: []string{"--template", "{{.number}}"},
59+
wantsExport: nil,
60+
wantsError: "cannot use `--template` without specifying `--json`",
61+
},
62+
{
63+
name: "with JSON fields",
64+
fields: []string{"id", "number", "title"},
65+
args: []string{"--json", "number,title"},
66+
wantsExport: &exportFormat{
67+
fields: []string{"number", "title"},
68+
filter: "",
69+
template: "",
70+
},
71+
},
72+
{
73+
name: "with jq filter",
74+
fields: []string{"id", "number", "title"},
75+
args: []string{"--json", "number", "-q.number"},
76+
wantsExport: &exportFormat{
77+
fields: []string{"number"},
78+
filter: ".number",
79+
template: "",
80+
},
81+
},
82+
{
83+
name: "with Go template",
84+
fields: []string{"id", "number", "title"},
85+
args: []string{"--json", "number", "-t", "{{.number}}"},
86+
wantsExport: &exportFormat{
87+
fields: []string{"number"},
88+
filter: "",
89+
template: "{{.number}}",
90+
},
91+
},
92+
}
93+
for _, tt := range tests {
94+
t.Run(tt.name, func(t *testing.T) {
95+
cmd := &cobra.Command{Run: func(*cobra.Command, []string) {}}
96+
cmd.Flags().Bool("web", false, "")
97+
var exporter Exporter
98+
AddJSONFlags(cmd, &exporter, tt.fields)
99+
cmd.SetArgs(tt.args)
100+
cmd.SetOut(ioutil.Discard)
101+
cmd.SetErr(ioutil.Discard)
102+
_, err := cmd.ExecuteC()
103+
if tt.wantsError == "" {
104+
require.NoError(t, err)
105+
} else {
106+
assert.EqualError(t, err, tt.wantsError)
107+
return
108+
}
109+
if tt.wantsExport == nil {
110+
assert.Nil(t, exporter)
111+
} else {
112+
assert.Equal(t, tt.wantsExport, exporter)
113+
}
114+
})
115+
}
116+
}
117+
118+
func Test_exportFormat_Write(t *testing.T) {
119+
type args struct {
120+
data interface{}
121+
colorEnabled bool
122+
}
123+
tests := []struct {
124+
name string
125+
exporter exportFormat
126+
args args
127+
wantW string
128+
wantErr bool
129+
}{
130+
{
131+
name: "regular JSON output",
132+
exporter: exportFormat{},
133+
args: args{
134+
data: map[string]string{"name": "hubot"},
135+
colorEnabled: false,
136+
},
137+
wantW: "{\"name\":\"hubot\"}\n",
138+
wantErr: false,
139+
},
140+
{
141+
name: "with jq filter",
142+
exporter: exportFormat{filter: ".name"},
143+
args: args{
144+
data: map[string]string{"name": "hubot"},
145+
colorEnabled: false,
146+
},
147+
wantW: "hubot\n",
148+
wantErr: false,
149+
},
150+
{
151+
name: "with Go template",
152+
exporter: exportFormat{template: "{{.name}}"},
153+
args: args{
154+
data: map[string]string{"name": "hubot"},
155+
colorEnabled: false,
156+
},
157+
wantW: "hubot",
158+
wantErr: false,
159+
},
160+
}
161+
for _, tt := range tests {
162+
t.Run(tt.name, func(t *testing.T) {
163+
w := &bytes.Buffer{}
164+
if err := tt.exporter.Write(w, tt.args.data, tt.args.colorEnabled); (err != nil) != tt.wantErr {
165+
t.Errorf("exportFormat.Write() error = %v, wantErr %v", err, tt.wantErr)
166+
return
167+
}
168+
if gotW := w.String(); gotW != tt.wantW {
169+
t.Errorf("exportFormat.Write() = %v, want %v", gotW, tt.wantW)
170+
}
171+
})
172+
}
173+
}

0 commit comments

Comments
 (0)
X Tutup