Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix handling of \n in submission cli --add-file= option #6504

Merged
merged 4 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions src/bindings/python/flux/cli/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,23 @@ def init_jobspec(self, args):
"""
raise NotImplementedError()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit message: s/indicaates/indicates/

def handle_add_file_arg(self, jobspec, arg):
"""Process a single argument to --add-file=ARG."""
# Note: Replace any newline escaped by the shell with literal '\n'
# so that newline detection below works for file data passed on
# on the command line:
name, _, data = arg.replace("\\n", "\n").partition("=")
if not data:
# No '=' implies path-only argument (no multiline allowed)
if "\n" in name:
raise ValueError("--add-file: file name missing")
data = name
name = basename(data)
try:
jobspec.add_file(name, data)
except (TypeError, ValueError, OSError) as exc:
raise ValueError(f"--add-file={arg}: {exc}") from None

# pylint: disable=too-many-branches,too-many-statements
def jobspec_create(self, args):
"""
Expand Down Expand Up @@ -1081,17 +1098,7 @@ def jobspec_create(self, args):

if args.add_file is not None:
for arg in args.add_file:
name, _, data = arg.partition("=")
if not data:
# No '=' implies path-only argument (no multiline allowed)
if "\n" in name:
raise ValueError("--add-file: file name missing")
data = name
name = basename(data)
try:
jobspec.add_file(name, data)
except (TypeError, ValueError, OSError) as exc:
raise ValueError(f"--add-file={arg}: {exc}") from None
self.handle_add_file_arg(jobspec, arg)

return jobspec

Expand Down
2 changes: 1 addition & 1 deletion src/bindings/python/flux/job/Jobspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ def add_file(self, path, data, perms=0o0600, encoding=None):
raise TypeError("data must be a Mapping or string")

files = self.jobspec["attributes"]["system"].get("files", {})
if "\n" in data and encoding is None:
if encoding is None and "\n" in data:
# Use default encoding of utf-8 if data contains newlines,
# since this is presumed to be file content.
encoding = "utf-8"
Expand Down
5 changes: 5 additions & 0 deletions t/t2710-python-cli-submit.t
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,11 @@ test_expect_success 'flux submit --add-file=name=file works' '
cp {{tmpdir}}/myfile . &&
test_cmp file.txt myfile
'
test_expect_success 'flux submit --add-file=name=data works' '
flux submit -n1 --watch --add-file=add-file.test="this is a test\n" \
cp {{tmpdir}}/add-file.test . &&
grep "this is a test" add-file.test
'
test_expect_success 'flux submit --add-file complains for non-regular files' '
test_must_fail flux submit -n1 --add-file=/tmp hostname
'
Expand Down
Loading