[PATCH] Don't process spoken messages with the shell in sd_generic.

[Christopher Brannon]

This patch changes the way that data is sent to a synthesizer program
by the generic module.
The command defined by GenericExecuteSynth should now expect its
data on standard input, rather than its command line.
This guarantees that no inadvertent shell substitution is performed
on the incoming data.
---
 config/modules/dtk-generic.conf           |    2 +-
 config/modules/epos-generic.conf          |    2 +-
 config/modules/espeak-generic.conf        |    2 +-
 config/modules/espeak-mbrola-generic.conf |    2 +-
 config/modules/llia_phon-generic.conf     |    2 +-
 config/modules/swift-generic.conf         |    2 +-
 doc/speech-dispatcher.texi                |    7 +--
 src/modules/generic.c                     |   82 ++++++++++++++--------------
 8 files changed, 49 insertions(+), 52 deletions(-)

diff --git a/config/modules/dtk-generic.conf b/config/modules/dtk-generic.conf
index f485e93..2bfdeb3 100644
--- a/config/modules/dtk-generic.conf
+++ b/config/modules/dtk-generic.conf
@@ -26,7 +26,7 @@
 # I know of to fix this is to upgrade from DECTalk 4.61 to DECTalk 5.
 GenericExecuteSynth \
 "echo \"[:n$VOICE][:ra $RATE][:dv ap $PITCH]\" >$TMPDIR/dtk-speak.txt \
-&& echo \'$DATA\' | fmt >>$TMPDIR/dtk-speak.txt && say -fi 
$TMPDIR/dtk-speak.txt"
+&& fmt >>$TMPDIR/dtk-speak.txt && say -fi $TMPDIR/dtk-speak.txt"
 
 # GenericStripPunctChars is a list (enclosed in doublequotes) of
 # all the characters that should be replaced by whitespaces in
diff --git a/config/modules/epos-generic.conf b/config/modules/epos-generic.conf
index 8c327e3..6066d89 100644
--- a/config/modules/epos-generic.conf
+++ b/config/modules/epos-generic.conf
@@ -20,7 +20,7 @@
 
 GenericExecuteSynth \
 "epos-say -o --language $LANGUAGE --voice $VOICE --init_f $PITCH --init_t 
$RATE \
-\'$DATA\' | sed -e /unknown*/d >$TMPDIR/epos-said.wav && play 
$TMPDIR/epos-said.wav >/dev/null"
+\"$(cat)\" | sed -e /unknown*/d >$TMPDIR/epos-said.wav && play 
$TMPDIR/epos-said.wav >/dev/null"
 
 # GenericStripPunctChars is a list (enclosed in doublequotes) of
 # all the characters that should be replaced by whitespaces in
diff --git a/config/modules/espeak-generic.conf 
b/config/modules/espeak-generic.conf
index d76c03a..cabfb84 100644
--- a/config/modules/espeak-generic.conf
+++ b/config/modules/espeak-generic.conf
@@ -20,7 +20,7 @@
 # can modify this value, see other parameters).
 # The command can be split into more lines, if necessary, using '\'.
 GenericExecuteSynth \
-"echo \'$DATA\' | espeak -w $TMPDIR/espeak.wav -v $VOICE -s $RATE -a $VOLUME 
-p $PITCH $PUNCT --stdin && $PLAY_COMMAND $TMPDIR/espeak.wav"
+"espeak -w $TMPDIR/espeak.wav -v $VOICE -s $RATE -a $VOLUME -p $PITCH $PUNCT 
--stdin && $PLAY_COMMAND $TMPDIR/espeak.wav"
 
 # The following three items control punctuation levels None, Some, and All.
 # Each of these values will be substituted into the $PUNCT variable depending
diff --git a/config/modules/espeak-mbrola-generic.conf 
b/config/modules/espeak-mbrola-generic.conf
index 7bcc5c6..87724d9 100644
--- a/config/modules/espeak-mbrola-generic.conf
+++ b/config/modules/espeak-mbrola-generic.conf
@@ -18,7 +18,7 @@
 # can modify this value, see other parameters).
 # The command can be split into more lines, if necessary, using '\'.
 GenericExecuteSynth \
