From 7aa3da1cd7e3221b6acac8e4f97cc2af2ed88dc9 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 22 Sep 2020 14:15:52 -0400 Subject: [PATCH 1/8] Use IS_CONSOLE macro to detect that we're indeed using a console --- programs/util.c | 4 ++++ tests/playTests.sh | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/programs/util.c b/programs/util.c index d828dc428..b4f1f402e 100644 --- a/programs/util.c +++ b/programs/util.c @@ -90,6 +90,10 @@ int g_utilDisplayLevel; int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, const char* acceptableLetters) { int ch, result; + /* If input is presented via stdin, dont use prompt as it may swallow characters */ + if (!IS_CONSOLE(stdin)) + return 0; + UTIL_DISPLAY("%s", prompt); ch = getchar(); result = 0; diff --git a/tests/playTests.sh b/tests/playTests.sh index d9407db89..44fd74008 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -309,7 +309,6 @@ test -f precompressedFilterTestDir/input.5.zst.zst test -f precompressedFilterTestDir/input.6.zst.zst println "Test completed" - println "\n===> recursive mode test " # combination of -r with empty list of input file zstd -c -r < tmp > tmp.zst From 21cd640b931f5a3ce2acc14dcb0f2e931bf7296e Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 22 Sep 2020 14:16:26 -0400 Subject: [PATCH 2/8] Add unit tests to guard against bad stdin --- tests/playTests.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/playTests.sh b/tests/playTests.sh index 44fd74008..aee790da5 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -309,6 +309,18 @@ test -f precompressedFilterTestDir/input.5.zst.zst test -f precompressedFilterTestDir/input.6.zst.zst println "Test completed" + + +println "\n===> warning prompt does not swallow characters" +println "y" > tmpPrompt +println "hello world" >> tmpPrompt +zstd tmpPrompt +zstd < tmpPrompt -o tmpPrompt.zst +zstd -q -d tmpPrompt.zst -o tmpPromptRegenerated +$DIFF tmpPromptRegenerated tmpPrompt +println "Test completed" + + println "\n===> recursive mode test " # combination of -r with empty list of input file zstd -c -r < tmp > tmp.zst From 0e8ac6b995c63665703495982f95a3f9df48e4d4 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 24 Sep 2020 15:49:30 -0400 Subject: [PATCH 3/8] Add fCtx to FIO_openDstFile() --- programs/fileio.c | 12 ++++++------ programs/util.c | 3 --- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index b27f314ad..b8700f5d4 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -603,8 +603,8 @@ static FILE* FIO_openSrcFile(const char* srcFileName) * condition : `dstFileName` must be non-NULL. * @result : FILE* to `dstFileName`, or NULL if it fails */ static FILE* -FIO_openDstFile(FIO_prefs_t* const prefs, - const char* srcFileName, const char* dstFileName) +FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, + const char* srcFileName, const char* dstFileName) { if (prefs->testMode) return NULL; /* do not open file in test mode */ @@ -1566,7 +1566,7 @@ static int FIO_compressFilename_dstFile(FIO_ctx_t* const fCtx, if (ress.dstFile == NULL) { closeDstFile = 1; DISPLAYLEVEL(6, "FIO_compressFilename_dstFile: opening dst: %s \n", dstFileName); - ress.dstFile = FIO_openDstFile(prefs, srcFileName, dstFileName); + ress.dstFile = FIO_openDstFile(fCtx, prefs, srcFileName, dstFileName); if (ress.dstFile==NULL) return 1; /* could not open dstFileName */ /* Must only be added after FIO_openDstFile() succeeds. * Otherwise we may delete the destination file if it already exists, @@ -1773,7 +1773,7 @@ int FIO_compressMultipleFilenames(FIO_ctx_t* const fCtx, FIO_freeCResources(ress); return 1; } - ress.dstFile = FIO_openDstFile(prefs, NULL, outFileName); + ress.dstFile = FIO_openDstFile(fCtx, prefs, NULL, outFileName); if (ress.dstFile == NULL) { /* could not open outFileName */ error = 1; } else { @@ -2458,7 +2458,7 @@ static int FIO_decompressDstFile(FIO_ctx_t* const fCtx, if ((ress.dstFile == NULL) && (prefs->testMode==0)) { releaseDstFile = 1; - ress.dstFile = FIO_openDstFile(prefs, srcFileName, dstFileName); + ress.dstFile = FIO_openDstFile(fCtx, prefs, srcFileName, dstFileName); if (ress.dstFile==NULL) return 1; /* Must only be added after FIO_openDstFile() succeeds. @@ -2688,7 +2688,7 @@ FIO_decompressMultipleFilenames(FIO_ctx_t* const fCtx, return 1; } if (!prefs->testMode) { - ress.dstFile = FIO_openDstFile(prefs, NULL, outFileName); + ress.dstFile = FIO_openDstFile(fCtx, prefs, NULL, outFileName); if (ress.dstFile == 0) EXM_THROW(19, "cannot open %s", outFileName); } for (; fCtx->currFileIdx < fCtx->nbFilesTotal; fCtx->currFileIdx++) { diff --git a/programs/util.c b/programs/util.c index b4f1f402e..441ccb235 100644 --- a/programs/util.c +++ b/programs/util.c @@ -90,9 +90,6 @@ int g_utilDisplayLevel; int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, const char* acceptableLetters) { int ch, result; - /* If input is presented via stdin, dont use prompt as it may swallow characters */ - if (!IS_CONSOLE(stdin)) - return 0; UTIL_DISPLAY("%s", prompt); ch = getchar(); From 432186cbea108291dff4785cb2bcc098e03aafb7 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 24 Sep 2020 15:55:30 -0400 Subject: [PATCH 4/8] Add FIO_determineHasStdinInput() function and member to fCtx --- programs/fileio.c | 12 ++++++++++++ programs/fileio.h | 1 + programs/zstdcli.c | 1 + 3 files changed, 14 insertions(+) diff --git a/programs/fileio.c b/programs/fileio.c index b8700f5d4..aa28d05f2 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -330,6 +330,7 @@ struct FIO_ctx_s { /* file i/o info */ int nbFilesTotal; + int hasStdinInput; /* file i/o state */ int currFileIdx; @@ -386,6 +387,7 @@ FIO_ctx_t* FIO_createContext(void) if (!ret) EXM_THROW(21, "Allocation error : not enough memory"); ret->currFileIdx = 0; + ret->hasStdinInput = 0; ret->nbFilesTotal = 1; ret->nbFilesProcessed = 0; ret->totalBytesInput = 0; @@ -539,6 +541,16 @@ void FIO_setNbFilesTotal(FIO_ctx_t* const fCtx, int value) fCtx->nbFilesTotal = value; } +void FIO_determineHasStdinInput(FIO_ctx_t* const fCtx, const FileNamesTable* const filenames) { + int i = 0; + for ( ; i < filenames->tableSize; ++i) { + if (!strcmp(stdinmark, filenames->fileNames[i])) { + fCtx->hasStdinInput = 1; + return; + } + } +} + /*-************************************* * Functions ***************************************/ diff --git a/programs/fileio.h b/programs/fileio.h index bec651a15..866d679d9 100644 --- a/programs/fileio.h +++ b/programs/fileio.h @@ -106,6 +106,7 @@ void FIO_setContentSize(FIO_prefs_t* const prefs, int value); /* FIO_ctx_t functions */ void FIO_setNbFilesTotal(FIO_ctx_t* const fCtx, int value); +void FIO_determineHasStdinInput(FIO_ctx_t* const fCtx, const FileNamesTable* const filenames); /*-************************************* * Single File functions diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 5d1c09de6..3df91c10e 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -1286,6 +1286,7 @@ int main(int const argCount, const char* argv[]) /* IO Stream/File */ FIO_setNbFilesTotal(fCtx, (int)filenames->tableSize); + FIO_determineHasStdinInput(fCtx, filenames); FIO_setNotificationLevel(g_displayLevel); FIO_setPatchFromMode(prefs, patchFromDictFileName != NULL); if (memLimit == 0) { From 93d63eaeb874b1532547fd663bd12090bcb0996d Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 24 Sep 2020 15:58:06 -0400 Subject: [PATCH 5/8] Expand UTIL_requireUserConfirmation to include stdin input check --- programs/fileio.c | 4 ++-- programs/util.c | 5 ++++- programs/util.h | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index aa28d05f2..bfcb58c27 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -662,7 +662,7 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, return NULL; } DISPLAY("zstd: %s already exists; ", dstFileName); - if (UTIL_requireUserConfirmation("overwrite (y/n) ? ", "Not overwritten \n", "yY")) + if (UTIL_requireUserConfirmation("overwrite (y/n) ? ", "Not overwritten \n", "yY", fCtx->hasStdinInput)) return NULL; } /* need to unlink */ @@ -859,7 +859,7 @@ static int FIO_removeMultiFilesWarning(FIO_ctx_t* const fCtx, const FIO_prefs_t* } DISPLAYLEVEL(2, "\nThe concatenated output CANNOT regenerate the original directory tree. ") if (prefs->removeSrcFile) { - error = g_display_prefs.displayLevel > displayLevelCutoff && UTIL_requireUserConfirmation("This is a destructive operation. Proceed? (y/n): ", "Aborting...", "yY"); + error = g_display_prefs.displayLevel > displayLevelCutoff && UTIL_requireUserConfirmation("This is a destructive operation. Proceed? (y/n): ", "Aborting...", "yY", fCtx->hasStdinInput); } } DISPLAY("\n"); diff --git a/programs/util.c b/programs/util.c index 441ccb235..fd31ee9e1 100644 --- a/programs/util.c +++ b/programs/util.c @@ -88,9 +88,12 @@ UTIL_STATIC void* UTIL_realloc(void *ptr, size_t size) int g_utilDisplayLevel; int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, - const char* acceptableLetters) { + const char* acceptableLetters, int hasStdinInput) { int ch, result; + if (hasStdinInput) + return 1; + UTIL_DISPLAY("%s", prompt); ch = getchar(); result = 0; diff --git a/programs/util.h b/programs/util.h index eeb6a15e2..25fa3f53a 100644 --- a/programs/util.h +++ b/programs/util.h @@ -96,8 +96,9 @@ extern int g_utilDisplayLevel; /** * Displays a message prompt and returns success (0) if first character from stdin * matches any from acceptableLetters. Otherwise, returns failure (1) and displays abortMsg. + * If any of the inputs are stdin itself, then automatically return failure (1). */ -int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, const char* acceptableLetters); +int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, const char* acceptableLetters, int hasStdinInput); /*-**************************************** From 88f4410390e9b727bddcc3c0d141cc402d35a553 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 24 Sep 2020 16:29:12 -0400 Subject: [PATCH 6/8] Add more useful failure message when stdin is an input --- programs/util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/programs/util.c b/programs/util.c index fd31ee9e1..f7436be54 100644 --- a/programs/util.c +++ b/programs/util.c @@ -91,8 +91,10 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, const char* acceptableLetters, int hasStdinInput) { int ch, result; - if (hasStdinInput) + if (hasStdinInput) { + UTIL_DISPLAY("Stdin is an input - not proceeding.\n"); return 1; + } UTIL_DISPLAY("%s", prompt); ch = getchar(); From 9f7212a48bcaade4e4e8c6431ed065fe33be5ba1 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 24 Sep 2020 16:44:33 -0400 Subject: [PATCH 7/8] Update unit tests --- programs/fileio.c | 2 +- tests/playTests.sh | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index bfcb58c27..3ee338677 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -542,7 +542,7 @@ void FIO_setNbFilesTotal(FIO_ctx_t* const fCtx, int value) } void FIO_determineHasStdinInput(FIO_ctx_t* const fCtx, const FileNamesTable* const filenames) { - int i = 0; + size_t i = 0; for ( ; i < filenames->tableSize; ++i) { if (!strcmp(stdinmark, filenames->fileNames[i])) { fCtx->hasStdinInput = 1; diff --git a/tests/playTests.sh b/tests/playTests.sh index aee790da5..e55b02f1f 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -311,13 +311,19 @@ println "Test completed" -println "\n===> warning prompt does not swallow characters" +println "\n===> warning prompts should not occur if stdin is an input" println "y" > tmpPrompt println "hello world" >> tmpPrompt -zstd tmpPrompt -zstd < tmpPrompt -o tmpPrompt.zst -zstd -q -d tmpPrompt.zst -o tmpPromptRegenerated -$DIFF tmpPromptRegenerated tmpPrompt +zstd tmpPrompt -f +zstd < tmpPrompt -o tmpPrompt.zst && die "should have aborted immediately and failed to overwrite" +zstd < tmpPrompt -o tmpPrompt.zst -f # should successfully overwrite with -f +zstd -q -d -f tmpPrompt.zst -o tmpPromptRegenerated +$DIFF tmpPromptRegenerated tmpPrompt # the first 'y' character should not be swallowed + +echo 'yes' | zstd tmpPrompt -o tmpPrompt.zst # accept piped "y" input to force overwrite when using files +echo 'yes' | zstd < tmpPrompt -o tmpPrompt.zst && die "should have aborted immediately and failed to overwrite" +zstd tmpPrompt - < tmpPrompt -o tmpPromp.zst --rm && die "should have aborted immediately and failed to remove" + println "Test completed" From 02422db84148799fc7b9242f5bde486a5ff224a8 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Fri, 25 Sep 2020 11:51:35 -0400 Subject: [PATCH 8/8] Fix Stdin typo --- programs/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/util.c b/programs/util.c index f7436be54..980ab5a42 100644 --- a/programs/util.c +++ b/programs/util.c @@ -92,7 +92,7 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, int ch, result; if (hasStdinInput) { - UTIL_DISPLAY("Stdin is an input - not proceeding.\n"); + UTIL_DISPLAY("stdin is an input - not proceeding.\n"); return 1; }