[RFC/PATCH 1/5] gettext: fix bug in git-sh-i18n's eval_gettext() by using envsubst(1)

From

Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Date

Oct 31, 2010, 11:34 UTC

Message-ID

<1288524860-538-2-git-send-email-avarab@gmail.com>

In-Reply-To

<AANLkTikOgMGqw5fc95c2VGwXxKu9rmsA+=z5_jykD92=@mail.gmail.com>

    cmd="git foo"; eval_gettext "command: \$cmd"

 * Added our git-compat-util.h header for xrealloc() and friends.

 * Removed inclusion of gettext-specific headers.

 * Removed most of main() and replaced it with my own. The modified
   version doesn't do option parsing at all, because it doesn't need
   to.

 * Modified error() invocations to use our error() instead of
   error(3).

 * Replaced the gettext XNMALLOC(n, size) macro with just
   xmalloc(n). Since XNMALLOC() only allocated char's.

 * Removed functions made redundant since I deleted some code paths
   from main(). These were:

    * print_variables
    * note_variables
    * print_variable
    * string_list_init
    * cmp_string
    * string_list_sort
    * string_list_destroy
    * find_variables
    * note_variable
    * note_variables

 * Replaced the use of stdbool.h (a C99 header) by doing the following
   replacements on the code:

    * s/bool/unsigned short int/g
    * s/true/1/g
    * s/false/0/g

Reported-by: Johannes Sixt <j.sixt@viscovery.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .gitignore                   |    1 +
 Makefile                     |    1 +
 git-sh-i18n.sh               |    7 +-
 sh-i18n--envsubst.c          |  303 ++++++++++++++++++++++++++++++++++++++++++
 t/lib-gettext.sh             |   25 ++++
 t/t0200-gettext-basic.sh     |    3 +
 t/t0201-gettext-fallbacks.sh |    3 +
 7 files changed, 338 insertions(+), 5 deletions(-)
 create mode 100644 sh-i18n--envsubst.c

diff --git a/.gitignore b/.gitignore
index 80ca718..d2d9ec2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -126,6 +126,7 @@
 /git-send-email
 /git-send-pack
 /git-sh-i18n
+/git-sh-i18n--envsubst
 /git-sh-setup
 /git-shell
 /git-shortlog
diff --git a/Makefile b/Makefile
index e1650ba..c55baa6 100644
--- a/Makefile
+++ b/Makefile
@@ -435,6 +435,7 @@ PROGRAM_OBJS += shell.o
 PROGRAM_OBJS += show-index.o
 PROGRAM_OBJS += upload-pack.o
 PROGRAM_OBJS += http-backend.o
+PROGRAM_OBJS += sh-i18n--envsubst.o
 
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index f8dd43a..41580a8 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -38,9 +38,7 @@ then
 
 		# Solaris has a gettext(1) but no eval_gettext(1)
 		eval_gettext () {
-			gettext_out=$(gettext "$1")
-			gettext_eval="printf '%s' \"$gettext_out\""
-			printf "%s" "`eval \"$gettext_eval\"`"
+			gettext "$1" | git sh-i18n--envsubst
 		}
 	else
 		# Since gettext.sh isn't available we'll have to define our own
@@ -55,8 +53,7 @@ then
 		}
 
 		eval_gettext () {
-			gettext_eval="printf '%s' \"$1\""
-			printf "%s" "`eval \"$gettext_eval\"`"
+			printf "%s" "$1" | git sh-i18n--envsubst
 		}
 	fi
 else
