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

Bug: cannot add a very long command to queue #24

Closed
orsharir opened this issue Sep 22, 2022 · 14 comments
Closed

Bug: cannot add a very long command to queue #24

orsharir opened this issue Sep 22, 2022 · 14 comments

Comments

@orsharir
Copy link

When trying to add a command with many parameters, task-spooler will crash at:

error("Reading the size of the name");

I'm currently using a workaround by having a script that stores the command in a variable, and then pass task-spooler a script that reads the command from the variable and execute it. But that's just a hack of course.

Here are my workaround scripts:

ts_helper with usage ts_helper OPTIONS @ LONG_COMMAND:

#!/bin/bash
IFS='@' read -ra ARGS <<< "$@";
if [ ${#ARGS[@]} -eq 2 ];
then
  SCRIPTRAW="${ARGS[1]}"
  SCRIPT="$SCRIPTRAW" ts "${ARGS[0]}" ts_helper_runner;
else
  echo "Wrong number of arguments";
fi;

which calls the script ts_helper_runner:

#!/bin/bash
exec $SCRIPT
@justanhduc
Copy link
Owner

Hey @orsharir. Thanks for you interest in ts. Unfortunately for long and complex command, putting it in a script is actually a recommend way as ts does not have a some fancy arg parsing tool. Another way is using sh -c 'your command' as you can find here in the Bugs section.
Nonetheless, I find it quite odd when the error happens when sending the log filename, as it should not have anything to do with the command. If possible could you please share your command so I can investigate?

@orsharir
Copy link
Author

To clarify, my command is a script with many parameters, so there shouldn't be any need for sh in my case. My specific command call is of no concern (it's a script for training an ML model with many arguments to configure it), but here is a simple example.

This call will work fine:

ts echo 'A long long time ago, in a Galaxy far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far away'

However, this slightly longer call will fail (I'm using -f here to capture the error message from ts):

ts -f echo 'A long long time ago, in a Galaxy far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far and even further away'

@justanhduc
Copy link
Owner

Cool! My gut feeling is that something overflows at some point. I'm a bit occupied at the moment but I will investigate it asap. If you have any insight please let me know. Thanks!

@mw75
Copy link
Contributor

mw75 commented Oct 18, 2022

I have a similar problem, but it shows a little different. I create tasks via webhook using https://github.com/adnanh/webhook and i'm using the -m/Mail function of ts and all that works great - almost!

My last parameter is one long string. The spooled script receives the last parameter complete! But the created mail has some binary garbage in the line starting with "Command:". That already breaks the view in Outlook and on mobile devices but the webview works. The garbage begins at character 500 of the mail body.

Hopefully these information may help to reproduce and fix the problem.

@justanhduc
Copy link
Owner

Hey @mw75. Thanks for your info. Just to clarify, did you get it to work with a shorter command?

@justanhduc
Copy link
Owner

Hi @orsharir @mw75. The issue should be fixed in the latest commit in master. Thanks for reporting the bug!

@orsharir
Copy link
Author

Thanks @justanhduc, it appears to be fixed on my end.

@mw75
Copy link
Contributor

mw75 commented Oct 21, 2022

Sadly it does not fix my problem!

While compiling with make cpu i get

execute.c:207:17: warning: ‘strncpy’ specified bound 10 equals destination size [-Wstringop-truncation]
                 strncpy(errfname, outfname_full, sizeof errfname);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
execute.c:184:17: warning: ‘strncpy’ specified bound 10 equals destination size [-Wstringop-truncation]
                 strncpy(errfname, outfname_full, sizeof errfname);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...

man.c:801:22: warning: string length ‘11589’ is greater than the length ‘4095’ ISO C99 compilers are required to support [-Woverlength-strings]
                      ".B ts.", tm.tm_year + 1900, tm.tm_mon + 1, TS_MAKE_STR(TS_VERSION), TS_MAKE_STR(TS_VERSION));
                      ^~~~~~~~

I don't know if it is related to the problem or not. Is there something i'm doing wrong. Running and compiling on debian 10.

@orsharir could you have a second look, please?

@justanhduc
Copy link
Owner

Hey @mw75. They are warnings, and that piece of codes is only executed when you use -E to queue jobs.
Is there any way you can reproduce the problem for me, for e.g., Colab or Docker? Otherwise, it is very difficult to debug just by the information you gave me.

@mw75
Copy link
Contributor

mw75 commented Nov 2, 2022

@justanhduc Sorry, missed your response because the issue is still closed!

Steps to reproduce an clean debian system:

#!/bin/bash
cd root/
apt-get update && apt-get install -y build-essential git
git clone https://github.com/justanhduc/task-spooler.git
cd task-spooler/
make cpu
cat >/usr/sbin/sendmail <<EOF
#!/bin/bash
echo "$0 $@" >> /tmp/sendmail.out
cat >> /tmp/sendmail.out
EOF
chmod 755 /usr/sbin/sendmail
./ts -t "$( [email protected] ./ts -m /bin/bash -c "echo Start;echo looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong;sleep 5;echo End" )"
cat /tmp/sendmail.out

I run it with podman run --rm -it -v .:/root debian /root/tsp-problem-replay.sh

Output:

From: Task Spooler <taskspooler>
To: [email protected]
Subject: the task 0 finished with error 0.

Command: /bin/bash -c echo Start;echo looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
{Ba�{Ba�Output:
/bin/bash -c echo Start;echo looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong;sleep 5;echo End

There is also some more garbage in front of "Output:"

I hope that helps to reproduce! Please reopen the issue so i get notified on updates!

@mw75
Copy link
Contributor

mw75 commented Nov 2, 2022

diff --git a/mail.c b/mail.c
index 6d3d511..8f1feb6 100755
--- a/mail.c
+++ b/mail.c
@@ -50,7 +50,7 @@ static void write_header(int fd, const char *dest, const char *command,
     fd_nprintf(fd, 500, "To: %s\n", dest);
     fd_nprintf(fd, 500, "Subject: the task %i finished with error %i. \n", jobid,
                errorlevel);
-    fd_nprintf(fd, 500, "\nCommand: %s\n", command);
+    fd_nprintf(fd, 12+(int)strlen(command), "\nCommand: %s\n", command);
     fd_nprintf(fd, 500, "Output:\n");
 }

Don't know if this works with unicode/multibyte strings, but it fixed my problem.

@justanhduc justanhduc reopened this Nov 3, 2022
@justanhduc
Copy link
Owner

diff --git a/mail.c b/mail.c
index 6d3d511..8f1feb6 100755
--- a/mail.c
+++ b/mail.c
@@ -50,7 +50,7 @@ static void write_header(int fd, const char *dest, const char *command,
     fd_nprintf(fd, 500, "To: %s\n", dest);
     fd_nprintf(fd, 500, "Subject: the task %i finished with error %i. \n", jobid,
                errorlevel);
-    fd_nprintf(fd, 500, "\nCommand: %s\n", command);
+    fd_nprintf(fd, 12+(int)strlen(command), "\nCommand: %s\n", command);
     fd_nprintf(fd, 500, "Output:\n");
 }

Don't know if this works with unicode/multibyte strings, but it fixed my problem.

Hey @mw75! Great catch! I can see that your command is a little longer than the hardcoded 500, so it truncates the command and prints some garbage at the end of the sentence.
I wouldn't worry much about unicode, as C itself handles it poorly and strlen is used everywhere else.
Would you like to make a PR? :)

@mw75
Copy link
Contributor

mw75 commented Nov 3, 2022

@justanhduc it would be least effort if you just copy paste the single line as i only cloned your repo without forking right now. Will try to take time for a PR next days if you absolutly prefer.

@justanhduc
Copy link
Owner

Well, sending PR is the best way to credit you as you will show up in the contributor list :). You can take as much time as you need.

justanhduc added a commit that referenced this issue Nov 26, 2022
valllle added a commit to valllle/task-spooler that referenced this issue Aug 16, 2023
bug from justanhduc#24 still persisted, fixxed it
justanhduc added a commit that referenced this issue Aug 24, 2023
Fixed bug in mail.c causing #24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants