Re: [Bug-tar] improved error messages when reading snapshot file

[Nathan Stratton Treadway]

On Sun, Jan 27, 2013 at 08:47:37 -0800, Paul Eggert wrote:
> Thanks for that patch.  It inspired me to install the
> following change to GNU tar.  This is not the same patch

Great, thanks!

> but it uses the same ideas.  I refactored the code, which
> had some unnecessary duplication that your patch exposed.

Yes, that refactoring certainly is an improvement :)


For comparision, I ran this version of tar against the various corrupted
snapshot files I used for testing my own patch, and got the following
output:

================
./tar: snaptest0: byte 36: sec 123456789012345678901... too long
./tar: Error is not recoverable: exiting now

./tar: snaptest1: Unexpected EOF in snapshot file
./tar: Error is not recoverable: exiting now

./tar: snaptest2: byte 160: nsec 6336301 followed by invalid byte 0x78
./tar: Error is not recoverable: exiting now

./tar: snaptest3: byte 142: (valid range 0..18446744073709551615)
        dev 99999999999999999999: Numerical result out of range
./tar: Error is not recoverable: exiting now

./tar: snaptest4: byte 142: dev -9999999999999999999: Invalid argument
./tar: Error is not recoverable: exiting now

./tar: snaptest5: byte 166: dev -2306: Invalid argument
./tar: Error is not recoverable: exiting now

./tar: snaptest6: byte 160: (valid range 
-9223372036854775808..9223372036854775807)
        sec 9223372036854775899: Numerical result out of range
./tar: Error is not recoverable: exiting now

./tar: snaptest7: byte 206: Missing record terminator
./tar: Error is not recoverable: exiting now

./tar: snaptest8: Unexpected EOF in snapshot file
./tar: Error is not recoverable: exiting now

./tar: snaptest9: Unexpected EOF in snapshot file
./tar: Error is not recoverable: exiting now
================


I see that the "Missing record terminator" messages doesn't include the
byte offset of the error, but if I follow the logic correctly, it
actually can only happen at the end of the file (so printing the byte
offset is not any more useful there than it is for the varous
"Unexpected EOF" messages).


Perhaps more interesting is that tests 4 and 5 don't print out the range
of valid values for the field in question, as tests 3 and 6 do.

If the "-2306" value is instead put in the nsec field, the range info is
printed:

./tar: snaptest5A: byte 58: (valid range 0..999999999)
        nsec -2306: Numerical result out of range
./tar: Error is not recoverable: exiting now


It turns out the issue is in the way misc.c:strtosysint() treats input
strings with leading "-" characters: if maxval > INTMAX_MAX (as is the
case for the "dev" field on Linux) then the leading "-" causes the
routine to generate an EINVAL error, but with a smaller maxval, ERANGE
is generated instead (even though minval = 0 and the input buffer is the
same in both cases).

Admittedly the case of negative numbers being found in a uintmax_t field
will be pretty rare (especially given that copying snapshot files
between systems rarely makes any sense, let alone copying the files onto
a different platform), but I thought I'd mention the issue in case you
think it's worth tweaking strtosyint's behavior.

(For what it's worth, I see that xheader.c:decode_signed_num() also
makes use of the distiction between EINVAL and ERANGE; I don't
know if the uses in that file are affected by this particular
situation.)


                                                        Nathan



----------------------------------------------------------------------------
Nathan Stratton Treadway  -  address@hidden  -  Mid-Atlantic region
Ray Ontko & Co.  -  Software consulting services  -   http://www.ontko.com/
 GPG Key: http://www.ontko.com/~nathanst/gpg_key.txt   ID: 1023D/ECFB6239
 Key fingerprint = 6AD8 485E 20B9 5C71 231C  0C32 15F3 ADCD ECFB 6239