diff --git a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c
new file mode 100644
index 0000000..d0e0cc9
--- /dev/null
+++ b/sh-i18n--envsubst.c
@@ -0,0 +1,303 @@
+/*
+ * sh-i18n--envsubst.c - a stripped-down version of gettext's envsubst(1)
+ *
+ * Copyright (C) 2010 Ævar Arnfjörð Bjarmason
+ *
+ * This is a modified version of
+ * 67d0871a8c:gettext-runtime/src/envsubst.c from the gettext.git
+ * repository. It has been stripped down to only implement the
+ * envsubst(1) features that we need in the git-sh-i18n fallbacks.
+ *
+ * The "Close standard error" part in main() is from
+ * 8dac033df0:gnulib-local/lib/closeout.c. The copyright notices for
+ * both files are reproduced immediately below.
+ */
+
+#include "git-compat-util.h"
+
+/* Substitution of environment variables in shell format strings.
+   Copyright (C) 2003-2007 Free Software Foundation, Inc.
+   Written by Bruno Haible <bruno@clisp.org>, 2003.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software Foundation,
+   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
+
+/* closeout.c - close standard output and standard error
+   Copyright (C) 1998-2007 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software Foundation,
+   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
+
+#include <errno.h>
+#include <getopt.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+/* If true, substitution shall be performed on all variables.  */
+static unsigned short int all_variables;
+
+static void subst_from_stdin (void);
+
+int
+main (int argc, char *argv[])
+{
+  all_variables = 1;
+
+  if (argc > 1)
+	{
+	  error ("too many arguments");
+	  exit (EXIT_FAILURE);
+	}
+
+  subst_from_stdin ();
+
+  /* Close standard error.  This is simpler than fwriteerror_no_ebadf, because
+     upon failure we don't need an errno - all we can do at this point is to
+     set an exit status.  */
+  errno = 0;
+  if (ferror (stderr) || fflush (stderr))
+    { 
+      fclose (stderr);
+      exit (EXIT_FAILURE);
+    }
+  if (fclose (stderr) && errno != EBADF)
+    exit (EXIT_FAILURE);
+
+  exit (EXIT_SUCCESS);
+}
+
+/* Type describing list of immutable strings,
+   implemented using a dynamic array.  */
+typedef struct string_list_ty string_list_ty;
+struct string_list_ty
+{
+  const char **item;
+  size_t nitems;
+  size_t nitems_max;
+};
+
+/* Append a single string to the end of a list of strings.  */
+static inline void
+string_list_append (string_list_ty *slp, const char *s)
+{
+  /* Grow the list.  */
+  if (slp->nitems >= slp->nitems_max)
+    {
+      size_t nbytes;
+
+      slp->nitems_max = slp->nitems_max * 2 + 4;
+      nbytes = slp->nitems_max * sizeof (slp->item[0]);
+      slp->item = (const char **) xrealloc (slp->item, nbytes);
+    }
+
+  /* Add the string to the end of the list.  */
+  slp->item[slp->nitems++] = s;
+}
+
+/* Test whether a string list contains a given string.  */
+static inline int
+string_list_member (const string_list_ty *slp, const char *s)
+{
+  size_t j;
+
+  for (j = 0; j < slp->nitems; ++j)
+    if (strcmp (slp->item[j], s) == 0)
+      return 1;
+  return 0;
+}
+
+/* Test whether a sorted string list contains a given string.  */
+static int
+sorted_string_list_member (const string_list_ty *slp, const char *s)
+{
+  size_t j1, j2;
+
+  j1 = 0;
+  j2 = slp->nitems;
+  if (j2 > 0)
+    {
+      /* Binary search.  */
+      while (j2 - j1 > 1)
+	{
+	  /* Here we know that if s is in the list, it is at an index j
+	     with j1 <= j < j2.  */
+	  size_t j = (j1 + j2) >> 1;
+	  int result = strcmp (slp->item[j], s);
+
+	  if (result > 0)
+	    j2 = j;
+	  else if (result == 0)
+	    return 1;
+	  else
+	    j1 = j + 1;
+	}
+      if (j2 > j1)
+	if (strcmp (slp->item[j1], s) == 0)
+	  return 1;
+    }
+  return 0;
+}
+
+
+/* Set of variables on which to perform substitution.
+   Used only if !all_variables.  */
+static string_list_ty variables_set;
+
+static int
+do_getc ()
+{
+  int c = getc (stdin);
+
+  if (c == EOF)
+    {
+      if (ferror (stdin))
+	error ("error while reading standard input");
+    }
+
+  return c;
+}
+
+static inline void
+do_ungetc (int c)
+{
+  if (c != EOF)
+    ungetc (c, stdin);
+}
+
+/* Copies stdin to stdout, performing substitutions.  */
+static void
+subst_from_stdin ()
+{
+  static char *buffer;
+  static size_t bufmax;
+  static size_t buflen;
+  int c;
+
+  for (;;)
+    {
+      c = do_getc ();
+      if (c == EOF)
+	break;
+      /* Look for $VARIABLE or ${VARIABLE}.  */
+      if (c == '$')
+	{
+	  unsigned short int opening_brace = 0;
+	  unsigned short int closing_brace = 0;
+
+	  c = do_getc ();
+	  if (c == '{')
+	    {
+	      opening_brace = 1;
+	      c = do_getc ();
+	    }
+	  if ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_')
+	    {
+	      unsigned short int valid;
+
+	      /* Accumulate the VARIABLE in buffer.  */
+	      buflen = 0;
+	      do
+		{
+		  if (buflen >= bufmax)
+		    {
+		      bufmax = 2 * bufmax + 10;
+		      buffer = xrealloc (buffer, bufmax);
+		    }
+		  buffer[buflen++] = c;
+
+		  c = do_getc ();
+		}
+	      while ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z')
+		     || (c >= '0' && c <= '9') || c == '_');
+
+	      if (opening_brace)
+		{
+		  if (c == '}')
+		    {
+		      closing_brace = 1;
+		      valid = 1;
+		    }
+		  else
+		    {
+		      valid = 0;
+		      do_ungetc (c);
+		    }
+		}
+	      else
+		{
+		  valid = 1;
+		  do_ungetc (c);
+		}
+
+	      if (valid)
+		{
+		  /* Terminate the variable in the buffer.  */
+		  if (buflen >= bufmax)
+		    {
+		      bufmax = 2 * bufmax + 10;
+		      buffer = xrealloc (buffer, bufmax);
+		    }
+		  buffer[buflen] = '\0';
+
+		  /* Test whether the variable shall be substituted.  */
+		  if (!all_variables
+		      && !sorted_string_list_member (&variables_set, buffer))
+		    valid = 0;
+		}
+
+	      if (valid)
+		{
+		  /* Substitute the variable's value from the environment.  */
+		  const char *env_value = getenv (buffer);
+
+		  if (env_value != NULL)
+		    fputs (env_value, stdout);
+		}
+	      else
+		{
+		  /* Perform no substitution at all.  Since the buffered input
+		     contains no other '$' than at the start, we can just
+		     output all the buffered contents.  */
+		  putchar ('$');
+		  if (opening_brace)
+		    putchar ('{');
+		  fwrite (buffer, buflen, 1, stdout);
+		  if (closing_brace)
+		    putchar ('}');
+		}
+	    }
+	  else
+	    {
+	      do_ungetc (c);
+	      putchar ('$');
+	      if (opening_brace)
+		putchar ('{');
+	    }
+	}
+      else
+	putchar (c);
+    }
+}
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index c9a079e..436192a 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -66,3 +66,28 @@ else
 		say "# lib-gettext: No GETTEXT support available"
 	fi
 fi
