[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Automake and whitespace in pwd
Re: Automake and whitespace in pwd
Thu, 27 Nov 2008 22:03:34 +0100
* Jim Meyering wrote on Thu, Nov 27, 2008 at 10:55:38AM CET:
> On one hand, I like the idea of accommodating the rare srcdir `pwd` that
> contains white space.
As Ralf alreay noted, it is not that rare with w32 users.
> On the other hand, since so many Makefile.am files
> are unprepared for the resulting names, I wonder if it's better to protect
> the users, and give Makefile.am writers a way to guarantee that "bad"
> `pwd` won't leak into places like $(top_srcdir).
But one point of my patch is exactly that it ensures that $(top_srcdir)
is safe to use *without* any quoting. It has to be: it is not possible
(portably) to quote file names that can appear as part of prerequisites
or targets in makefiles.
> This started with my
> auditing some Makefile.am files, adding quotes to protect against uses
> of potentially-tainted variables like $(top_srcdir). I found that many
> uses needed additional quoting. In addition, while single quoting was
> usually enough to protect them, sometimes I had to use double quotes,
> or a mix, to accommodate $$() and $$i-style shell constructs. Relying on
> that sort of correctness is fragile, in that there's nothing to actively
> prevent regressions, unless you write build tests that exercise each
> and every Makefile rule in the offending scenario.
You should not add any quoting to those variables which can reasonably
be relative. In this category are:
$(srcdir), $(top_srcdir), $(builddir), $(top_builddir)
My patch ensures these variables don't contain whitespace. Of course
the *_srcdir variables can still be absolute if the user specifies an
absolute path to `configure', but the sanity check will then abort if
those absolute names contain whitespace.
> What do you think of adding an automake option that would let the package
> maintainer specify that s/he does not want to expose package users to
> the risk of malfunction due to a tainted srcdir name?
Once more, my patch will ensure that srcdir is not tainted.
These names, however, can still contain whitespace:
$(abs_srcdir), $(abs_builddir), $(abs_top_srcdir), $(abs_top_builddir)
However, many package do not ever need to use these variables,
and Autoconf and Automake don't either (mostly), by default.
So I'm a bit reluctant to make it the default of forbidding
whitespace in them. The most likely cause of problems will be
when packages use some variable that itself uses $MISSING, *and*
does so outside of a makefile. Why? Because in a shell script,
the extra 'eval' step needed for interpreting the quotation I added
is missing. Good example is texi2dvi (which already does the right
thing by eval'ing $makeinfo).
Things are different for Libtool. We might want to add a check
to Libtool-using packages to forbid whitespace in the absolute build
> This feels like a package-level policy question, after all.
> Maintaining correctly-quoted Makefiles is hard enough without
> having to worry about such embedded white space.
> Sure, any package maintainer can add a few lines to configure.ac
> to make start-up code examine $srcdir and fail upon irregularity,
> but if automake were to make this easy, I'd be much more likely
> to use it than if it means copying around a custom .m4 file or
> repeating a short snippet of code in each package's configure.ac.
Again, $srcdir may not contain white space.
> That said, this patch looks fine. "make check" passed
> and it did the right thing in a couple of manual test scenarios.
Good, thanks for testing. I have applied it after incorporating your
NEWS suggestion, and after fixing a small portability wart, as in the
incremental patch below. (The 'case' issue is listed in the Autoconf
manual; sigh, I think it was me who added it ...)
> P.S. Would you please re-post your latest parallelizing-tests patches?
> I missed that functionality when running automake's lengthy,
> single-threaded "make check" on a quad-core system.
Hope to get to reposting them soonish. I've found some more issues with
them, and wanted to incorporate as many of Akim's good suggestions as
diff --git a/NEWS b/NEWS
index 2721e69..8b43c69 100644
@@ -138,12 +138,16 @@ New in 1.10a:
- The `missing' script works better with versioned tool names.
+ - Automake's early configure-time sanity check now diagnoses an
+ unsafe absolute source directory name and makes configure fail.
- The Automake macros and rules cope better with whitespace in the
current directory name, as long as the relative path to `configure'
does not contain whitespace. To this end, the values of `$(MISSING)'
and `$(install_sh)' may contain suitable quoting, and their expansion
might need `eval'uation if used outside of a makefile. These
- undocumented variables may be used in several documented macros.
+ undocumented variables may be used in several documented macros such
+ as $(AUTOCONF) or $(MAKEINFO).
Bugs fixed in 1.10a:
diff --git a/m4/sanity.m4 b/m4/sanity.m4
index 1704d83..3d2f304 100644
@@ -21,11 +21,11 @@ echo timestamp > conftest.file
case `pwd` in
AC_MSG_ERROR([unsafe absolute working directory name]);;
case $srcdir in
- *[[\"\#\$\&\'\\\`$am_lf\ \ ]]*)
+ *[[\\\"\#\$\&\'\`$am_lf\ \ ]]*)
AC_MSG_ERROR([unsafe srcdir value: `$srcdir']);;