Fix curl argument parsing

common/commands/curl.go

@@ -146,34 +146,107 @@ func (curlCmd *CurlCommand) GetServerDetails() (*config.ServerDetails, error) {
 // Use this method ONLY after removing all JFrog-CLI flags, i.e. flags in the form: '--my-flag=value' are not allowed.
 // An argument is any provided candidate which is not a flag or a flag value.
 func (curlCmd *CurlCommand) findUriValueAndIndex() (int, string) {
-	skipThisArg := false
+	// curlBooleanFlags is a set of curl flags that do NOT take a value.
+	curlBooleanFlags := map[string]struct{}{
+		"-#": {}, "-:": {}, "-0": {}, "-1": {}, "-2": {}, "-3": {}, "-4": {}, "-6": {},
+		"-a": {}, "-B": {}, "-f": {}, "-g": {}, "-G": {}, "-I": {}, "-i": {},
+		"-j": {}, "-J": {}, "-k": {}, "-l": {}, "-L": {}, "-M": {}, "-n": {},
+		"-N": {}, "-O": {}, "-p": {}, "-q": {}, "-R": {}, "-s": {}, "-S": {},
+		"-v": {}, "-V": {}, "-Z": {},
+		"--anyauth": {}, "--append": {}, "--basic": {}, "--ca-native": {},
+		"--cert-status": {}, "--compressed": {}, "--compressed-ssh": {},
+		"--create-dirs": {}, "--crlf": {}, "--digest": {}, "--disable": {},
+		"--disable-eprt": {}, "--disable-epsv": {}, "--disallow-username-in-url": {},
+		"--doh-cert-status": {}, "--doh-insecure": {}, "--fail": {},
+		"--fail-early": {}, "--fail-with-body": {}, "--false-start": {},
+		"--form-escape": {}, "--ftp-create-dirs": {}, "--ftp-pasv": {},
+		"--ftp-pret": {}, "--ftp-skip-pasv-ip": {}, "--ftp-ssl-ccc": {},
+		"--ftp-ssl-control": {}, "--get": {}, "--globoff": {},
+		"--haproxy-protocol": {}, "--head": {}, "--http0.9": {}, "--http1.0": {},
+		"--http1.1": {}, "--http2": {}, "--http2-prior-knowledge": {},
+		"--http3": {}, "--http3-only": {}, "--ignore-content-length": {},
+		"--include": {}, "--insecure": {}, "--ipv4": {}, "--ipv6": {},
+		"--junk-session-cookies": {}, "--list-only": {}, "--location": {},
+		"--location-trusted": {}, "--mail-rcpt-allowfails": {}, "--manual": {},
+		"--metalink": {}, "--negotiate": {}, "--netrc": {}, "--netrc-optional": {},
+		"--next": {}, "--no-alpn": {}, "--no-buffer": {}, "--no-clobber": {},
+		"--no-keepalive": {}, "--no-npn": {}, "--no-progress-meter": {},
+		"--no-sessionid": {}, "--ntlm": {}, "--ntlm-wb": {}, "--parallel": {},
+		"--parallel-immediate": {}, "--path-as-is": {}, "--post301": {},
+		"--post302": {}, "--post303": {}, "--progress-bar": {},
+		"--proxy-anyauth": {}, "--proxy-basic": {}, "--proxy-ca-native": {},
+		"--proxy-digest": {}, "--proxy-http2": {}, "--proxy-insecure": {},
+		"--proxy-negotiate": {}, "--proxy-ntlm": {}, "--proxy-ssl-allow-beast": {},
+		"--proxy-ssl-auto-client-cert": {}, "--proxy-tlsv1": {}, "--proxytunnel": {},
+		"--raw": {}, "--remote-header-name": {}, "--remote-name": {},
+		"--remote-name-all": {}, "--remote-time": {}, "--remove-on-error": {},
+		"--retry-all-errors": {}, "--retry-connrefused": {}, "--sasl-ir": {},
+		"--show-error": {}, "--silent": {}, "--socks5-basic": {},
+		"--socks5-gssapi": {}, "--socks5-gssapi-nec": {}, "--ssl": {},
+		"--ssl-allow-beast": {}, "--ssl-auto-client-cert": {}, "--ssl-no-revoke": {},
+		"--ssl-reqd": {}, "--ssl-revoke-best-effort": {}, "--sslv2": {},
+		"--sslv3": {}, "--styled-output": {}, "--suppress-connect-headers": {},
+		"--tcp-fastopen": {}, "--tcp-nodelay": {}, "--tftp-no-options": {},
+		"--tlsv1": {}, "--tlsv1.0": {}, "--tlsv1.1": {}, "--tlsv1.2": {},
+		"--tlsv1.3": {}, "--tr-encoding": {}, "--trace-ids": {},
+		"--trace-time": {}, "--use-ascii": {}, "--verbose": {}, "--version": {},
+		"--xattr": {},
+	}
+
+	// isBooleanFlag checks if a flag is in the boolean flags
+	isBooleanFlag := func(flag string) bool {
+		_, exists := curlBooleanFlags[flag]
+		return exists
+	}
+
+	skipNextArg := false
 	for index, arg := range curlCmd.arguments {
-		// Check if shouldn't check current arg.
-		if skipThisArg {
-			skipThisArg = false
+		// Check if this arg should be skipped (it's a value for the previous flag)
+		if skipNextArg {
+			skipNextArg = false
 			continue
 		}
-		// If starts with '--', meaning a flag which its value is at next slot.
-		if strings.HasPrefix(arg, "--") {
-			skipThisArg = true
-			continue
-		}
-		// Check if '-'.
+
+		// Check if this is a flag
 		if strings.HasPrefix(arg, "-") {
-			if len(arg) > 2 {
-				// Meaning that this flag also contains its value.
+			// Check for flags with inline values like --header=value
+			if strings.HasPrefix(arg, "--") && strings.Contains(arg, "=") {
 				continue
 			}
-			// If reached here, means that the flag value is at the next arg.
-			skipThisArg = true
+
+			// Check if it a boolean flag
+			if isBooleanFlag(arg) {
+				continue
+			}
+
+			// For short flags
+			if !strings.HasPrefix(arg, "--") && len(arg) > 2 {
+				// find a flag that takes a value, everything after it is the inline value
+				for i := 1; i < len(arg); i++ {
+					charFlag := "-" + string(arg[i])
+					if !isBooleanFlag(charFlag) {
+						// Found a flag that takes a value
+						if i < len(arg)-1 {
+							// Inline value exists (e.g., -XGET, -Lotest.txt)
+							break
+						}
+						// No inline value (e.g., -Lo, -sX), next arg is the value
+						skipNextArg = true
+						break
+					}
+				}
+				continue
+			}
+
+			// Flag takes a value in the next argument
+			skipNextArg = true
 			continue
 		}
 
-		// Found an argument
+		// Found a non-flag argument - this is the URL/API path
 		return index, arg
 	}
 
-	// If reached here, didn't find an argument.
 	return -1, ""
 }
 

common/commands/curl_test.go

@@ -68,11 +68,31 @@ func TestBuildCommandUrl(t *testing.T) {
 		uriValue  string
 		expectErr bool
 	}{
-		{"test1", []string{"-X", "GET", "/api/build/test1", "--server-id", "test1", "--foo", "bar"}, 2, "https://artifactory:8081/artifactory/api/build/test1", false},
-		{"test2", []string{"-X", "GET", "/api/build/test2", "--server-idea", "foo", "--server-id=test2"}, 2, "https://artifactory:8081/artifactory/api/build/test2", false},
-		{"test3", []string{"-XGET", "--/api/build/test3", "--server-id="}, 1, "https://artifactory:8081/artifactory/api/build/test3", true},
-		{"test4", []string{"-XGET", "-Test4", "--server-id", "bar"}, 1, "https://artifactory:8081/artifactory/api/build/test4", true},
-		{"test5", []string{"-X", "GET", "api/build/test5", "--server-id", "test5", "--foo", "bar"}, 2, "https://artifactory:8081/artifactory/api/build/test5", false},
+		{"basicGetWithLeadingSlash", []string{"-X", "GET", "/api/build/test1", "--server-id", "test1", "--foo", "bar"}, 2, "https://artifactory:8081/artifactory/api/build/test1", false},
+		{"getWithInlineServerId", []string{"-X", "GET", "/api/build/test2", "--server-idea", "foo", "--server-id=test2"}, 2, "https://artifactory:8081/artifactory/api/build/test2", false},
+		{"uriStartsWithDashDash", []string{"-XGET", "--/api/build/test3", "--server-id="}, 1, "https://artifactory:8081/artifactory/api/build/test3", true},
+		{"uriStartsWithDash", []string{"-XGET", "-Test4", "--server-id", "bar"}, 1, "https://artifactory:8081/artifactory/api/build/test4", true},
+		{"basicGetWithoutLeadingSlash", []string{"-X", "GET", "api/build/test5", "--server-id", "test5", "--foo", "bar"}, 2, "https://artifactory:8081/artifactory/api/build/test5", false},
+		{"fullHTTP", []string{"-L", "http://example.com/api/test"}, -1, "", true},
+		{"fullHTTPS", []string{"-L", "https://example.com/api/test"}, -1, "", true},
+		{"noURI", []string{"-X", "GET", "-H", "Auth: token"}, -1, "", true},
+		{"onlyFlags", []string{"-L", "-v", "-s", "--insecure"}, -1, "", true},
+		{"specialChars", []string{"-X", "GET", "/api/test?param=value&foo=bar"}, 2, "https://artifactory:8081/artifactory/api/test?param=value&foo=bar", false},
+		{"uriWithSpaces", []string{"-X", "GET", "/api/test%20space"}, 2, "https://artifactory:8081/artifactory/api/test%20space", false},
+		{"multipleURIs", []string{"api/first", "-X", "GET", "api/second"}, 0, "https://artifactory:8081/artifactory/api/first", false},
+		{"dashInURI", []string{"-X", "GET", "-not-a-flag/api/test"}, -1, "", true},
+		{"booleanFlags", []string{"-L", "-k", "-v", "-s", "api/test"}, 4, "https://artifactory:8081/artifactory/api/test", false},
+		{"longBooleanFlags", []string{"--location", "--insecure", "--verbose", "api/test"}, 3, "https://artifactory:8081/artifactory/api/test", false},
+		{"embeddedValue", []string{"--header=Authorization: Bearer token", "-XPOST", "api/test"}, 2, "https://artifactory:8081/artifactory/api/test", false},
+		{"multipleEmbedded", []string{"--data=json", "--header=Content-Type: application/json", "api/test"}, 2, "https://artifactory:8081/artifactory/api/test", false},
+		{"combinedBoolean", []string{"-Lkvs", "api/test"}, 1, "https://artifactory:8081/artifactory/api/test", false},
+		{"combinedMixed", []string{"-LvX", "POST", "api/test"}, 2, "https://artifactory:8081/artifactory/api/test", false},
+		{"trailingValueFlag", []string{"api/test", "-X"}, 0, "https://artifactory:8081/artifactory/api/test", false},
+		{"trailingBooleanFlag", []string{"api/test", "-L"}, 0, "https://artifactory:8081/artifactory/api/test", false},
+		{"emptyArgs", []string{"-X", "GET", "", "api/test"}, 2, "https://artifactory:8081/artifactory/", false},
+		{"emptyFlagValue", []string{"-H", "", "api/test"}, 2, "https://artifactory:8081/artifactory/api/test", false},
+		{"uriFirst", []string{"api/test", "-X", "GET", "-L"}, 0, "https://artifactory:8081/artifactory/api/test", false},
+		{"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},
 	}
 
 	command := &CurlCommand{}
@@ -102,3 +122,226 @@ func TestBuildCommandUrl(t *testing.T) {
 		})
 	}
 }
+
+func TestFindUriWithBooleanFlags(t *testing.T) {
+	tests := []struct {
+		name             string
+		arguments        []string
+		expectedUriIndex int
+		expectedUri      string
+	}{
+		{
+			name:             "shortSilentWithLongVerbose",
+			arguments:        []string{"-s", "--show-error", "api/repositories/dev-master-maven-local", "--verbose"},
+			expectedUriIndex: 2,
+			expectedUri:      "api/repositories/dev-master-maven-local",
+		},
+		{
+			name:             "outputFlagWithCombinedVerbose",
+			arguments:        []string{"-o", "helm.tar.gz", "-L", "-vvv", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
+			expectedUriIndex: 4,
+			expectedUri:      "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
+		},
+		{
+			name:             "combinedVerboseBeforeLocation",
+			arguments:        []string{"-o", "helm.tar.gz", "-vvv", "-L", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
+			expectedUriIndex: 4,
+			expectedUri:      "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
+		},
+		{
+			name:             "outputWithLocationAndSilent",
+			arguments:        []string{"-o", "helm.tar.gz", "-L", "-s", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
+			expectedUriIndex: 4,
+			expectedUri:      "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
+		},
+		{
+			name:             "locationFirstThenOutput",
+			arguments:        []string{"-L", "-o", "helm.tar.gz", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
+			expectedUriIndex: 3,
+			expectedUri:      "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
+		},
+		{
+			name:             "outputThenLocationSimple",
+			arguments:        []string{"-o", "helm.tar.gz", "-L", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
+			expectedUriIndex: 3,
+			expectedUri:      "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
+		},
+		{
+			name:             "locationCombinedVerboseThenOutput",
+			arguments:        []string{"-L", "-vvv", "-o", "helm.tar.gz", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
+			expectedUriIndex: 4,
+			expectedUri:      "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
+		},
+		{
+			name:             "locationOutputThenCombinedVerbose",
+			arguments:        []string{"-L", "-o", "helm.tar.gz", "-vvv", "helm-sh/helm-v3.19.0-linux-amd64.tar.gz"},
+			expectedUriIndex: 4,
+			expectedUri:      "helm-sh/helm-v3.19.0-linux-amd64.tar.gz",
+		},
+		{
+			name:             "combinedSilentShowErrorWithLocation",
+			arguments:        []string{"-sS", "-L", "api/system/ping"},
+			expectedUriIndex: 2,
+			expectedUri:      "api/system/ping",
+		},
+		{
+			name:             "longFormSilentShowErrorLocation",
+			arguments:        []string{"--silent", "--show-error", "--location", "api/system/ping"},
+			expectedUriIndex: 3,
+			expectedUri:      "api/system/ping",
+		},
+		{
+			name:             "getWithHeaderAndBooleanFlags",
+			arguments:        []string{"-X", "GET", "-H", "Content-Type: application/json", "--verbose", "--insecure", "api/repositories"},
+			expectedUriIndex: 6,
+			expectedUri:      "api/repositories",
+		},
+		{
+			name:             "inlineRequestAndHeaderWithLocation",
+			arguments:        []string{"-XPOST", "-HContent-Type:application/json", "-L", "api/repositories"},
+			expectedUriIndex: 3,
+			expectedUri:      "api/repositories",
+		},
+		{
+			name:             "longFormInlineRequestAndHeader",
+			arguments:        []string{"--request=GET", "--header=Accept:application/json", "-v", "api/system/ping"},
+			expectedUriIndex: 3,
+			expectedUri:      "api/system/ping",
+		},
+		{
+			name:             "allShortBooleanFlags",
+			arguments:        []string{"-#", "-0", "-1", "-2", "-3", "-4", "-6", "-a", "-B", "-f", "-g", "-G", "-I", "-i", "api/test"},
+			expectedUriIndex: 14,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "mixedKnownUnknownFlags",
+			arguments:        []string{"-L", "-9", "possibleValue", "api/test"},
+			expectedUriIndex: 3,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "complexCombinedFlags",
+			arguments:        []string{"-sLkvo", "output.txt", "api/test"},
+			expectedUriIndex: 2,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "httpMethodsAsValues",
+			arguments:        []string{"-X", "PATCH", "-X", "OPTIONS", "-X", "TRACE", "api/test"},
+			expectedUriIndex: 6,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "quotedValues",
+			arguments:        []string{"-H", `Authorization: "Bearer token"`, "-H", "Accept: */*", "api/test"},
+			expectedUriIndex: 4,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "jsonData",
+			arguments:        []string{"-d", `{"test": "value", "array": [1,2,3]}`, "-H", "Content-Type: application/json", "api/test"},
+			expectedUriIndex: 4,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "fileReference",
+			arguments:        []string{"-d", "@/path/to/file.json", "-T", "/path/to/upload.tar", "api/test"},
+			expectedUriIndex: 4,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "repeatedBooleanFlags",
+			arguments:        []string{"-v", "-v", "-v", "-L", "-L", "api/test"},
+			expectedUriIndex: 5,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "urlEncodedData",
+			arguments:        []string{"--data-urlencode", "param=value&other=test", "api/test"},
+			expectedUriIndex: 2,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "proxySettings",
+			arguments:        []string{"-x", "proxy.server:8080", "-U", "proxyuser:pass", "api/test"},
+			expectedUriIndex: 4,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "certAndKeyFlags",
+			arguments:        []string{"-E", "/path/to/cert.pem", "--key", "/path/to/key.pem", "api/test"},
+			expectedUriIndex: 4,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "rangeHeader",
+			arguments:        []string{"-r", "0-1023", "-C", "-", "api/download/file"},
+			expectedUriIndex: 4,
+			expectedUri:      "api/download/file",
+		},
+		{
+			name:             "userAgentAndReferer",
+			arguments:        []string{"-A", "CustomUserAgent/1.0", "-e", "https://referrer.com", "api/test"},
+			expectedUriIndex: 4,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "formData",
+			arguments:        []string{"-F", "field1=value1", "-F", "field2=@file.txt", "-F", "field3=<file2.txt", "api/upload"},
+			expectedUriIndex: 6,
+			expectedUri:      "api/upload",
+		},
+		{
+			name:             "cookiesAndJar",
+			arguments:        []string{"-b", "cookies.txt", "-c", "newcookies.txt", "-b", "name=value", "api/test"},
+			expectedUriIndex: 6,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "speedAndTime",
+			arguments:        []string{"-Y", "1000", "-y", "30", "-m", "120", "api/test"},
+			expectedUriIndex: 6,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "retryOptions",
+			arguments:        []string{"--retry", "3", "--retry-delay", "5", "--retry-max-time", "60", "api/test"},
+			expectedUriIndex: 6,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "writeOutFormat",
+			arguments:        []string{"-w", `%{http_code}\n`, "-o", "/dev/null", "-s", "api/test"},
+			expectedUriIndex: 5,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "ipv6Address",
+			arguments:        []string{"-H", "Host: [::1]", "-6", "api/test"},
+			expectedUriIndex: 3,
+			expectedUri:      "api/test",
+		},
+		{
+			name:             "traceAndDump",
+			arguments:        []string{"--trace", "trace.txt", "--trace-ascii", "ascii.txt", "--dump-header", "headers.txt", "api/test"},
+			expectedUriIndex: 6,
+			expectedUri:      "api/test",
+		},
+	}
+
+	command := &CurlCommand{}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			command.arguments = test.arguments
+			actualIndex, actualUri := command.findUriValueAndIndex()
+
+			if actualIndex != test.expectedUriIndex {
+				t.Errorf("Expected URI index: %d, got: %d. Arguments: %v", test.expectedUriIndex, actualIndex, test.arguments)
+			}
+			if actualUri != test.expectedUri {
+				t.Errorf("Expected URI: %s, got: %s. Arguments: %v", test.expectedUri, actualUri, test.arguments)
+			}
+		})
+	}
+}