sqlite 3.23.0 zipfile2 test failures

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

sqlite 3.23.0 zipfile2 test failures

Petr Kubat
Hello everyone,

I have recently rebased Fedora's version of sqlite to 3.23.0 and saw
some failures in one of the zipfile2 test cases during running
self-tests (full logs [1]):

    Time: zipfile.test 193 ms
    ! zipfile2-2.0 expected: [1 {error in fread()}]
    ! zipfile2-2.0 got:      [0 {}]
    Time: zipfile2.test 12 ms
    SQLite 2018-04-02 11:04:16 736b53f57f70b23172c30880186dce7ad9baa3b74e3838cae5847cffb98falt1
    1 errors out of 187562 tests on  Linux 64-bit big-endian

After looking a bit more into the failure it seems like this piece of
code (from (ext/misc/zipfile.c:zipfileReadEOCD)) is the reason for the
failures:

       fseek(pFile, 0, SEEK_END);
       szFile = (i64)ftell(pFile);
       if( szFile==0 ){
         memset(pEOCD, 0, sizeof(ZipfileEOCD));
         return SQLITE_OK;
       }
       nRead = (int)(MIN(szFile, ZIPFILE_BUFFER_SIZE));
       iOff = szFile - nRead;
       rc = zipfileReadData(pFile, aRead, nRead, iOff, &pTab->base.zErrMsg);

The issue here is that `fseek` and `ftell` are being run on a FILE
pointer that is created from fopen-ing a directory. On some systems, and
I think this is tied to the filesystem used (I managed to reproduce the
failure on a VM using XFS), this leads to the `zipfileReadData` call
being skipped due to `ftell` returning 0 (returns LONG_MAX on ext4) and
SQLITE_OK is returned instead of failing with error (through
`zipfileReadData`).

To me it seems like calling `fseek` and `ftell` on a directory results
in undefined behaviour so it would make more sense to explicitly check
the type of the target before attempting it. However, I am not sure
where such a change would be best to take place (which is why there is
no fix attached to this).

Petr

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: sqlite 3.23.0 zipfile2 test failures

Petr Kubat
Actually forgot to append the link tot he full logs so one more mail to
fix it:

https://kojipkgs.fedoraproject.org//work/tasks/7200/26137200/build.log

On 04/12/2018 02:23 PM, Petr Kubat wrote:

> Hello everyone,
>
> I have recently rebased Fedora's version of sqlite to 3.23.0 and saw
> some failures in one of the zipfile2 test cases during running
> self-tests (full logs [1]):
>
>    Time: zipfile.test 193 ms
>    ! zipfile2-2.0 expected: [1 {error in fread()}]
>    ! zipfile2-2.0 got:      [0 {}]
>    Time: zipfile2.test 12 ms
>    SQLite 2018-04-02 11:04:16
> 736b53f57f70b23172c30880186dce7ad9baa3b74e3838cae5847cffb98falt1
>    1 errors out of 187562 tests on  Linux 64-bit big-endian
>
> After looking a bit more into the failure it seems like this piece of
> code (from (ext/misc/zipfile.c:zipfileReadEOCD)) is the reason for the
> failures:
>
>       fseek(pFile, 0, SEEK_END);
>       szFile = (i64)ftell(pFile);
>       if( szFile==0 ){
>         memset(pEOCD, 0, sizeof(ZipfileEOCD));
>         return SQLITE_OK;
>       }
>       nRead = (int)(MIN(szFile, ZIPFILE_BUFFER_SIZE));
>       iOff = szFile - nRead;
>       rc = zipfileReadData(pFile, aRead, nRead, iOff,
> &pTab->base.zErrMsg);
>
> The issue here is that `fseek` and `ftell` are being run on a FILE
> pointer that is created from fopen-ing a directory. On some systems,
> and I think this is tied to the filesystem used (I managed to
> reproduce the failure on a VM using XFS), this leads to the
> `zipfileReadData` call being skipped due to `ftell` returning 0
> (returns LONG_MAX on ext4) and SQLITE_OK is returned instead of
> failing with error (through `zipfileReadData`).
>
> To me it seems like calling `fseek` and `ftell` on a directory results
> in undefined behaviour so it would make more sense to explicitly check
> the type of the target before attempting it. However, I am not sure
> where such a change would be best to take place (which is why there is
> no fix attached to this).
>
> Petr
>
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users