+
+test_eval_gettext_interpolation() {
+	test_expect_success NO_GETTEXT_POISON 'eval_gettext: our eval_gettext() fallback can interpolate whitespace variables' '
+	    git_am_cmdline="git am" &&
+	    export git_am_cmdline &&
+	    printf "test git am" >expect &&
+	    eval_gettext "test \$git_am_cmdline" >actual &&
+	    test_cmp expect actual
+	'
+
+	test_expect_success NO_GETTEXT_POISON 'eval_gettext: git am $cmdline bug' '
+	    cmdline="git am -3" &&
+	    export cmdline &&
+	    cat >expect <<EOF &&
+When you have resolved this problem run "git am -3 --resolved".
+If you would prefer to skip this patch, instead run "git am -3 --skip".
+To restore the original branch and stop patching run "git am -3 --abort".
+EOF
+	    eval_gettext "When you have resolved this problem run \"\$cmdline --resolved\".
+If you would prefer to skip this patch, instead run \"\$cmdline --skip\".
+To restore the original branch and stop patching run \"\$cmdline --abort\"." >actual &&
+	    echo >>actual &&
+	    test_cmp expect actual
+	'
+}
diff --git a/t/t0200-gettext-basic.sh b/t/t0200-gettext-basic.sh
index 8853d8a..58948fa 100755
--- a/t/t0200-gettext-basic.sh
+++ b/t/t0200-gettext-basic.sh
@@ -105,4 +105,7 @@ test_expect_success GETTEXT_LOCALE 'sanity: Some gettext("") data for real local
     test -s real-locale
 '
 
+# Test eval_gettext() interpolation with the actual eval_gettext function
+test_eval_gettext_interpolation
+
 test_done
diff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh
index 7a85d9b..6d18e21 100755
--- a/t/t0201-gettext-fallbacks.sh
+++ b/t/t0201-gettext-fallbacks.sh
@@ -46,4 +46,7 @@ test_expect_success NO_GETTEXT_POISON 'eval_gettext: our eval_gettext() fallback
     test_cmp expect actual
 '
 
+# Test eval_gettext() interpolation with the fallback eval_gettext function
+test_eval_gettext_interpolation
+
 test_done

-- 
1.7.3.2.312.ge13a7