In --json mode, redirect any RUN ... command stdout to stderr

We really don't want to mess up the JSON we produce on stdout!!
This commit is contained in:
Dianne Skoll
2025-08-14 21:47:40 -04:00
parent b9fb215d9d
commit de2ec1aa7b
9 changed files with 83 additions and 15 deletions

View File

@@ -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

View File

@@ -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;

View File

@@ -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 */

View File

@@ -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);

View File

@@ -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);

View File

@@ -17,6 +17,7 @@ static char const DontEscapeMe[] =
#include "err.h"
#include <string.h>
#include <unistd.h>
#ifdef HAVE_STRINGS_H
#include <strings.h>
@@ -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 */

5
tests/json-redirect.rem Normal file
View File

@@ -0,0 +1,5 @@
BANNER %
SET $AddBlankLines 0
REM MSG Hello
REM RUN echo This is executed by the shell.
REM MSG Goodbye

View File

@@ -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"

View File

@@ -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.