Windows/DOS line endings in parameters cause jobs to fail under Slurm

[dasmoth]

In general, Slivka quotes command parameters when generating scripts to pass to the batch system. So, for example, if I include a newline in a parameter, I might end up with a batch system script which looks like:

#!/usr/bin/env sh
touch started
python /path/to/example.py --rep 1,2 --delay 0 -- 'some
thing'
echo $? > finished

...which works as intended.

However, there's a special case if carriage returns are present. Consider a nearly-identical example, but with a Windows line-ending (carriage return, line feed, ASCII 0x0d,0x0a) in the quoted parameter. This still works fine as a shell script, but is unfortunately no longer a valid Slurm batch script. sbatch exits with:

sbatch: error: Batch script contains DOS line breaks (\r\n)
sbatch: error: instead` of expected UNIX line breaks (\n)

(In combination with #119 this currently kills the Slivka scheduler).

I appreciate this is a fairly obscure edge case, but thought it was worth raising, since it means that there are inputs which I would expect to work with other runners, but to fail under SlurmRunner, which is a bit annoying. And when dealing with user input, especially from web interfaces, Windows line endings are something that tend to show up when you're not expecting them.

For what it's worth, Slurm will accept:

#!/usr/bin/env bash
touch started
python /path/to/example.py -- rep 1,2 --delay 0 -- $'some\r\nthing'
echo $? > finished

i.e. bash's C-style escaping. I don't know if this is a good solution (in particular, I can see the merits of not depending on bash), but it is a possible answer.

[foreveremain]

in the general case I guess proper escaping should be done, but I shudder to think of the commandline tools that pass newlines via optargs !! I'm fairly sure SGE and other schedulers will similarly balk unless proper bash syntax (like <<END : https://stackoverflow.com/questions/32126653/how-does-end-work-in-bash-to-create-a-multi-line-comment-block )

[warownia1]

I agree that proper escaping is the way to go, however it's a very shell dependent solution

On Wed, 23 Feb 2022 at 14:24, Jim Procter @.***> wrote:

in the general case I guess proper escaping should be done, but I shudder to think of the commandline tools that pass newlines via optargs !! I'm fairly sure SGE and other schedulers will similarly balk unless proper bash syntax (like <<END : https://stackoverflow.com/questions/32126653/how-does-end-work-in-bash-to-create-a-multi-line-comment-block )

— Reply to this email directly, view it on GitHub https://github.com/bartongroup/slivka/issues/120#issuecomment-1048778361, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADAMLDQZQ37DQRMCSCWUMZ3U4TNZXANCNFSM5PBBRQYA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

[dasmoth]

foreveremain: yes, completely agree, this is a very awkward corner that probably isn't going to happen intentionally. However, it does creep in once you're dealing with (potentially copy-pasted) user input.

I'm not sure myself what would be the best solution. Just forbidding this kind of input might not be totally unreasonable, but in that case I suggest it should be an error at job submission time, rather than accepting the job then failing in a -- runner-dependent -- fashion.

warownia1: in a sense, I don't think this is particularly shell-dependent as such -- rather, it's down to Slurm imposing some requirements that the actual shells do not. On that basis, one possible solution would be to write the command line to the filesystem as a separate shell script, and then just make the job script submitted to Slurm a shim which invokes the "actual" job script.

[warownia1]

What I meant by shell dependent is that different shells have slightly different ways to escape special characters. Does slurm respect shebang line and run the script with shell specified at the top of the file? If so, we can stick to sh escaping which should be quite easy to do.

Get Outlook for Androidhttps://aka.ms/AAb9ysg

---

From: Thomas Down @.> Sent: Friday, February 25, 2022 10:51:10 AM To: bartongroup/slivka @.> Cc: Mateusz Warowny @.>; Comment @.> Subject: Re: [bartongroup/slivka] Windows/DOS line endings in parameters cause jobs to fail under Slurm (Issue #120)

foreveremain: yes, completely agree, this is a very awkward corner that probably isn't going to happen intentionally. However, it does creep in once you're dealing with (potentially copy-pasted) user input.

I'm not sure myself what would be the best solution. Just forbidding this kind of input might not be totally unreasonable, but in that case I suggest it should be an error at job submission time, rather than accepting the job then failing in a -- runner-dependent -- fashion.

warownia1: in a sense, I don't think this is particularly shell-dependent as such -- rather, it's down to Slurm imposing some requirements that the actual shells do not. On that basis, one possible solution would be to write the command line to the filesystem as a separate shell script, and then just make the job script submitted to Slurm a shim which invokes the "actual" job script.

— Reply to this email directly, view it on GitHubhttps://github.com/bartongroup/slivka/issues/120#issuecomment-1050701279, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADAMLDVJ3VLDG3KIRLFRWF3U45GI5ANCNFSM5PBBRQYA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you commented.Message ID: @.***>

[warownia1]

Pushed a fix that escapes all control characters using ANSI-C quoting in bash. I'm not closing this issue until I fix this on the main branch.