[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] Correct exit codes for invalid tar archives
From:       Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn () axis ! com>
Date:       2013-11-21 11:46:24
Message-ID: alpine.DEB.2.10.1311211100260.27154 () enwn ! fr ! nkvf ! pbz
[Download RAW message or body]

On Wed, 20 Nov 2013, Denys Vlasenko wrote:
>
> Your patch makes tar die on read < 512 bytes at once, without
> considering the possibility that it was a perfectly valid gzipped tarball
> which become shorter than 512 bytes after gzipping.
>
> The code before patch is:
>
>         i = full_read(archive_handle->src_fd, &tar, 512);
>         if (i == 0) {
>                 xfunc_error_retval = 0;
>                 bb_error_msg_and_die("short read");
>         }
>         if (i != 512) {
>                 IF_FEATURE_TAR_AUTODETECT(goto autodetect;)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ NOTE THIS
>                 bb_error_msg_and_die("short read");
>         }
>
> You make it impossible to reach that line.

Of course you're right.  Got it now.

> In fact I think you are right, zero-sized file is likely
> a result of error somewhere (a failed download or tarball creation)
> and it's better to refuse to process such "tarball". I fixed it in git.

Sounds good.  `tar' with the latest patches works for us.  Thanks.

Did some more tests.  Files of /dev/zero and /dev/urandom bytes;
sizes 1, 2, 3, 511, 512, 513, 1023, 1034, 1025 bytes.

I use GNU tar 1.26. Found one discrepancy, though

	$ bb-tar tf ../data/1023.zero-bytes;echo $?
	tar: invalid tar magic
	1
	$ gnu-tar tf ../data/1023.zero-bytes;echo $?
	tar: A lone zero block at 1
	0

I'm not sure busybox-tar needs to care about that one, but you decide.


Cheers,

-- 
Cristian
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]