Resolve escaping error in genrules sources with $
'$' cannot be included in the genrule source file name as it is an invalid character. One workaround to include a file with '$' in the filename is to use glob(*) and match pattern. However, shell currently evaluates the '$' sign and leads to unexpected behavior. This change fixes the issue by shell-escaping the filepath in generating build actions. Test: m Bug: b/194980152 Change-Id: I6fd919c568b5b6526e4de5155104a08ecadab307
This commit is contained in:
@@ -26,6 +26,7 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"android/soong/bazel/cquery"
|
"android/soong/bazel/cquery"
|
||||||
|
|
||||||
"github.com/google/blueprint"
|
"github.com/google/blueprint"
|
||||||
"github.com/google/blueprint/bootstrap"
|
"github.com/google/blueprint/bootstrap"
|
||||||
"github.com/google/blueprint/proptools"
|
"github.com/google/blueprint/proptools"
|
||||||
@@ -468,6 +469,7 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
|
|||||||
return "SOONG_ERROR", nil
|
return "SOONG_ERROR", nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Apply shell escape to each cases to prevent source file paths containing $ from being evaluated in shell
|
||||||
switch name {
|
switch name {
|
||||||
case "location":
|
case "location":
|
||||||
if len(g.properties.Tools) == 0 && len(g.properties.Tool_files) == 0 {
|
if len(g.properties.Tools) == 0 && len(g.properties.Tool_files) == 0 {
|
||||||
@@ -481,15 +483,15 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
|
|||||||
return reportError("default label %q has multiple files, use $(locations %s) to reference it",
|
return reportError("default label %q has multiple files, use $(locations %s) to reference it",
|
||||||
firstLabel, firstLabel)
|
firstLabel, firstLabel)
|
||||||
}
|
}
|
||||||
return paths[0], nil
|
return proptools.ShellEscape(paths[0]), nil
|
||||||
case "in":
|
case "in":
|
||||||
return strings.Join(cmd.PathsForInputs(srcFiles), " "), nil
|
return strings.Join(proptools.ShellEscapeList(cmd.PathsForInputs(srcFiles)), " "), nil
|
||||||
case "out":
|
case "out":
|
||||||
var sandboxOuts []string
|
var sandboxOuts []string
|
||||||
for _, out := range task.out {
|
for _, out := range task.out {
|
||||||
sandboxOuts = append(sandboxOuts, cmd.PathForOutput(out))
|
sandboxOuts = append(sandboxOuts, cmd.PathForOutput(out))
|
||||||
}
|
}
|
||||||
return strings.Join(sandboxOuts, " "), nil
|
return strings.Join(proptools.ShellEscapeList(sandboxOuts), " "), nil
|
||||||
case "depfile":
|
case "depfile":
|
||||||
referencedDepfile = true
|
referencedDepfile = true
|
||||||
if !Bool(g.properties.Depfile) {
|
if !Bool(g.properties.Depfile) {
|
||||||
@@ -497,7 +499,7 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
|
|||||||
}
|
}
|
||||||
return "__SBOX_DEPFILE__", nil
|
return "__SBOX_DEPFILE__", nil
|
||||||
case "genDir":
|
case "genDir":
|
||||||
return cmd.PathForOutput(task.genDir), nil
|
return proptools.ShellEscape(cmd.PathForOutput(task.genDir)), nil
|
||||||
default:
|
default:
|
||||||
if strings.HasPrefix(name, "location ") {
|
if strings.HasPrefix(name, "location ") {
|
||||||
label := strings.TrimSpace(strings.TrimPrefix(name, "location "))
|
label := strings.TrimSpace(strings.TrimPrefix(name, "location "))
|
||||||
@@ -509,7 +511,7 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
|
|||||||
return reportError("label %q has multiple files, use $(locations %s) to reference it",
|
return reportError("label %q has multiple files, use $(locations %s) to reference it",
|
||||||
label, label)
|
label, label)
|
||||||
}
|
}
|
||||||
return paths[0], nil
|
return proptools.ShellEscape(paths[0]), nil
|
||||||
} else {
|
} else {
|
||||||
return reportError("unknown location label %q is not in srcs, out, tools or tool_files.", label)
|
return reportError("unknown location label %q is not in srcs, out, tools or tool_files.", label)
|
||||||
}
|
}
|
||||||
@@ -520,7 +522,7 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
|
|||||||
if len(paths) == 0 {
|
if len(paths) == 0 {
|
||||||
return reportError("label %q has no files", label)
|
return reportError("label %q has no files", label)
|
||||||
}
|
}
|
||||||
return strings.Join(paths, " "), nil
|
return proptools.ShellEscape(strings.Join(paths, " ")), nil
|
||||||
} else {
|
} else {
|
||||||
return reportError("unknown locations label %q is not in srcs, out, tools or tool_files.", label)
|
return reportError("unknown locations label %q is not in srcs, out, tools or tool_files.", label)
|
||||||
}
|
}
|
||||||
|
@@ -422,7 +422,7 @@ func TestGenruleCmd(t *testing.T) {
|
|||||||
|
|
||||||
allowMissingDependencies: true,
|
allowMissingDependencies: true,
|
||||||
|
|
||||||
expect: "cat ***missing srcs :missing*** > __SBOX_SANDBOX_DIR__/out/out",
|
expect: "cat '***missing srcs :missing***' > __SBOX_SANDBOX_DIR__/out/out",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "tool allow missing dependencies",
|
name: "tool allow missing dependencies",
|
||||||
@@ -434,7 +434,7 @@ func TestGenruleCmd(t *testing.T) {
|
|||||||
|
|
||||||
allowMissingDependencies: true,
|
allowMissingDependencies: true,
|
||||||
|
|
||||||
expect: "***missing tool :missing*** > __SBOX_SANDBOX_DIR__/out/out",
|
expect: "'***missing tool :missing***' > __SBOX_SANDBOX_DIR__/out/out",
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -878,6 +878,92 @@ func TestGenruleWithBazel(t *testing.T) {
|
|||||||
android.AssertDeepEquals(t, "output deps", expectedOutputFiles, gen.outputDeps.Strings())
|
android.AssertDeepEquals(t, "output deps", expectedOutputFiles, gen.outputDeps.Strings())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestGenruleWithGlobPaths(t *testing.T) {
|
||||||
|
testcases := []struct {
|
||||||
|
name string
|
||||||
|
bp string
|
||||||
|
additionalFiles android.MockFS
|
||||||
|
expectedCmd string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "single file in directory with $ sign",
|
||||||
|
bp: `
|
||||||
|
genrule {
|
||||||
|
name: "gen",
|
||||||
|
srcs: ["inn*.txt"],
|
||||||
|
out: ["out.txt"],
|
||||||
|
cmd: "cp $(in) $(out)",
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
additionalFiles: android.MockFS{"inn$1.txt": nil},
|
||||||
|
expectedCmd: "cp 'inn$1.txt' __SBOX_SANDBOX_DIR__/out/out.txt",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "multiple file in directory with $ sign",
|
||||||
|
bp: `
|
||||||
|
genrule {
|
||||||
|
name: "gen",
|
||||||
|
srcs: ["inn*.txt"],
|
||||||
|
out: ["."],
|
||||||
|
cmd: "cp $(in) $(out)",
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
additionalFiles: android.MockFS{"inn$1.txt": nil, "inn$2.txt": nil},
|
||||||
|
expectedCmd: "cp 'inn$1.txt' 'inn$2.txt' __SBOX_SANDBOX_DIR__/out",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "file in directory with other shell unsafe character",
|
||||||
|
bp: `
|
||||||
|
genrule {
|
||||||
|
name: "gen",
|
||||||
|
srcs: ["inn*.txt"],
|
||||||
|
out: ["out.txt"],
|
||||||
|
cmd: "cp $(in) $(out)",
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
additionalFiles: android.MockFS{"inn@1.txt": nil},
|
||||||
|
expectedCmd: "cp 'inn@1.txt' __SBOX_SANDBOX_DIR__/out/out.txt",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "glob location param with filepath containing $",
|
||||||
|
bp: `
|
||||||
|
genrule {
|
||||||
|
name: "gen",
|
||||||
|
srcs: ["**/inn*"],
|
||||||
|
out: ["."],
|
||||||
|
cmd: "cp $(in) $(location **/inn*)",
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
additionalFiles: android.MockFS{"a/inn$1.txt": nil},
|
||||||
|
expectedCmd: "cp 'a/inn$1.txt' 'a/inn$1.txt'",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "glob locations param with filepath containing $",
|
||||||
|
bp: `
|
||||||
|
genrule {
|
||||||
|
name: "gen",
|
||||||
|
tool_files: ["**/inn*"],
|
||||||
|
out: ["out.txt"],
|
||||||
|
cmd: "cp $(locations **/inn*) $(out)",
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
additionalFiles: android.MockFS{"a/inn$1.txt": nil},
|
||||||
|
expectedCmd: "cp '__SBOX_SANDBOX_DIR__/tools/src/a/inn$1.txt' __SBOX_SANDBOX_DIR__/out/out.txt",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, test := range testcases {
|
||||||
|
t.Run(test.name, func(t *testing.T) {
|
||||||
|
result := android.GroupFixturePreparers(
|
||||||
|
prepareForGenRuleTest,
|
||||||
|
android.FixtureMergeMockFs(test.additionalFiles),
|
||||||
|
).RunTestWithBp(t, test.bp)
|
||||||
|
gen := result.Module("gen", "").(*Module)
|
||||||
|
android.AssertStringEquals(t, "command", test.expectedCmd, gen.rawCommands[0])
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
type testTool struct {
|
type testTool struct {
|
||||||
android.ModuleBase
|
android.ModuleBase
|
||||||
outputFile android.Path
|
outputFile android.Path
|
||||||
|
Reference in New Issue
Block a user