From 467c530e65f5f8fc500b33a0dac94474f1edd05f Mon Sep 17 00:00:00 2001 From: Olly Betts Date: Mon, 7 Feb 2022 09:47:01 +1300 Subject: [PATCH] Remove support for the "command" encoder The "command" encoder was mostly intended for use in `%rename` - most uses can be achieved using the "regex" encoder, so we recommend using that instead. The "command" encoder suffers from a number of issues - as the documentation for it admitted, "[it] is extremely slow compared to all the other [encoders] as it involves spawning a separate process and using it for many declarations is not recommended" and that it "should generally be avoided because of performance considerations". But it's also not portable. The design assumes that `/bin/sh` supports `<<<` but that's a bash-specific feature so it doesn't work on platforms where `/bin/sh` is not bash - it fails on Debian, Ubuntu and probably some other Linux distros, plus most non-Linux platforms. Microsoft Windows doesn't even have a /bin/sh as standard. Finally, no escaping of the passed string is done, so it has potential security issues (though at least with %rename the input is limited to valid C/C++ symbol names). Fixes #1806 --- CHANGES.current | 23 +++++++++++ Doc/Manual/SWIG.html | 15 +------ .../test-suite/errors/swig_command_encoder.i | 6 +++ .../errors/swig_command_encoder.stderr | 1 + Examples/test-suite/rename_camel.i | 8 +--- Source/Swig/misc.c | 39 ++----------------- 6 files changed, 36 insertions(+), 56 deletions(-) create mode 100644 Examples/test-suite/errors/swig_command_encoder.i create mode 100644 Examples/test-suite/errors/swig_command_encoder.stderr diff --git a/CHANGES.current b/CHANGES.current index d647226a5..cfa441625 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,29 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.1.0 (in progress) =========================== +2022-02-07: olly + #1806 Remove support for the "command" encoder, which was mostly + intended for use in `%rename` - most uses can be achieved using + the "regex" encoder, so we recommend using that instead. + + The "command" encoder suffers from a number of issues - as the + documentation for it admitted, "[it] is extremely slow compared to + all the other [encoders] as it involves spawning a separate process + and using it for many declarations is not recommended" and that it + "should generally be avoided because of performance + considerations". + + But it's also not portable. The design assumes that `/bin/sh` + supports `<<<` but that's a bash-specific feature so it doesn't + work on platforms where `/bin/sh` is not bash - it fails on + Debian, Ubuntu and probably some other Linux distros, plus most + non-Linux platforms. Microsoft Windows doesn't even have a + /bin/sh as standard. + + Finally, no escaping of the passed string is done, so it has + potential security issues (though at least with %rename the input + is limited to valid C/C++ symbol names). + 2022-02-06: olly #2193 -DFOO on the SWIG command line now sets FOO to 1 for consistency with C/C++ compiler preprocessors. Previously diff --git a/Doc/Manual/SWIG.html b/Doc/Manual/SWIG.html index 6ad6c6770..33cba6d23 100644 --- a/Doc/Manual/SWIG.html +++ b/Doc/Manual/SWIG.html @@ -2064,23 +2064,10 @@ and a more descriptive one, but the two functions are otherwise equivalent: %rename("regex:/(\\w+)_(.*)/\\u\\2/") prefix_printPrint - - command:cmd - Output of an external command cmd with the string passed to - it as input. Notice that this function is extremely slow compared to all - the other ones as it involves spawning a separate process and using it for - many declarations is not recommended. The cmd is not enclosed in - square brackets but must be terminated with a triple '<' sign, - e.g. %rename("command:tr -d aeiou <<<") - (nonsensical example removing all vowels) - PrintPrnt -

-The most general function of all of the above ones (not counting -command which is even more powerful in principle but which should -generally be avoided because of performance considerations) is the +The most general function of all of the above ones is the regex one. Here are some more examples of its use:

diff --git a/Examples/test-suite/errors/swig_command_encoder.i b/Examples/test-suite/errors/swig_command_encoder.i new file mode 100644 index 000000000..aaae82bb5 --- /dev/null +++ b/Examples/test-suite/errors/swig_command_encoder.i @@ -0,0 +1,6 @@ +%module xxx + +// This feature was removed in SWIG 4.1.0 so check it gives an error. +%define SedCmd "%(command:sed -e 's/\([a-z]\)/\U\\1/' -e 's/\(_\)\([a-z]\)/\U\\2/g' <<<)s" %enddef +%rename(SedCmd) ""; +int x; diff --git a/Examples/test-suite/errors/swig_command_encoder.stderr b/Examples/test-suite/errors/swig_command_encoder.stderr new file mode 100644 index 000000000..0fcf2dfb8 --- /dev/null +++ b/Examples/test-suite/errors/swig_command_encoder.stderr @@ -0,0 +1 @@ +SWIG:1: Error: Command encoder no longer supported - use regex encoder instead. diff --git a/Examples/test-suite/rename_camel.i b/Examples/test-suite/rename_camel.i index 970bb9215..0f4ae11a1 100644 --- a/Examples/test-suite/rename_camel.i +++ b/Examples/test-suite/rename_camel.i @@ -20,18 +20,14 @@ } -%define SedCmd "%(command:sed -e 's/\([a-z]\)/\U\\1/' -e 's/\(_\)\([a-z]\)/\U\\2/g' <<<)s" %enddef - %rename(CamelCase1) camel_case_1; -%rename(SedCmd) camel_case_2; +%rename("%(camelcase)s") camel_case_2; %rename("%(ctitle)s") camel_case_3; %rename("%(utitle)s") CamelCase_5; -%define awk_cmd "%(command:awk '/^i/{print toupper($1)}' <<<)s" %enddef - -%rename(awk_cmd) ""; +%rename("%(regex:/\\(.*i.*\\)/\\U\\1/)s") ""; %rename("%(title)s",regexmatch$parentNode$type="enum .*") ""; diff --git a/Source/Swig/misc.c b/Source/Swig/misc.c index 62a97875e..ac7392d9d 100644 --- a/Source/Swig/misc.c +++ b/Source/Swig/misc.c @@ -1157,47 +1157,14 @@ int Swig_scopename_check(const String *s) { /* ----------------------------------------------------------------------------- * Swig_string_command() * - * Executes a external command via popen with the string as a command - * line parameter. For example: - * - * Printf(stderr,"%(command:sed 's/[a-z]/\U\\1/' <<<)s","hello") -> Hello + * Feature removed in SWIG 4.1.0. * ----------------------------------------------------------------------------- */ -#if defined(_MSC_VER) -# define popen _popen -# define pclose _pclose -# if !defined(HAVE_POPEN) -# define HAVE_POPEN 1 -# endif -#else -# if !defined(_WIN32) -/* These Posix functions are not ISO C and so are not always defined in stdio.h */ -extern FILE *popen(const char *command, const char *type); -extern int pclose(FILE *stream); -# endif -#endif String *Swig_string_command(String *s) { - String *res = NewStringEmpty(); -#if defined(HAVE_POPEN) - if (Len(s)) { - char *command = Char(s); - FILE *fp = popen(command, "r"); - if (fp) { - char buffer[1025]; - while (fscanf(fp, "%1024s", buffer) != EOF) { - Append(res, buffer); - } - pclose(fp); - } else { - Swig_error("SWIG", Getline(s), "Command encoder fails attempting '%s'.\n", s); - SWIG_exit(EXIT_FAILURE); - } - } -#endif - return res; + Swig_error("SWIG", Getline(s), "Command encoder no longer supported - use regex encoder instead.\n"); + SWIG_exit(EXIT_FAILURE); } - /* ----------------------------------------------------------------------------- * Swig_string_strip() *