ltdl .la file parsing (was: Coverage for libltdl/slist.c and fallout)

[Ralf Wildenhues]

Hi Bob,

* Bob Friesenhahn wrote on Sun, Dec 06, 2009 at 07:33:28PM CET:
> There is no doubt that testing is important.  It seems most
> important (to me) that externally-exposed interfaces should be
> validated before there is intense focus on test coverage of internal
> interfaces.  For example, it is already known that the .la file
> parser is fragile and trivial edits to an .la file will cause the
> using program to core-dump.  The .la file parsing is an external
> interface so it should get more priority.

Good idea.  Proposed patch.  What other things do you know about that
are mishandled?

OK to commit?

Thanks,
Ralf

    Improve parsing of .la files in libltdl.
    
    * libltdl/ltdl.c (trim): Do not dump core upon missing quote in
    module .la file.
    * tests/lalib-syntax.at (syntax of .la files): New file, new
    test.
    * Makefile.am (TESTSUITE_AT): Add tests/lalib-syntax.at.
    Report by Bob Friesenhahn.

diff --git a/Makefile.am b/Makefile.am
index 2b09991..c26f05b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -484,6 +484,7 @@ TESTSUITE_AT        = tests/testsuite.at \
                  tests/lt_dlopen_a.at \
                  tests/lt_dlopenext.at \
                  tests/ltdl-api.at \
+                 tests/lalib-syntax.at \
                  tests/slist.at \
                  tests/need_lib_prefix.at \
                  tests/standalone.at \
diff --git a/libltdl/ltdl.c b/libltdl/ltdl.c
index acbc68f..dc1d209 100644
--- a/libltdl/ltdl.c
+++ b/libltdl/ltdl.c
@@ -1000,7 +1000,7 @@ trim (char **dest, const char *str)
 
   FREE (*dest);
 
-  if (!end)
+  if (!end || end == str)
     return 1;
 
   if (len > 3 && str[0] == '\'')
diff --git a/tests/lalib-syntax.at b/tests/lalib-syntax.at
new file mode 100644
index 0000000..65d6b55
--- /dev/null
+++ b/tests/lalib-syntax.at
@@ -0,0 +1,125 @@
+# lalib-syntax.at -- parsing .la files robustly  -*- Autotest -*-
+#
+#   Copyright (C) 2009 Free Software Foundation, Inc.
+#
+#   This file is part of GNU Libtool.
+#
+# GNU Libtool 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 of
+# the License, or (at your option) any later version.
+#
+# GNU Libtool 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 GNU Libtool; see the file COPYING.  If not, a copy
+# can be downloaded from  http://www.gnu.org/licenses/gpl.html,
+# or obtained by writing to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+####
+
+AT_SETUP([syntax of .la files])
+AT_KEYWORDS([libtool])
+AT_KEYWORDS([libltdl])
+
+AT_DATA([main.c],
+[[#include <ltdl.h>
+#include <stdio.h>
+
+int
+main (int argc, char* argv[])
+{
+  int err = 0;
+  lt_dlhandle plugin_handle;
+
+  if (argc < 2)
+    {
+      fprintf (stderr, "usage: %s plugin\n", argv[0]);
+      return 1;
+    }
+
+  lt_dlinit ();
+  plugin_handle = lt_dlopenext (argv[1]);
+  if (NULL != plugin_handle)
+    {
+      printf ("plugin opened successfully!\n");
+      lt_dlclose (plugin_handle);
+    }
+  else
+    {
+      printf ("plugin failed to open: %s\n", lt_dlerror());
+      err = 1;
+    }
+  lt_dlexit ();
+  return err;
+}
+]])
+
+AT_DATA([module.c],
+[[int foo (void) { return 0; }
+]])
+
+: ${LTDLINCL="-I$abs_top_srcdir/libltdl"}
+: ${LIBLTDL="$abs_builddir/../libltdl/libltdlc.la"}
+CPPFLAGS="$CPPFLAGS $LTDLINCL"
+
+AT_CHECK([$CC $CPPFLAGS $CFLAGS -c main.c], [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c module.c],
+        [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o module.la module.lo ]dnl
+        [-module -avoid-version -rpath /somewhere], [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o main main.$OBJEXT 
$LIBLTDL],
+        [], [ignore], [ignore])
+
+
+# Several bogus test cases.
+
+AT_DATA([missing-closing-quote.la],
+[[# missing-closing-quote.la - a libtool library file
+# Generated by libtool
+dlname='module.so
+library_names='module.so module.so module.so'
+old_library='module.a'
+installed=no
+shouldnotlink=yes
+libdir='/somewhere'
+]])
+
+AT_DATA([wrong-quotes.la],
+[[# wrong-quotes.la - a libtool library file
+# Generated by libtool
+dlname="module.so"
+library_names='module.so module.so module.so'
+old_library='module.a'
+installed=no
+shouldnotlink=yes
+libdir='/somewhere'
+]])
+
+AT_DATA([no-dlname.la],
+[[# no-dlname.la - a libtool library file
+# Generated by libtool
+installed=no
+shouldnotlink=yes
+libdir='/somewhere'
+]])
+
+AT_DATA([nonexistent-dlname.la],
+[[# nonexistent-dlname.la - a libtool library file
+# Generated by libtool
+dlname='does-not-exist.so'
+installed=no
+shouldnotlink=yes
+libdir='/somewhere'
+]])
+
+for file in ./missing-closing-quote.la ./wrong-quotes.la \
+           ./no-dlname.la ./nonexistent-dlname.la; do
+  LT_AT_EXEC_CHECK([./main], [1], [stdout], [ignore], [$file])
+  AT_CHECK([grep 'plugin failed to open' stdout], [], [ignore])
+done
+
+AT_CLEANUP