-"echo \'$DATA\' | espeak -v mb-$VOICE $PUNCT -s $RATE -a $VOLUME | mbrola -v 
$VOLUME -e /usr/share/mbrola/$VOICE/$VOICE - -.au | $PLAY_COMMAND
+"espeak -v mb-$VOICE $PUNCT -s $RATE -a $VOLUME | mbrola -v $VOLUME -e 
/usr/share/mbrola/$VOICE/$VOICE - -.au | $PLAY_COMMAND
 
 # The following three items control punctuation levels None, Some, and All.
 # Each of these values will be substituted into the $PUNCT variable depending
diff --git a/config/modules/llia_phon-generic.conf 
b/config/modules/llia_phon-generic.conf
index 3301637..cae3440 100644
--- a/config/modules/llia_phon-generic.conf
+++ b/config/modules/llia_phon-generic.conf
@@ -18,7 +18,7 @@
 # The command can be split into more lines, if necessary, using '\'.
 
 GenericExecuteSynth \
-"echo \'$DATA\' > $TMPDIR/llia_phon.txt && llia_phon | mbrola --f $PITCH --t 
$RATE \
+"cat > $TMPDIR/llia_phon.txt && llia_phon | mbrola --f $PITCH --t $RATE \
 -e -I LIAPHON/data/noarch/initfile.lia (directory with the voice) $VOICE \
 $TMPDIR/llia_phon.txt -.au | $PLAY_COMMAND -t au - >/dev/null"
 
diff --git a/config/modules/swift-generic.conf 
b/config/modules/swift-generic.conf
index 1623559..d52fda7 100644
--- a/config/modules/swift-generic.conf
+++ b/config/modules/swift-generic.conf
@@ -23,7 +23,7 @@
 # can modify this value, see other parameters).
 # The command can be split into more lines, if necessary, using '\'.
 GenericExecuteSynth \
- "echo \'$DATA\' >/tmp/swift-speak.txt && /opt/swift/bin/swift -p 
speech/rate=$RATE,speech/pitch/shift=$PITCH,tts/content-type=text/plain,tts/text-encoding=utf-8,config/default-voice=$VOICE
 -f /tmp/swift-speak.txt -o /tmp/swift-speak.wav&& $PLAY_COMMAND 
/tmp/swift-speak.wav" 
+ "cat >/tmp/swift-speak.txt && /opt/swift/bin/swift -p 
speech/rate=$RATE,speech/pitch/shift=$PITCH,tts/content-type=text/plain,tts/text-encoding=utf-8,config/default-voice=$VOICE
 -f /tmp/swift-speak.txt -o /tmp/swift-speak.wav&& $PLAY_COMMAND 
/tmp/swift-speak.wav" 
 
 # GenericStripPunctChars is a list (enclosed in doublequotes) of
 # all the characters that should be replaced by whitespaces in
diff --git a/doc/speech-dispatcher.texi b/doc/speech-dispatcher.texi
index ff609ca..0a9fa2b 100644
--- a/doc/speech-dispatcher.texi
+++ b/doc/speech-dispatcher.texi
@@ -862,16 +862,13 @@ saying the message, the output module will send a KILL 
signal to
 the process group, so it's important that it immediately
 stops speaking after the processes are killed. (On most GNU/Linux
 system, the @code{play} utility has this property).
+The text to be spoken is written to the command's standard input.
 
 In the execution string, you can use the following variables,
 which will be substituted by the desired values before executing
 the command.
 
 @itemize
