Skip to content

Commit f8481d1

Browse files
reshmifrogreshmifrog
authored
Fix curl argument parsing (#1502)
1 parent 3cc5f34 commit f8481d1

File tree

4 files changed

+346
-30
lines changed

4 files changed

+346
-30
lines changed

‎common/commands/curl.go‎

Lines changed: 89 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -146,34 +146,107 @@ func (curlCmd *CurlCommand) GetServerDetails() (*config.ServerDetails, error) {
146146
// Use this method ONLY after removing all JFrog-CLI flags, i.e. flags in the form: '--my-flag=value' are not allowed.
147147
// An argument is any provided candidate which is not a flag or a flag value.
148148
func (curlCmd *CurlCommand) findUriValueAndIndex() (int, string) {
149-
skipThisArg := false
149+
// curlBooleanFlags is a set of curl flags that do NOT take a value.
150+
curlBooleanFlags := map[string]struct{}{
151+
"-#": {}, "-:": {}, "-0": {}, "-1": {}, "-2": {}, "-3": {}, "-4": {}, "-6": {},
152+
"-a": {}, "-B": {}, "-f": {}, "-g": {}, "-G": {}, "-I": {}, "-i": {},
153+
"-j": {}, "-J": {}, "-k": {}, "-l": {}, "-L": {}, "-M": {}, "-n": {},
154+
"-N": {}, "-O": {}, "-p": {}, "-q": {}, "-R": {}, "-s": {}, "-S": {},
155+
"-v": {}, "-V": {}, "-Z": {},
156+
"--anyauth": {}, "--append": {}, "--basic": {}, "--ca-native": {},
157+
"--cert-status": {}, "--compressed": {}, "--compressed-ssh": {},
158+
"--create-dirs": {}, "--crlf": {}, "--digest": {}, "--disable": {},
159+
"--disable-eprt": {}, "--disable-epsv": {}, "--disallow-username-in-url": {},
160+
"--doh-cert-status": {}, "--doh-insecure": {}, "--fail": {},
161+
"--fail-early": {}, "--fail-with-body": {}, "--false-start": {},
162+
"--form-escape": {}, "--ftp-create-dirs": {}, "--ftp-pasv": {},
163+
"--ftp-pret": {}, "--ftp-skip-pasv-ip": {}, "--ftp-ssl-ccc": {},
164+
"--ftp-ssl-control": {}, "--get": {}, "--globoff": {},
165+
"--haproxy-protocol": {}, "--head": {}, "--http0.9": {}, "--http1.0": {},
166+
"--http1.1": {}, "--http2": {}, "--http2-prior-knowledge": {},
167+
"--http3": {}, "--http3-only": {}, "--ignore-content-length": {},
168+
"--include": {}, "--insecure": {}, "--ipv4": {}, "--ipv6": {},
169+
"--junk-session-cookies": {}, "--list-only": {}, "--location": {},
170+
"--location-trusted": {}, "--mail-rcpt-allowfails": {}, "--manual": {},
171+
"--metalink": {}, "--negotiate": {}, "--netrc": {}, "--netrc-optional": {},
172+
"--next": {}, "--no-alpn": {}, "--no-buffer": {}, "--no-clobber": {},
173+
"--no-keepalive": {}, "--no-npn": {}, "--no-progress-meter": {},
174+
"--no-sessionid": {}, "--ntlm": {}, "--ntlm-wb": {}, "--parallel": {},
175+
"--parallel-immediate": {}, "--path-as-is": {}, "--post301": {},
176+
"--post302": {}, "--post303": {}, "--progress-bar": {},
177+
"--proxy-anyauth": {}, "--proxy-basic": {}, "--proxy-ca-native": {},
178+
"--proxy-digest": {}, "--proxy-http2": {}, "--proxy-insecure": {},
179+
"--proxy-negotiate": {}, "--proxy-ntlm": {}, "--proxy-ssl-allow-beast": {},
180+
"--proxy-ssl-auto-client-cert": {}, "--proxy-tlsv1": {}, "--proxytunnel": {},
181+
"--raw": {}, "--remote-header-name": {}, "--remote-name": {},
182+
"--remote-name-all": {}, "--remote-time": {}, "--remove-on-error": {},
183+
"--retry-all-errors": {}, "--retry-connrefused": {}, "--sasl-ir": {},
184+
"--show-error": {}, "--silent": {}, "--socks5-basic": {},
185+
"--socks5-gssapi": {}, "--socks5-gssapi-nec": {}, "--ssl": {},
186+
"--ssl-allow-beast": {}, "--ssl-auto-client-cert": {}, "--ssl-no-revoke": {},
187+
"--ssl-reqd": {}, "--ssl-revoke-best-effort": {}, "--sslv2": {},
188+
"--sslv3": {}, "--styled-output": {}, "--suppress-connect-headers": {},
189+
"--tcp-fastopen": {}, "--tcp-nodelay": {}, "--tftp-no-options": {},
190+
"--tlsv1": {}, "--tlsv1.0": {}, "--tlsv1.1": {}, "--tlsv1.2": {},
191+
"--tlsv1.3": {}, "--tr-encoding": {}, "--trace-ids": {},
192+
"--trace-time": {}, "--use-ascii": {}, "--verbose": {}, "--version": {},
193+
"--xattr": {},
194+
}
195+
196+
// isBooleanFlag checks if a flag is in the boolean flags
197+
isBooleanFlag := func(flag string) bool {
198+
_, exists := curlBooleanFlags[flag]
199+
return exists
200+
}
201+
202+
skipNextArg := false
150203
for index, arg := range curlCmd.arguments {
151-
// Check if shouldn't check current arg.
152-
if skipThisArg {
153-
skipThisArg = false
204+
// Check if this arg should be skipped (it's a value for the previous flag)
205+
if skipNextArg {
206+
skipNextArg = false
154207
continue
155208
}
156-
// If starts with '--', meaning a flag which its value is at next slot.
157-
if strings.HasPrefix(arg, "--") {
158-
skipThisArg = true
159-
continue
160-
}
161-
// Check if '-'.
209+
210+
// Check if this is a flag
162211
if strings.HasPrefix(arg, "-") {
163-
if len(arg) > 2 {
164-
// Meaning that this flag also contains its value.
212+
// Check for flags with inline values like --header=value
213+
if strings.HasPrefix(arg, "--") && strings.Contains(arg, "=") {
165214
continue
166215
}
167-
// If reached here, means that the flag value is at the next arg.
168-
skipThisArg = true
216+
217+
// Check if it a boolean flag
218+
if isBooleanFlag(arg) {
219+
continue
220+
}
221+
222+
// For short flags
223+
if !strings.HasPrefix(arg, "--") && len(arg) > 2 {
224+
// find a flag that takes a value, everything after it is the inline value
225+
for i := 1; i < len(arg); i++ {
226+
charFlag := "-" + string(arg[i])
227+
if !isBooleanFlag(charFlag) {
228+
// Found a flag that takes a value
229+
if i < len(arg)-1 {
230+
// Inline value exists (e.g., -XGET, -Lotest.txt)
231+
break
232+
}
233+
// No inline value (e.g., -Lo, -sX), next arg is the value
234+
skipNextArg = true
235+
break
236+
}
237+
}
238+
continue
239+
}
240+
241+
// Flag takes a value in the next argument
242+
skipNextArg = true
169243
continue
170244
}
171245

172-
// Found an argument
246+
// Found a non-flag argument - this is the URL/API path
173247
return index, arg
174248
}
175249

176-
// If reached here, didn't find an argument.
177250
return -1, ""
178251
}
179252

‎common/commands/curl_test.go‎

Lines changed: 248 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,31 @@ func TestBuildCommandUrl(t *testing.T) {
6868
uriValue string
6969
expectErr bool
7070
}{
71-
{"test1", []string{"-X", "GET", "/api/build/test1", "--server-id", "test1", "--foo", "bar"}, 2, "https://artifactory:8081/artifactory/api/build/test1", false},
72-
{"test2", []string{"-X", "GET", "/api/build/test2", "--server-idea", "foo", "--server-id=test2"}, 2, "https://artifactory:8081/artifactory/api/build/test2", false},
73-
{"test3", []string{"-XGET", "--/api/build/test3", "--server-id="}, 1, "https://artifactory:8081/artifactory/api/build/test3", true},
74-
{"test4", []string{"-XGET", "-Test4", "--server-id", "bar"}, 1, "https://artifactory:8081/artifactory/api/build/test4", true},
75-
{"test5", []string{"-X", "GET", "api/build/test5", "--server-id", "test5", "--foo", "bar"}, 2, "https://artifactory:8081/artifactory/api/build/test5", false},
71+
{"basicGetWithLeadingSlash", []string{"-X", "GET", "/api/build/test1", "--server-id", "test1", "--foo", "bar"}, 2, "https://artifactory:8081/artifactory/api/build/test1", false},
72+
{"getWithInlineServerId", []string{"-X", "GET", "/api/build/test2", "--server-idea", "foo", "--server-id=test2"}, 2, "https://artifactory:8081/artifactory/api/build/test2", false},
73+
{"uriStartsWithDashDash", []string{"-XGET", "--/api/build/test3", "--server-id="}, 1, "https://artifactory:8081/artifactory/api/build/test3", true},
74+
{"uriStartsWithDash", []string{"-XGET", "-Test4", "--server-id", "bar"}, 1, "https://artifactory:8081/artifactory/api/build/test4", true},
75+
{"basicGetWithoutLeadingSlash", []string{"-X", "GET", "api/build/test5", "--server-id", "test5", "--foo", "bar"}, 2, "https://artifactory:8081/artifactory/api/build/test5", false},
76+
{"fullHTTP", []string{"-L", "http://example.com/api/test"}, -1, "", true},
77+
{"fullHTTPS", []string{"-L", "https://example.com/api/test"}, -1, "", true},
78+
{"noURI", []string{"-X", "GET", "-H", "Auth: token"}, -1, "", true},
79+
{"onlyFlags", []string{"-L", "-v", "-s", "--insecure"}, -1, "", true},
80+
{"specialChars", []string{"-X", "GET", "/api/test?param=value&foo=bar"}, 2, "https://artifactory:8081/artifactory/api/test?param=value&foo=bar", false},
81+
{"uriWithSpaces", []string{"-X", "GET", "/api/test%20space"}, 2, "https://artifactory:8081/artifactory/api/test%20space", false},
82+
{"multipleURIs", []string{"api/first", "-X", "GET", "api/second"}, 0, "https://artifactory:8081/artifactory/api/first", false},
83+
{"dashInURI", []string{"-X", "GET", "-not-a-flag/api/test"}, -1, "", true},
84+
{"booleanFlags", []string{"-L", "-k", "-v", "-s", "api/test"}, 4, "https://artifactory:8081/artifactory/api/test", false},
85+
{"longBooleanFlags", []string{"--location", "--insecure", "--verbose", "api/test"}, 3, "https://artifactory:8081/artifactory/api/test", false},
86+
{"embeddedValue", []string{"--header=Authorization: Bearer token", "-XPOST", "api/test"}, 2, "https://artifactory:8081/artifactory/api/test", false},
87+
{"multipleEmbedded", []string{"--data=json", "--header=Content-Type: application/json", "api/test"}, 2, "https://artifactory:8081/artifactory/api/test", false},
88+
{"combinedBoolean", []string{"-Lkvs", "api/test"}, 1, "https://artifactory:8081/artifactory/api/test", false},
89+
{"combinedMixed", []string{"-LvX", "POST", "api/test"}, 2, "https://artifactory:8081/artifactory/api/test", false},
90+
{"trailingValueFlag", []string{"api/test", "-X"}, 0, "https://artifactory:8081/artifactory/api/test", false},
91+
{"trailingBooleanFlag", []string{"api/test", "-L"}, 0, "https://artifactory:8081/artifactory/api/test", false},
92+
{"emptyArgs", []string{"-X", "GET", "", "api/test"}, 2, "https://artifactory:8081/artifactory/", false},
93+
{"emptyFlagValue", []string{"-H", "", "api/test"}, 2, "https://artifactory:8081/artifactory/api/test", false},
94+
{"uriFirst", []string{"api/test", "-X", "GET", "-L"}, 0, "https://artifactory:8081/artifactory/api/test", false},
95+
{"realWorld", []string{"-sS", "-L", "-X", "POST", "-H", "Content-Type: application/json", "-d", `{"key":"value"}`, "--insecure", "api/repos/test"}, 9, "https://artifactory:8081/artifactory/api/repos/test", false},
7696
}
7797

7898
command := &CurlCommand{}
@@ -102,3 +122,226 @@ func TestBuildCommandUrl(t *testing.T) {
102122
})
103123
}
104124
}
125+
126+
func TestFindUriWithBooleanFlags(t *testing.T) {
127+
tests := []struct {
128+
name string
129+
arguments []string
130+
expectedUriIndex int
131+
expectedUri string
132+
}{
133+
{
134+
name: "shortSilentWithLongVerbose",
135+
arguments: []string{"-s", "--show-error", "api/repositories/dev-master-maven-local", "--verbose"},
136+
expectedUriIndex: 2,
137+
expectedUri: "api/repositories/dev-master-maven-local",
138+
},
139+
{
140+
name: "outputFlagWithCombinedVerbose",
141+
arguments: []string{"-o", "helm.tar.gz", "-L", "-vvv", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
142+
expectedUriIndex: 4,
143+
expectedUri: "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
144+
},
145+
{
146+
name: "combinedVerboseBeforeLocation",
147+
arguments: []string{"-o", "helm.tar.gz", "-vvv", "-L", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
148+
expectedUriIndex: 4,
149+
expectedUri: "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
150+
},
151+
{
152+
name: "outputWithLocationAndSilent",
153+
arguments: []string{"-o", "helm.tar.gz", "-L", "-s", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
154+
expectedUriIndex: 4,
155+
expectedUri: "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
156+
},
157+
{
158+
name: "locationFirstThenOutput",
159+
arguments: []string{"-L", "-o", "helm.tar.gz", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
160+
expectedUriIndex: 3,
161+
expectedUri: "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
162+
},
163+
{
164+
name: "outputThenLocationSimple",
165+
arguments: []string{"-o", "helm.tar.gz", "-L", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
166+
expectedUriIndex: 3,
167+
expectedUri: "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
168+
},
169+
{
170+
name: "locationCombinedVerboseThenOutput",
171+
arguments: []string{"-L", "-vvv", "-o", "helm.tar.gz", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
172+
expectedUriIndex: 4,
173+
expectedUri: "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
174+
},
175+
{
176+
name: "locationOutputThenCombinedVerbose",
177+
arguments: []string{"-L", "-o", "helm.tar.gz", "-vvv", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
178+
expectedUriIndex: 4,
179+
expectedUri: "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
180+
},
181+
{
182+
name: "combinedSilentShowErrorWithLocation",
183+
arguments: []string{"-sS", "-L", "api/system/ping"},
184+
expectedUriIndex: 2,
185+
expectedUri: "api/system/ping",
186+
},
187+
{
188+
name: "longFormSilentShowErrorLocation",
189+
arguments: []string{"--silent", "--show-error", "--location", "api/system/ping"},
190+
expectedUriIndex: 3,
191+
expectedUri: "api/system/ping",
192+
},
193+
{
194+
name: "getWithHeaderAndBooleanFlags",
195+
arguments: []string{"-X", "GET", "-H", "Content-Type: application/json", "--verbose", "--insecure", "api/repositories"},
196+
expectedUriIndex: 6,
197+
expectedUri: "api/repositories",
198+
},
199+
{
200+
name: "inlineRequestAndHeaderWithLocation",
201+
arguments: []string{"-XPOST", "-HContent-Type:application/json", "-L", "api/repositories"},
202+
expectedUriIndex: 3,
203+
expectedUri: "api/repositories",
204+
},
205+
{
206+
name: "longFormInlineRequestAndHeader",
207+
arguments: []string{"--request=GET", "--header=Accept:application/json", "-v", "api/system/ping"},
208+
expectedUriIndex: 3,
209+
expectedUri: "api/system/ping",
210+
},
211+
{
212+
name: "allShortBooleanFlags",
213+
arguments: []string{"-#", "-0", "-1", "-2", "-3", "-4", "-6", "-a", "-B", "-f", "-g", "-G", "-I", "-i", "api/test"},
214+
expectedUriIndex: 14,
215+
expectedUri: "api/test",
216+
},
217+
{
218+
name: "mixedKnownUnknownFlags",
219+
arguments: []string{"-L", "-9", "possibleValue", "api/test"},
220+
expectedUriIndex: 3,
221+
expectedUri: "api/test",
222+
},
223+
{
224+
name: "complexCombinedFlags",
225+
arguments: []string{"-sLkvo", "output.txt", "api/test"},
226+
expectedUriIndex: 2,
227+
expectedUri: "api/test",
228+
},
229+
{
230+
name: "httpMethodsAsValues",
231+
arguments: []string{"-X", "PATCH", "-X", "OPTIONS", "-X", "TRACE", "api/test"},
232+
expectedUriIndex: 6,
233+
expectedUri: "api/test",
234+
},
235+
{
236+
name: "quotedValues",
237+
arguments: []string{"-H", `Authorization: "Bearer token"`, "-H", "Accept: */*", "api/test"},
238+
expectedUriIndex: 4,
239+
expectedUri: "api/test",
240+
},
241+
{
242+
name: "jsonData",
243+
arguments: []string{"-d", `{"test": "value", "array": [1,2,3]}`, "-H", "Content-Type: application/json", "api/test"},
244+
expectedUriIndex: 4,
245+
expectedUri: "api/test",
246+
},
247+
{
248+
name: "fileReference",
249+
arguments: []string{"-d", "@/path/to/file.json", "-T", "/path/to/upload.tar", "api/test"},
250+
expectedUriIndex: 4,
251+
expectedUri: "api/test",
252+
},
253+
{
254+
name: "repeatedBooleanFlags",
255+
arguments: []string{"-v", "-v", "-v", "-L", "-L", "api/test"},
256+
expectedUriIndex: 5,
257+
expectedUri: "api/test",
258+
},
259+
{
260+
name: "urlEncodedData",
261+
arguments: []string{"--data-urlencode", "param=value&other=test", "api/test"},
262+
expectedUriIndex: 2,
263+
expectedUri: "api/test",
264+
},
265+
{
266+
name: "proxySettings",
267+
arguments: []string{"-x", "proxy.server:8080", "-U", "proxyuser:pass", "api/test"},
268+
expectedUriIndex: 4,
269+
expectedUri: "api/test",
270+
},
271+
{
272+
name: "certAndKeyFlags",
273+
arguments: []string{"-E", "/path/to/cert.pem", "--key", "/path/to/key.pem", "api/test"},
274+
expectedUriIndex: 4,
275+
expectedUri: "api/test",
276+
},
277+
{
278+
name: "rangeHeader",
279+
arguments: []string{"-r", "0-1023", "-C", "-", "api/download/file"},
280+
expectedUriIndex: 4,
281+
expectedUri: "api/download/file",
282+
},
283+
{
284+
name: "userAgentAndReferer",
285+
arguments: []string{"-A", "CustomUserAgent/1.0", "-e", "https://referrer.com", "api/test"},
286+
expectedUriIndex: 4,
287+
expectedUri: "api/test",
288+
},
289+
{
290+
name: "formData",
291+
arguments: []string{"-F", "field1=value1", "-F", "[email protected]", "-F", "field3=<file2.txt", "api/upload"},
292+
expectedUriIndex: 6,
293+
expectedUri: "api/upload",
294+
},
295+
{
296+
name: "cookiesAndJar",
297+
arguments: []string{"-b", "cookies.txt", "-c", "newcookies.txt", "-b", "name=value", "api/test"},
298+
expectedUriIndex: 6,
299+
expectedUri: "api/test",
300+
},
301+
{
302+
name: "speedAndTime",
303+
arguments: []string{"-Y", "1000", "-y", "30", "-m", "120", "api/test"},
304+
expectedUriIndex: 6,
305+
expectedUri: "api/test",
306+
},
307+
{
308+
name: "retryOptions",
309+
arguments: []string{"--retry", "3", "--retry-delay", "5", "--retry-max-time", "60", "api/test"},
310+
expectedUriIndex: 6,
311+
expectedUri: "api/test",
312+
},
313+
{
314+
name: "writeOutFormat",
315+
arguments: []string{"-w", `%{http_code}\n`, "-o", "/dev/null", "-s", "api/test"},
316+
expectedUriIndex: 5,
317+
expectedUri: "api/test",
318+
},
319+
{
320+
name: "ipv6Address",
321+
arguments: []string{"-H", "Host: [::1]", "-6", "api/test"},
322+
expectedUriIndex: 3,
323+
expectedUri: "api/test",
324+
},
325+
{
326+
name: "traceAndDump",
327+
arguments: []string{"--trace", "trace.txt", "--trace-ascii", "ascii.txt", "--dump-header", "headers.txt", "api/test"},
328+
expectedUriIndex: 6,
329+
expectedUri: "api/test",
330+
},
331+
}
332+
333+
command := &CurlCommand{}
334+
for _, test := range tests {
335+
t.Run(test.name, func(t *testing.T) {
336+
command.arguments = test.arguments
337+
actualIndex, actualUri := command.findUriValueAndIndex()
338+
339+
if actualIndex != test.expectedUriIndex {
340+
t.Errorf("Expected URI index: %d, got: %d. Arguments: %v", test.expectedUriIndex, actualIndex, test.arguments)
341+
}
342+
if actualUri != test.expectedUri {
343+
t.Errorf("Expected URI: %s, got: %s. Arguments: %v", test.expectedUri, actualUri, test.arguments)
344+
}
345+
})
346+
}
347+
}

0 commit comments

Comments
 (0)