Re: portsnap5 problem, portsnap error handling

On Tue, Oct 04, 2011 at 01:32:45PM +0300, Andriy Gapon wrote:
> Not sure which list would be best for this, so using current_at_.

> $ portsnap fetch && portsnap update
> Looking up portsnap.FreeBSD.org mirrors... 5 mirrors found.
> Fetching snapshot tag from portsnap5.FreeBSD.org... done.
> Fetching snapshot metadata... fetch:
> http://portsnap5.FreeBSD.org/t/c1bdea4c38cc6b417dedc1d0e75727118ed2ee08c41726ee49931f2c99288162:
> Not Found
> sha256: c1bdea4c38cc6b417dedc1d0e75727118ed2ee08c41726ee49931f2c99288162: No
> such file or directory
> [: !=: unexpected operator
> mv: rename c1bdea4c38cc6b417dedc1d0e75727118ed2ee08c41726ee49931f2c99288162 to
> tINDEX.new: No such file or directory
> done.
> grep: tINDEX.new: No such file or directory
> look: tINDEX.new: No such file or directory

> Portsnap metadata appears bogus.
> Cowardly refusing to proceed any further.

> First, it seems that portsnap5 host is missing at least one important file.
> Second, it seems that portsnap should detect this kind of problem a little bit
> earlier.  No need to call sha256, mv, etc if fetch clearly failed.
> (Not sure, perhaps this is fetch not returning an error code).

The important part is the error from [. Because the check is for
inequality, in case of a [ syntax error the "equal" path is taken and
the script continues as if everything is fine.

The script arrives there because of a missing backslash so that the
fetch(1) command's exit status is not checked.

The below patch should fix this fairly simply. More paranoid people will
probably want to test(1) hashes for equality rather than for inequality
so that a test(1) syntax error will fail safely.

Index: usr.sbin/portsnap/portsnap/portsnap.sh
===================================================================
--- usr.sbin/portsnap/portsnap/portsnap.sh	(revision 225917)
+++ usr.sbin/portsnap/portsnap/portsnap.sh	(working copy)
_at__at_ -536,9 +536,9 _at__at_
 	rm -f ${SNAPSHOTHASH} tINDEX.new
 
 	echo ${NDEBUG} "Fetching snapshot metadata... "
-	fetch ${QUIETFLAG} http://${SERVERNAME}/t/${SNAPSHOTHASH}
+	fetch ${QUIETFLAG} http://${SERVERNAME}/t/${SNAPSHOTHASH} \
 	    2>${QUIETREDIR} || return
-	if [ `${SHA256} -q ${SNAPSHOTHASH}` != ${SNAPSHOTHASH} ]; then
+	if [ "`${SHA256} -q ${SNAPSHOTHASH}`" != ${SNAPSHOTHASH} ]; then
 		echo "snapshot metadata corrupt."
 		return 1
 	fi
_at__at_ -606,7 +606,7 _at__at_
 # Verify a list of files
 fetch_snapshot_verify() {
 	while read F; do
-		if [ `gunzip -c snap/${F} | ${SHA256} -q` != ${F} ]; then
+		if [ "`gunzip -c snap/${F} | ${SHA256} -q`" != ${F} ]; then
 			echo "snapshot corrupt."
 			return 1
 		fi

-- 
Jilles Tjoelker