- at item @code{$DATA}
-The text data that should be said. The string's characters that would interfere
-with bash processing are already escaped. However, it may be necessary to put
-double quotes around it (like this: @code{\"$DATA\"}).
 @item @code{$LANG}
 The language identification string (it's defined by GenericLanguage).
 @item @code{$VOICE}
@@ -886,7 +883,7 @@ Here is an example from 
@file{etc/speech-dispatcher/modules/epos-generic.conf}
 @example
 GenericExecuteSynth \
 "epos-say -o --language $LANG --voice $VOICE --init_f $PITCH --init_t $RATE \
-\"$DATA\" | sed -e s+unknown.*$++ >/tmp/epos-said.wav && play 
/tmp/epos-said.wav >/dev/null"
+\"$(cat)\" | sed -e s+unknown.*$++ >/tmp/epos-said.wav && play 
/tmp/epos-said.wav >/dev/null"
 @end example
 @end defvr
 
diff --git a/src/modules/generic.c b/src/modules/generic.c
index 2b615ff..ad1cab0 100644
--- a/src/modules/generic.c
+++ b/src/modules/generic.c
@@ -47,8 +47,11 @@ static EMessageType generic_message_type;
 static int generic_position = 0;
 static int generic_pause_requested = 0;
 
-static char *execute_synth_str1;
-static char *execute_synth_str2;
+static char *synth_command;
+
+/* For ignoring SIGPIPE in the child process. */
+static struct sigaction child_sigaction;
+
 
 /* Internal functions prototypes */
 static void* _generic_speak(void*);
@@ -369,7 +372,6 @@ _generic_speak(void* nothing)
         case 0:
             {
                char *e_string;
-               char *p;               
                char *tmpdir, *homedir;
                const char *helper;
                char *play_command;
@@ -429,16 +431,8 @@ _generic_speak(void* nothing)
                else
                    e_string = string_replace(e_string, "$VOICE", "no_voice");
 
-               /* Cut it into two strings */           
-               p = strstr(e_string, "$DATA");
-               if (p == NULL) exit(1);
-               *p = 0;
-               execute_synth_str1 = strdup(e_string);
-               execute_synth_str2 = strdup(p + (strlen("$DATA")));
-               
-               free(e_string);
-               
-               /* execute_synth_str1 se sem musi nejak dostat */
+               synth_command = e_string;
+
                DBG("Starting child...\n");
                _generic_child(module_pipe, GenericMaxChunkLength);
             }
@@ -480,10 +474,10 @@ _generic_child(TModuleDoublePipe dpipe, const size_t 
maxlen)
     sigset_t some_signals;
     int bytes;
     char *command;
-    GString *message;
-    int i;
     int ret;
 
+    child_sigaction.sa_handler = SIG_IGN;
+    sigaction(SIGPIPE, &child_sigaction, NULL);
     sigfillset(&some_signals);
     module_sigunblockusr(&some_signals);
 
@@ -503,42 +497,48 @@ _generic_child(TModuleDoublePipe dpipe, const size_t 
maxlen)
         text[bytes] = 0;
         DBG("text read is: |%s|\n", text);
 
-        /* Escape any quotes */
-        message = g_string_new("");
-        for(i=0; i<=bytes-1; i++){
-            if (text[i] == '\"')
-                message = g_string_append(message, "\\\"");
-            else if (text[i] == '`')
-                message = g_string_append(message, "\\`");
-            else if (text[i] == '\\')
-                message = g_string_append(message, "\\\\");
-           else{
-                g_string_append_printf(message, "%c", text[i]);
-            }
-        }
-
-        DBG("child: escaped text is |%s|", message->str);
-
-        command = malloc((strlen(message->str)+strlen(execute_synth_str1)+
-                          strlen(execute_synth_str2) + 8) * sizeof(char));
-
-        if (strlen(message->str) != 0){
-            sprintf(command, "%s%s%s", execute_synth_str1, message->str, 
execute_synth_str2);
-
+        if (strlen(text) != 0){
+            char *text_pos = text;
             DBG("child: synth command = |%s|", command);
 
             DBG("Speaking in child...");
             module_sigblockusr(&some_signals);        
             {
-               ret = system(command);
-              DBG("Executed shell command returned with %d", ret);
+              FILE *synth_stdin = popen(synth_command, "w");
+
+              if (synth_stdin != NULL) {
+                int errors = 0;
+                int at_eof = 0;
+
+                while ((bytes > 0) && !errors && !at_eof) {
+                 size_t bytes_written = fwrite(text_pos, 1,
+                                                bytes, synth_stdin);
+
+                  if (bytes_written == 0) {
+                    if (ferror(synth_stdin)) {
+                      errors = 1;
+                      DBG("Cannot write data to module.");
+                    }
+                    else
+                      at_eof = 1;
+                  }
+                  else {
+                    bytes -= bytes_written;
+                   text_pos += bytes_written;
+                 }
+                }
+
+                ret = pclose(synth_stdin);
+                DBG("Executed shell command returned with %d", ret);
+              }
+              else
+                DBG("Unable to spawn child process.");
             }
         }
         module_sigunblockusr(&some_signals);        
 
-        xfree(command);
         xfree(text);
-        g_string_free(message, 1);
+
 
         DBG("child->parent: ok, send more data");      
         module_child_dp_write(dpipe, "C", 1);
-- 
1.7.2.1