Skip to content

Commit 3fbb6b5

Browse files
committed
Merge remote-tracking branch 'upstream/main' into RTECO-1574
2 parents addae70 + bb92ab7 commit 3fbb6b5

13 files changed

Lines changed: 1795 additions & 25 deletions

build/gradle.go

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os/exec"
1111
"path/filepath"
1212
"regexp"
13+
"runtime"
1314
"strings"
1415

1516
"github.com/jfrog/build-info-go/utils"
@@ -260,41 +261,72 @@ func (config *gradleRunConfig) GetCmd() *exec.Cmd {
260261
}
261262
// Add BUILDINFO_PROPFILE system property if extractor properties file exists
262263
if config.extractorPropsFile != "" {
263-
jvmProp := fmt.Sprintf("-D%s=%s", extractorPropsDir, config.extractorPropsFile)
264-
if strings.Contains(config.extractorPropsFile, " ") {
265-
jvmProp = fmt.Sprintf("-D%s='%s'", extractorPropsDir, config.extractorPropsFile)
266-
}
267-
cmd = append(cmd, jvmProp)
264+
cmd = append(cmd, fmt.Sprintf("-D%s=%s", extractorPropsDir, config.extractorPropsFile))
268265
}
269-
cmd = append(cmd, formatCommandProperties(config.tasks)...)
270-
config.logger.Info("Running gradle command:", strings.Join(cmd, " "))
266+
cmd = append(cmd, stripPropertyQuotes(config.tasks)...)
267+
config.logger.Info("Running gradle command:", strings.Join(quoteArgsForLog(cmd), " "))
271268
return exec.Command(cmd[0], cmd[1:]...)
272269
}
273270

