Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 201771

Summary: sys-apps/portage - unpack() should reject absolute paths + misleading error message
Product: Portage Development Reporter: Jakub Moc (RETIRED) <jakub>
Component: Core - Ebuild SupportAssignee: Portage team <dev-portage>
Severity: normal CC: grobian
Priority: High Keywords: InVCS
Version: 2.1   
Hardware: All   
OS: Linux   
See Also:
Package list:
Runtime testing required: ---
Bug Depends on:    
Bug Blocks: 200044    
Attachments: suggested patch for unpack()

Description Jakub Moc (RETIRED) gentoo-dev 2007-12-09 14:40:42 UTC
Well, basically making a mistake in ebuild and passing an absolute tarball path to unpack() made me waste quite some time due to completely misleading message. Since unpack only accepts ./ as path, it should reject all the rest instead with a proper error message.

In case it's really needed, an ebuild to reproduce is at, just replace unpack ./data.tar.gz with unpack "${S}/data.tar.gz and watch this:

 * ERROR: media-sound/asoundconf-1.0.14 failed.
 * Call stack:
 *     , line   46:  Called src_unpack
 *             environment, line  164:  Called unpack 'src_unpack'
 *     , line  324:  Called die
 * The specific snippet of code:
 *   		[ ! -s "${srcdir}${x}" ] && die "$myfail"
 *  The die message:
 *   /var/tmp/portage/media-sound/asoundconf-1.0.14/work/data.tar.gz does not exist

Well... that isn't useful indication of the problem at all:

$ file /var/tmp/portage/media-sound/asoundconf-1.0.14/work/data.tar.gz
/var/tmp/portage/media-sound/asoundconf-1.0.14/work/data.tar.gz: gzip compressed data, from Unix, max compression
Comment 1 Fabian Groffen gentoo-dev 2007-12-09 14:48:09 UTC
more detailed:

unpack() prefixes its argument with ${DISTDIR} unconditionally unless it starts with "./".  This is fine, but results in the confusing error message when used with an absolute path, as it looks for ${DISTDIR}/abspath/foo.

The error message could be improved by printing ${srcdir}${x} to make it print the full path it is also checking.  However, that results in some bloat that probably shouldn't be triggered at all, by simply spitting out an error message if the argument given to unpack starts with a / (and hence is absolute).
Comment 2 Fabian Groffen gentoo-dev 2007-12-09 14:58:58 UTC
Created attachment 138096 [details, diff]
suggested patch for unpack()

Attached a suggestion for how to change unpack()'s behaviour to refuse absolute paths.  I don't know if it is useful to allow subdirs in unpack (e.g. unpack somedir/data.tar.bz2), otherwise that could be rejected as well.
Comment 3 SpanKY gentoo-dev 2007-12-09 23:00:15 UTC
please fix the wording while you're in there

"should not" -> "cannot"
Comment 4 Zac Medico gentoo-dev 2007-12-14 01:15:01 UTC
This has been released in 2.1.4_rc10.

(In reply to comment #3)
> please fix the wording while you're in there
> "should not" -> "cannot"