git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 8/9] t: validate 'test_i18ngrep's parameters

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Feb 8, 2018, 15:56 UTC
Message-ID
<20180208155656.9831-9-szeder.dev@gmail.com>
In-Reply-To
<20180208155656.9831-1-szeder.dev@gmail.com>

Some of the previous patches in this series fixed bogus 'test_i18ngrep' invocations:

  - Two invocations where the tested git command's standard output is
    directly piped into 'test_i18ngrep'.  While convenient, this is an
    antipattern, because the pipe hides the git command's exit code,
    and the test could continue even if the command exited with error.
  - Two invocations that had neither a filename parameter nor anything
    piped into their standard input, yet both managed to remain
    unnoticed for years.  A third similarly bogus invocation is
    currently lurking in 'pu' for a couple of weeks now.

Prevent similar mistakes in the future by validating 'test_i18ngrep's parameters requiring that

  - The last parameter names an existing file to be read, effectively
    forbiding piping into 'test_i18ngrep'.
    Note that this change will also forbid cases where 'test_i18ngrep'
    would legitimately read its standard input, e.g. when its standard
    input is redirected from a file, or when a git command's standard
    output is first written to an intermediate file, which is then
    preprocessed by a non-git command before the results are piped
    into 'test_i18ngrep'.  See two of the previous patches for the
    only such cases we had in our test suite.  However, reliably
    preventing the piping antipattern is arguably more important than
    supporting these cases, which can be easily worked around by
    opening the file directly or using an intermediate file anyway.
  - There are at least two parameters, not including the optional '!'
    to negate the pattern.  This ought to catch corner cases when
    'test_i18ngrep' looks for the name of an existing file on its
    standard input; the above check would miss this case becase the
    filename as pattern would be the last parameter.
    Note that this is not quite perfect, as it doesn't account for any
    'grep --options' given as parameters.  However, doing so would be
    far too complicated, considering that patterns can start with
    dashes as well, and in the majority of the cases we don't use any
    such options anyway.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 t/test-lib-functions.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 92ed029371..a1676e0386 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -719,6 +719,18 @@ test_i18ncmp () {
 # under GETTEXT_POISON this pretends that the command produced expected
 # results.
 test_i18ngrep () {
+	eval "last_arg=\"\${$#}\""
+
+	test -f "$last_arg" ||
+	error "bug in the test script: test_i18ngrep requires a file" \
+	      "to read as the last parameter"
+
+	if test $# -lt 2 ||
+	   { test "x!" = "x$1" && test $# -lt 3 ; }
+	then
+		error "bug in the test script: too few parameters to test_i18ngrep"
+	fi
+
 	if test -n "$GETTEXT_POISON"
 	then
 	    : # pretend success
-- 
2.16.1.158.ge6451079d
Previous: SZEDER GáborNext: Jeff King
Message 41 of 49 in “'test_i18ngrep'-related fixes and improvements”
  1. 00/10 'test_i18ngrep'-related fixes and improvementsSZEDER Gábor, Jan 26, 2018
  2. 01/10 t5541: add 'test_i18ngrep's missing filename parameterSZEDER Gábor, Jan 26, 2018
  3. Jeff KingJan 26, 2018
  4. Jeff KingJan 26, 2018
  5. 04/10 t4001: don't run 'git status' upstream of a pipeSZEDER Gábor, Jan 26, 2018
  6. 07/10 t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh'SZEDER Gábor, Jan 26, 2018
  7. Junio C HamanoJan 26, 2018
  8. Jeff KingJan 26, 2018
  9. SZEDER GáborJan 26, 2018
  10. 10/10 t: make 'test_i18ngrep' more informative on failureSZEDER Gábor, Jan 26, 2018
  11. Jeff KingJan 26, 2018
  12. SZEDER GáborJan 26, 2018
  13. Jeff KingJan 26, 2018
  14. SZEDER GáborJan 26, 2018
  15. Jeff KingJan 26, 2018
  16. 06/10 t5536: let 'test_i18ngrep' read the file without redirectionSZEDER Gábor, Jan 26, 2018
  17. 09/10 t: make sure that 'test_i18ngrep' got enough parametersSZEDER Gábor, Jan 26, 2018
  18. Jeff KingJan 26, 2018
  19. Eric SunshineJan 26, 2018
  20. 05/10 t5510: consolidate 'grep' and 'test_i18ngrep' patternsSZEDER Gábor, Jan 26, 2018
  21. Junio C HamanoJan 26, 2018
  22. SZEDER GáborJan 26, 2018
  23. Junio C HamanoJan 26, 2018
  24. 08/10 t: forbid piping into 'test_i18ngrep'SZEDER Gábor, Jan 26, 2018
  25. Junio C HamanoJan 26, 2018
  26. Junio C HamanoJan 26, 2018
  27. Jeff KingJan 26, 2018
  28. SZEDER GáborJan 26, 2018
  29. Junio C HamanoJan 26, 2018
  30. Jeff KingJan 26, 2018
  31. 02/10 t5812: add 'test_i18ngrep's missing filename parameterSZEDER Gábor, Jan 26, 2018
  32. Jeff KingJan 26, 2018
  33. SZEDER GáborFeb 7, 2018
  34. Jeff KingFeb 7, 2018
  35. Simon RuderichJan 30, 2018
  36. 03/10 t6022: don't run 'git merge' upstream of a pipeSZEDER Gábor, Jan 26, 2018
  37. Jeff KingJan 26, 2018
  38. 0/9 'test_i18ngrep'-related fixes and improvementsSZEDER Gábor, Feb 8, 2018
  39. 1/9 t5541: add 'test_i18ngrep's missing filename parameterSZEDER Gábor, Feb 8, 2018
  40. 4/9 t4001: don't run 'git status' upstream of a pipeSZEDER Gábor, Feb 8, 2018
  41. 8/9 t: validate 'test_i18ngrep's parametersSZEDER Gábor, Feb 8, 2018
  42. Jeff KingFeb 8, 2018
  43. 9/9 t: make 'test_i18ngrep' more informative on failureSZEDER Gábor, Feb 8, 2018
  44. 5/9 t5510: consolidate 'grep' and 'test_i18ngrep' patternsSZEDER Gábor, Feb 8, 2018
  45. 6/9 t5536: let 'test_i18ngrep' read the file without redirectionSZEDER Gábor, Feb 8, 2018
  46. 7/9 t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh'SZEDER Gábor, Feb 8, 2018
  47. 2/9 t5812: add 'test_i18ngrep's missing filename parameterSZEDER Gábor, Feb 8, 2018
  48. 3/9 t6022: don't run 'git merge' upstream of a pipeSZEDER Gábor, Feb 8, 2018
  49. Jeff KingFeb 8, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.