Skip to content

Commit

Permalink
Allow escaping commas and colons in source bind path with the followi…
Browse files Browse the repository at this point in the history
…ng syntax:

- `singularity shell -B /tmp/comma\\,separated_dir:/data image`
- `singularity shell -B "/tmp/comma\,separated_dir":/data image`
- `singularity shell -B /tmp/colon\\:sep_dir:/data image`
- `singularity shell -B /tmp/"colon\:sep_dir":/data image`

- Fixes apptainer#5923
- Fixes apptainer#5959
  • Loading branch information
cclerget committed Jun 4, 2021
1 parent 7cadf7b commit fba99bf
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 31 deletions.
2 changes: 1 addition & 1 deletion cmd/internal/cli/action_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ var actionAppFlag = cmdline.Flag{
var actionBindFlag = cmdline.Flag{
ID: "actionBindFlag",
Value: &BindPaths,
DefaultValue: []string{},
DefaultValue: cmdline.StringArray{}, // to allow commas in bind path
Name: "bind",
ShortHand: "B",
Usage: "a user-bind path specification. spec has the format src[:dest[:opts]], where src and dest are outside and inside paths. If dest is not given, it is set equal to src. Mount options ('opts') may be specified as 'ro' (read-only) or 'rw' (read/write, which is the default). Multiple bind paths can be given by a comma separated list.",
Expand Down
2 changes: 1 addition & 1 deletion cmd/internal/cli/actions_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func execStarter(cobraCmd *cobra.Command, image string, args []string, name stri
img.File.Close()
}

binds, err := singularityConfig.ParseBindPath(strings.Join(BindPaths, ","))
binds, err := singularityConfig.ParseBindPath(BindPaths)
if err != nil {
sylog.Fatalf("while parsing bind path: %s", err)
}
Expand Down
45 changes: 45 additions & 0 deletions e2e/actions/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"os"
"path/filepath"
"strconv"
"strings"
"syscall"
"testing"
"time"
Expand Down Expand Up @@ -1065,6 +1066,8 @@ func (c actionTests) actionBinds(t *testing.T) {

contCanaryFile := "/canary/file"
hostCanaryFile := filepath.Join(hostCanaryDir, "file")
hostCanaryFileWithComma := filepath.Join(hostCanaryDir, "file,comma")
hostCanaryFileWithColon := filepath.Join(hostCanaryDir, "file:colon")

canaryFileBind := hostCanaryFile + ":" + contCanaryFile
canaryDirBind := hostCanaryDir + ":" + contCanaryDir
Expand All @@ -1091,6 +1094,12 @@ func (c actionTests) actionBinds(t *testing.T) {
if err := fs.Touch(hostCanaryFile); err != nil {
t.Fatalf("failed to create canary_file: %s", err)
}
if err := fs.Touch(hostCanaryFileWithComma); err != nil {
t.Fatalf("failed to create canary_file_comma: %s", err)
}
if err := fs.Touch(hostCanaryFileWithColon); err != nil {
t.Fatalf("failed to create canary_file_colon: %s", err)
}
if err := os.Chmod(hostCanaryFile, 0777); err != nil {
t.Fatalf("failed to apply permissions on canary_file: %s", err)
}
Expand Down Expand Up @@ -1467,6 +1476,42 @@ func (c actionTests) actionBinds(t *testing.T) {
postRun: checkHostDir(filepath.Join(hostWorkDir, "scratch/scratch", "dir")),
exit: 0,
},
{
name: "BindFileWithCommaOK",
args: []string{
"--bind", strings.ReplaceAll(hostCanaryFileWithComma, ",", "\\,") + ":" + contCanaryFile,
sandbox,
"test", "-f", contCanaryFile,
},
exit: 0,
},
{
name: "BindFileWithCommaKO",
args: []string{
"--bind", hostCanaryFileWithComma + ":" + contCanaryFile,
sandbox,
"test", "-f", contCanaryFile,
},
exit: 255,
},
{
name: "BindFileWithColonOK",
args: []string{
"--bind", strings.ReplaceAll(hostCanaryFileWithColon, ":", "\\:") + ":" + contCanaryFile,
sandbox,
"test", "-f", contCanaryFile,
},
exit: 0,
},
{
name: "BindFileWithColonKO",
args: []string{
"--bind", hostCanaryFileWithColon + ":" + contCanaryFile,
sandbox,
"test", "-f", contCanaryFile,
},
exit: 255,
},
}

for _, profile := range e2e.Profiles {
Expand Down
16 changes: 16 additions & 0 deletions pkg/cmdline/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const (
Linux = "linux"
)

type StringArray []string

// Flag holds information about a command flag
type Flag struct {
ID string
Expand Down Expand Up @@ -92,6 +94,8 @@ func (m *flagManager) registerFlagForCmd(flag *Flag, cmds ...*cobra.Command) err
m.registerStringVar(flag, cmds)
case []string:
m.registerStringSliceVar(flag, cmds)
case StringArray:
m.registerStringArrayVar(flag, cmds)
case bool:
m.registerBoolVar(flag, cmds)
case int:
Expand Down Expand Up @@ -129,6 +133,18 @@ func (m *flagManager) registerStringSliceVar(flag *Flag, cmds []*cobra.Command)
return nil
}

func (m *flagManager) registerStringArrayVar(flag *Flag, cmds []*cobra.Command) error {
for _, c := range cmds {
if flag.ShortHand != "" {
c.Flags().StringArrayVarP(flag.Value.(*[]string), flag.Name, flag.ShortHand, ([]string)(flag.DefaultValue.(StringArray)), flag.Usage)
} else {
c.Flags().StringArrayVar(flag.Value.(*[]string), flag.Name, ([]string)(flag.DefaultValue.(StringArray)), flag.Usage)
}
m.setFlagOptions(flag, c)
}
return nil
}

func (m *flagManager) registerBoolVar(flag *Flag, cmds []*cobra.Command) error {
for _, c := range cmds {
if flag.ShortHand != "" {
Expand Down
119 changes: 90 additions & 29 deletions pkg/runtime/engine/singularity/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,8 @@ func (e *EngineConfig) GetCustomHome() bool {

// ParseBindPath parses a string and returns all encountered
// bind paths as array.
func ParseBindPath(bindpaths string) ([]BindPath, error) {
var bind string
func ParseBindPath(paths []string) ([]BindPath, error) {
var binds []BindPath
var elem int

var validOptions = map[string]bool{
"ro": true,
Expand All @@ -312,13 +310,16 @@ func ParseBindPath(bindpaths string) ([]BindPath, error) {
// - source1:destination1 -> [source1:, destination1]
// - source1:destination1:option1 -> [source1:, destination1:, option1]
// - source1:destination1:option1,option2 -> [source1:, destination1:, option1, option2]
for _, m := range re.FindAllString(bindpaths, -1) {
s := strings.TrimSpace(m)
isColon := bind != "" && bind[len(bind)-1] == ':'

// options are taken only if the bind has a source
// and a destination
if elem == 2 {
for _, path := range paths {
concatComma := false
concatColon := false
bind := ""
elem := 0

for _, m := range re.FindAllString(path, -1) {
s := strings.TrimSpace(m)

isOption := false

for option, flag := range validOptions {
Expand All @@ -334,54 +335,114 @@ func ParseBindPath(bindpaths string) ([]BindPath, error) {
}
}
}
if isOption {

if elem == 2 && !isOption {
bp, err := newBindPath(bind, validOptions)
if err != nil {
return nil, fmt.Errorf("while getting bind path: %s", err)
}
binds = append(binds, bp)
elem = 0
bind = ""
}

if elem == 0 {
// escaped commas and colons
if (len(s) > 0 && s[len(s)-1] == '\\') || concatComma {
if !concatComma {
bind += s[:len(s)-1] + ","
} else {
bind += s
elem++
}
concatComma = !concatComma
continue
} else if (len(s) >= 2 && s[len(s)-2] == '\\' && s[len(s)-1] == ':') || concatColon {
bind += s
if concatColon {
elem++
}
concatColon = !concatColon
continue
} else if bind == "" {
bind = s
}
}

isColon := bind != "" && bind[len(bind)-1] == ':'

// options are taken only if the bind has a source
// and a destination
if elem == 2 && isOption {
if !isColon {
bind += ","
}
bind += s
continue
} else if elem > 2 {
return nil, fmt.Errorf("wrong bind syntax: %s", bind)
}
} else if elem > 2 {
return nil, fmt.Errorf("wrong bind syntax: %s", bind)
}

elem++
if bind != "" {
if isColon {
if elem > 0 {
bind += s
}
fmt.Println("append", bind, s)
//bind += s
elem++
continue
}
bp, err := newBindPath(bind, validOptions)
if err != nil {
return nil, fmt.Errorf("while getting bind path: %s", err)
}
binds = append(binds, bp)
elem = 0
}
// new bind path
bind = s
elem++
}

if bind != "" {
if isColon {
bind += s
continue
}
bp, err := newBindPath(bind, validOptions)
if err != nil {
return nil, fmt.Errorf("while getting bind path: %s", err)
}
binds = append(binds, bp)
elem = 1
}
// new bind path
bind = s
}

if bind != "" {
bp, err := newBindPath(bind, validOptions)
if err != nil {
return nil, fmt.Errorf("while getting bind path: %s", err)
return binds, nil
}

func splitBy(str string, sep byte) []string {
var list []string

re := regexp.MustCompile(fmt.Sprintf(`(?m)([^\\]%c)`, sep))
cursor := 0

indexes := re.FindAllStringIndex(str, -1)
for i, index := range indexes {
list = append(list, str[cursor:index[1]-1])
cursor = index[1]
if len(indexes)-1 == i {
return append(list, str[cursor:])
}
binds = append(binds, bp)
}

return binds, nil
return append(list, str)
}

// newBindPath returns BindPath record based on the provided bind
// string argument and ensures that the options are valid.
func newBindPath(bind string, validOptions map[string]bool) (BindPath, error) {
var bp BindPath

splitted := strings.SplitN(bind, ":", 3)
splitted := splitBy(bind, ':')

bp.Source = splitted[0]
bp.Source = strings.ReplaceAll(splitted[0], "\\:", ":")
if bp.Source == "" {
return bp, fmt.Errorf("empty bind source for bind path %q", bind)
}
Expand Down

0 comments on commit fba99bf

Please sign in to comment.