diff --git a/man/remind.1.in b/man/remind.1.in index 6c0285de..88d4afab 100644 --- a/man/remind.1.in +++ b/man/remind.1.in @@ -553,7 +553,10 @@ In Agenda Mode, output JSON instead of the normal text-mode output. \fB\-\-json\fR also disables sorting (the \fB-g\fR option) and disables queueing (the \fB-q\fR option). See the section "AGENDA MODE JSON OUTPUT" for more details. The \fB\-\-json\fR option -is ignored in Calendar Mode. +is ignored in Calendar Mode. Note that in JSON mode, the output from +any RUN command that would normally appear on standard output is redirected +to standard error instead; this is so that RUN commands don't mess up +the output and cause invalid JSON to be produced on standard output. .TP .B \-\-print-errs The \fB\-\-print-errs\fR option causes \fBRemind\fR to print all diff --git a/src/files.c b/src/files.c index f0e504af..eb63c5cc 100644 --- a/src/files.c +++ b/src/files.c @@ -179,17 +179,14 @@ got_a_fresh_line(void) WarnedAboutImplicit = 0; } -static void set_cloexec(FILE *fp) +void set_cloexec(int fd) { int flags; - int fd; - if (fp) { - fd = fileno(fp); - flags = fcntl(fd, F_GETFD); - if (flags >= 0) { - flags |= FD_CLOEXEC; - fcntl(fd, F_SETFD, flags); - } + + flags = fcntl(fd, F_GETFD); + if (flags >= 0) { + flags |= FD_CLOEXEC; + fcntl(fd, F_SETFD, flags); } } @@ -218,7 +215,7 @@ static void OpenPurgeFile(char const *fname, char const *mode) fprintf(ErrFp, tr("Cannot open `%s' for writing: %s"), DBufValue(&fname_buf), strerror(errno)); fprintf(ErrFp, "\n"); } - set_cloexec(PurgeFP); + set_cloexec(fileno(PurgeFP)); DBufFree(&fname_buf); } @@ -427,7 +424,9 @@ static int OpenFile(char const *fname) } } else { fp = fopen(fname, "r"); - set_cloexec(fp); + if (fp) { + set_cloexec(fileno(fp)); + } if (DebugFlag & DB_TRACE_FILES) { fprintf(ErrFp, tr("Reading `%s': Opening file on disk"), fname); fprintf(ErrFp, "\n"); @@ -449,7 +448,7 @@ static int OpenFile(char const *fname) if (strcmp(fname, "-")) { fp = fopen(fname, "r"); if (!fp || !CheckSafety()) return E_CANT_OPEN; - set_cloexec(fp); + set_cloexec(fileno(fp)); if (PurgeMode) OpenPurgeFile(fname, "w"); } else { fp = stdin; @@ -650,7 +649,7 @@ static int PopFile(void) if (strcmp(i->filename, "-")) { fp = fopen(i->filename, "r"); if (!fp || !CheckSafety()) return E_CANT_OPEN; - set_cloexec(fp); + set_cloexec(fileno(fp)); if (PurgeMode) OpenPurgeFile(i->filename, "a"); } else { fp = stdin; diff --git a/src/init.c b/src/init.c index ad847b08..3b388565 100644 --- a/src/init.c +++ b/src/init.c @@ -803,6 +803,8 @@ void InitRemind(int argc, char const *argv[]) SortByTime = SORT_NONE; SortByDate = SORT_NONE; SortByPrio = SORT_NONE; + /* Make sure we don't blat errors to stdout! */ + ErrFp = stderr; } /* Figure out the offset from UTC */ diff --git a/src/main.c b/src/main.c index d08fe9e1..5fe7b61f 100644 --- a/src/main.c +++ b/src/main.c @@ -2146,7 +2146,12 @@ System(char const *cmd, int is_queued) } } /* This is the child process or original if we never forked */ - (void) system(cmd); + if (JSONMode) { + + (void) system_to_stderr(cmd); + } else { + (void) system(cmd); + } if (do_exit) { /* In the child process, so exit! */ exit(0); diff --git a/src/protos.h b/src/protos.h index d4fe5c60..6d2566eb 100644 --- a/src/protos.h +++ b/src/protos.h @@ -292,3 +292,5 @@ void SetCurrentFilename(char const *fname); char const *GetCurrentFilename(void); int get_scanfrom(Trigger const *t); void remove_trailing_newlines(DynamicBuffer *buf); +void set_cloexec(int fd); +int system_to_stderr(char const *cmd); diff --git a/src/utils.c b/src/utils.c index 64b37f04..86a88b93 100644 --- a/src/utils.c +++ b/src/utils.c @@ -17,6 +17,7 @@ static char const DontEscapeMe[] = #include "err.h" #include +#include #ifdef HAVE_STRINGS_H #include @@ -30,6 +31,43 @@ static char const DontEscapeMe[] = #include "globals.h" #include "protos.h" +/***************************************************************/ +/* */ +/* system_to_stderr */ +/* */ +/* Run system(...) but with stdout redirected to stderr */ +/* */ +/***************************************************************/ +int system_to_stderr(char const *cmd) +{ + int stdout_dup = dup(STDOUT_FILENO); + int r; + + if (stdout_dup < 0) { + perror("dup"); + return -1; + } + + /* Duplicate STDERR onto STDOUT */ + if (dup2(STDERR_FILENO, STDOUT_FILENO) < 0) { + (void) close(stdout_dup); + return -1; + } + + /* Set close-on-exec flag on stdout_dup */ + set_cloexec(stdout_dup); + + r = system(cmd); + + /* Restore original stdout */ + /* If this dup2 fails... there's not a whole lot we can do. */ + (void) dup2(stdout_dup, STDOUT_FILENO); + if (STDOUT_FILENO != stdout_dup) { + (void) close(stdout_dup); + } + return r; +} + /***************************************************************/ /* */ /* StrnCpy */ diff --git a/tests/json-redirect.rem b/tests/json-redirect.rem new file mode 100644 index 00000000..40afc075 --- /dev/null +++ b/tests/json-redirect.rem @@ -0,0 +1,5 @@ +BANNER % +SET $AddBlankLines 0 +REM MSG Hello +REM RUN echo This is executed by the shell. +REM MSG Goodbye \ No newline at end of file diff --git a/tests/test-rem b/tests/test-rem index f5362e02..54dee474 100644 --- a/tests/test-rem +++ b/tests/test-rem @@ -771,6 +771,13 @@ echo "Testing TODOS in calendar mode with completed todos hidden" >> ../tests/te ../src/remind -s --hide-completed-todos ../tests/todos.rem 2025-08-13 >> ../tests/test.out 2>&1 echo "Testing TODOS and JSON mode" >> ../tests/test.out 2>&1 ../src/remind --json ../tests/todos.rem 2025-08-13 >> ../tests/test.out 2>&1 + +echo "Testing proper redirection of RUN stdout in JSON mode... here's stdout" >> ../tests/test.out 2>&1 +../src/remind --json ../tests/json-redirect.rem 1 Jan 2025 >> ../tests/test.out 2>/dev/null + +echo "... and here is stderr" >> ../tests/test.out 2>&1 +../src/remind --json ../tests/json-redirect.rem 1 Jan 2025 > /dev/null 2>> ../tests/test.out + cmp -s ../tests/test.out ../tests/test.cmp if [ "$?" = "0" ]; then echo "Remind: Acceptance test PASSED" diff --git a/tests/test.cmp b/tests/test.cmp index 07b9e978..86af0a7a 100644 --- a/tests/test.cmp +++ b/tests/test.cmp @@ -39615,3 +39615,10 @@ Testing TODOS and JSON mode {"date":"2025-08-11","filename":"../tests/todos.rem","lineno":17,"d":11,"m":8,"y":2025,"is_todo":1,"trigbase":"2025-08-11","max_overdue":3,"priority":5000,"body":"Yup3 on 2025-08-11"}, {"date":"2025-08-10","filename":"../tests/todos.rem","lineno":18,"d":10,"m":8,"y":2025,"is_todo":1,"trigbase":"2025-08-10","max_overdue":3,"priority":5000,"body":"Yup4 on 2025-08-10"} ] +Testing proper redirection of RUN stdout in JSON mode... here's stdout +[ +{"date":"2025-01-01","filename":"../tests/json-redirect.rem","lineno":3,"is_todo":0,"priority":5000,"body":"Hello"}, +{"date":"2025-01-01","filename":"../tests/json-redirect.rem","lineno":5,"is_todo":0,"priority":5000,"body":"Goodbye"} +] +... and here is stderr +This is executed by the shell.