274-
func formatCommandProperties(tasks []string) []string {
275-
var cmdArgs []string
276-
for _, task := range tasks {
271+
// stripPropertyQuotes removes a matching pair of leading/trailing quote characters (' or ") from system/project
272+
// property values, e.g., -Dkey='val ue' => -Dkey=val ue.
273+
// This is only needed on Windows: cmd.exe and PowerShell don't strip single quotes from arguments the way
274+
// bash/zsh do, so a value quoted on the command line can still reach this process with the quotes literally
275+
// part of the string. Left as-is, they'd be passed straight through to Gradle, and end up uploaded to
276+
// Artifactory as part of the value. On other platforms the shell has already stripped shell-level quoting by
277+
// the time this process sees the argument, so any quotes still present were typed deliberately as part of the
278+
// value and must be left alone.
279+
func stripPropertyQuotes(tasks []string) []string {
280+
if runtime.GOOS != "windows" {
281+
return tasks
282+
}
283+
strippedTasks := make([]string, len(tasks))
284+
for i, task := range tasks {
277285
if isSystemOrProjectProperty(task) {
278-
task = quotePropertyIfNeeded(task)
286+
task = unquoteProperty(task)
279287
}
280-
cmdArgs = append(cmdArgs, task)
288+
strippedTasks[i] = task
289+
}
290+
return strippedTasks
291+
}
292+
293+
// Strips a matching pair of leading/trailing single or double quotes from a system or project property's value.
294+
func unquoteProperty(task string) string {
295+
parts := strings.SplitN(task, "=", 2)
296+
if len(parts) < 2 {
297+
return task
281298
}
282-
return cmdArgs
299+
return parts[0] + "=" + utils.StripSurroundingQuotes(parts[1])
300+
}
301+
302+
// quoteArgsForLog returns a copy of args with system/project property values wrapped in quotes, e.g.,
303+
// -Dkey=val ue => -Dkey='val ue', purely to make the printed command's property boundaries unambiguous.
304+
// This must never be used to build the actual arguments passed to exec.Command: those are executed directly,
305+
// without going through a shell, so added quote characters would be passed through literally instead of being
306+
// stripped, corrupting the property value.
307+
func quoteArgsForLog(args []string) []string {
308+
logArgs := make([]string, len(args))
309+
for i, arg := range args {
310+
if isSystemOrProjectProperty(arg) {
311+
arg = quoteProperty(arg)
312+
}
313+
logArgs[i] = arg
314+
}
315+
return logArgs
283316
}
284317

285318
func isSystemOrProjectProperty(task string) bool {
286319
hasPropertiesFlag := strings.HasPrefix(task, systemPropertiesFlag) || strings.HasPrefix(task, projectPropertiesFlag)
287320
return hasPropertiesFlag && strings.Contains(task, "=")
288321
}
289322

290-
// Wraps system or project property value in quotes if its value contain spaces, e.g., -Dkey=val ue => -Dkey='val ue'
291-
func quotePropertyIfNeeded(task string) string {
323+
// Wraps a system or project property's value in quotes, e.g., -Dkey=val ue => -Dkey='val ue'
324+
func quoteProperty(task string) string {
292325
parts := strings.SplitN(task, "=", 2)
293-
if strings.Contains(parts[1], " ") {
294-
return fmt.Sprintf(`%s='%s'`, parts[0], parts[1])
326+
if len(parts) < 2 {
327+
return task
295328
}
296-
297-
return task
329+
return fmt.Sprintf(`%s='%s'`, parts[0], parts[1])
298330
}
299331

300332
func (config *gradleRunConfig) runCmd(stdout, stderr io.Writer) error {

build/gradle_test.go

Lines changed: 89 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package build
33
import (
44
"fmt"
55
"path/filepath"
6+
"runtime"
67
"testing"
78

89
"github.com/jfrog/build-info-go/utils"
@@ -103,31 +104,113 @@ func TestParseGradleVersion(t *testing.T) {
103104
}
104105
}
105106

106-
func TestFormatCommandProperties(t *testing.T) {
107+
func TestQuoteArgsForLog(t *testing.T) {
107108
tests := []struct {
108109
input []string
109110
expected []string
110111
}{
111112
{
112113
input: []string{"clean", "-Dparam=value", "build", "-Pkey=value"},
113-
expected: []string{"clean", "-Dparam=value", "build", "-Pkey=value"},
114+
expected: []string{"clean", "-Dparam='value'", "build", "-Pkey='value'"},
114115
},
115116
{
116117
input: []string{"-Dprop1=value1", "test", "-Pprop2=value2"},
117-
expected: []string{"-Dprop1=value1", "test", "-Pprop2=value2"},
118+
expected: []string{"-Dprop1='value1'", "test", "-Pprop2='value2'"},
118119
},
119120
{
120121
input: []string{"-Dparam1=value1 value2", "-Pkey1=value1", "-Dparam2=value2", "-Pkey2=value1 value2"},
121-
expected: []string{"-Dparam1='value1 value2'", "-Pkey1=value1", "-Dparam2=value2", "-Pkey2='value1 value2'"},
122+
expected: []string{"-Dparam1='value1 value2'", "-Pkey1='value1'", "-Dparam2='value2'", "-Pkey2='value1 value2'"},
122123
},
123124
{
124125
input: []string{"-Dparam1=value1", "run", "-Psign"},
125-
expected: []string{"-Dparam1=value1", "run", "-Psign"},
126+
expected: []string{"-Dparam1='value1'", "run", "-Psign"},
126127
},
127128
}
128129

129130
for _, test := range tests {
130-
result := formatCommandProperties(test.input)
131+
result := quoteArgsForLog(test.input)
131132
assert.ElementsMatch(t, test.expected, result)
132133
}
133134
}
135+
136+
func TestUnquoteProperty(t *testing.T) {
137+
tests := []struct {
138+
input string
139+
expected string
140+
}{
141+
{input: "-Dparam=value", expected: "-Dparam=value"},
142+
{input: "-Ddeploy.test.property=test test", expected: "-Ddeploy.test.property=test test"},
143+
{input: "-Ddeploy.test.property='test test'", expected: "-Ddeploy.test.property=test test"},
144+
{input: `-Pkey="value with spaces"`, expected: "-Pkey=value with spaces"},
145+
// Mismatched quote pair: left as-is.
146+
{input: `-Dparam='value"`, expected: `-Dparam='value"`},
147+
// No '=' at all: returned unchanged rather than panicking on the missing value part.
148+
{input: "-Dparam", expected: "-Dparam"},
149+
}
150+
151+
for _, test := range tests {
152+
assert.Equal(t, test.expected, unquoteProperty(test.input))
153+
}
154+
}
155+
156+
func TestStripPropertyQuotes(t *testing.T) {
157+
tests := []struct {
158+
input []string
159+
expected []string
160+
}{
161+
{
162+
input: []string{"clean", "-Dparam=value", "build", "-Pkey=value"},
163+
expected: []string{"clean", "-Dparam=value", "build", "-Pkey=value"},
164+
},
165+
{
166+
// Quotes already stripped by the shell, value has no surrounding quotes: left as-is.
167+
input: []string{"-Ddeploy.test.property=test test"},
168+
expected: []string{"-Ddeploy.test.property=test test"},
169+
},
170+
{
171+
// Quotes still present in the raw arg (e.g. on Windows, where cmd.exe/PowerShell don't strip
172+
// single quotes the way bash/zsh do): stripped only on Windows.
173+
input: []string{"-Ddeploy.test.property='test test'"},
174+
expected: []string{"-Ddeploy.test.property=test test"},
175+
},
176+
{
177+
input: []string{`-Pkey="value with spaces"`},
178+
expected: []string{"-Pkey=value with spaces"},
179+
},
180+
{
181+
// Mismatched quote pair: left as-is.
182+
input: []string{`-Dparam='value"`},
183+
expected: []string{`-Dparam='value"`},
184+
},
185+
}
186+
187+
for _, test := range tests {
188+
result := stripPropertyQuotes(test.input)
189+
if runtime.GOOS == "windows" {
190+
assert.Equal(t, test.expected, result)
191+
} else {
192+
// On non-Windows platforms the shell already stripped shell-level quoting, so any quotes
193+
// still present in the argument were typed deliberately and must be left untouched.
194+
assert.Equal(t, test.input, result)
195+
}
196+
}
197+
}
198+
199+
func TestGetCmdDoesNotQuotePropertyValues(t *testing.T) {
200+
config := &gradleRunConfig{
201+
gradle: "gradle",
202+
tasks: []string{"artifactoryPublish", "-Ddeploy.test.property='test test'"},
203+
logger: utils.NewDefaultLogger(utils.INFO),
204+
}
205+
206+
cmd := config.GetCmd()
207+
208+
expectedValue := "'test test'"
209+
if runtime.GOOS == "windows" {
210+
// Pre-existing quotes are only stripped on Windows, where the shell doesn't strip them itself.
211+
expectedValue = "test test"
212+
}
213+
// exec.Command runs the process directly, without a shell, so quotes around the value would be passed to
214+
// Gradle literally, instead of being stripped, if we didn't strip them ourselves.
215+
assert.Equal(t, []string{"gradle", "artifactoryPublish", "-Ddeploy.test.property=" + expectedValue}, cmd.Args)
216+
}

entities/buildinfo.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const (
4949
Uv ModuleType = "uv"
5050
Gem ModuleType = "gem"
5151
Apk ModuleType = "apk"
52+
Cargo ModuleType = "cargo"
5253
)
5354

5455
type BuildInfo struct {

entities/buildinfo_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,3 +569,9 @@ func TestBuildInfoAppend_MavenSnapshotScenario(t *testing.T) {
569569
assert.Len(t, buildInfo1.Modules[0].Artifacts, 1, "Should have 1 artifact")
570570
assert.Equal(t, "deploy-sha", buildInfo1.Modules[0].Artifacts[0].Sha1, "Should have newer SHA1")
571571
}
572+
573+
func TestCargoModuleType(t *testing.T) {
574+
if Cargo != "cargo" {
575+
t.Fatalf("expected Cargo module type to be %q, got %q", "cargo", Cargo)
576+
}
577+
}

0 commit comments

Comments
 (0)