Add Gzip Filename, Filetime and Comment support

[noloader]

This is a tracking issue to document Filename, Filetime and Comment support for Gzip.

Also see Add Filename, Filetime and Comment support to Gzip? on the mailing list and Pull Request 418.

[noloader]

@c0ff,

When running a Debug build of cryptest.exe v, I'm seeing a failure:

Testing Compressors and Decompressors...

FAILED:   Gunzip: length check error
FAILED:  128 zips and unzips
passed:  128 deflates and inflates
passed:  128 zlib decompress and compress

The code responsible for the failure is somewhere around validat0.cpp : 187. The code in validat0.cpp are extended tests, and they are only available when -DDEBUG or -DCRYPTOPP_COVERAGE is in effect. Debug and Coverage defines enable the define CRYPTOPP_EXTENDED_VALIDATION, and CRYPTOPP_EXTENDED_VALIDATION enables the additional code paths.

Now, this could mean our dumb fuzzing has uncovered a gap/bug/crash. Or, things may be working as expected and we are incorrectly reporting FAILED: Gunzip: length check error.

In either case, would you mind looking at it?

[c0ff]

Ok, will do

[noloader]

Your commit 591b139 seems to be the problem. First of all it changs only the writing side, but not the reading - that's why the tests fail. And then it contradicts the RFC.

Crap, you are right. I stopped reading at the second paragraph/third diagram of section 2.

Let me back that out...

[noloader]

I was too fast on conclusions, sorry. Enabled DEBUG and CRYPTOPP_COVERAGE - got assert in CryptoPP::Inflator::DecodeBody.

Those are expected with bad data. When testing a run of 128, you usually see 2 or 3 of them.

Asserts are controlled by -DDEBUG or -DCRYPTOPP_DEBUG. You can suppress them with the following (which still enables the extended validation):

CXXFLAGS="-DNDEBUG -DCRYPTOPP_COVERAGE -g2 -O3 ..."

[c0ff]

The assert I get is quite interesting:
Assertion failed: zinflate.cpp(560): CryptoPP::Inflator::DecodeBody

// TODO: this surfaced during fuzzing. What do we do???
CRYPTOPP_ASSERT(m_distance < COUNTOF(distanceExtraBits));

But it only appears if I compile with /Zi but link without /DEBUG - according to MSDN this is an incorrect combination of flags:
https://msdn.microsoft.com/en-us/library/958x11bc.aspx
However, /Zi does imply /debug;

So this is not a problem with the code.

EDIT: Looks like this combination of flags triggers #414

[noloader]

But it only appears if I compile with /Zi but link without /DEBUG

That does not sound right, but I may be missing something obvious.

CRYPTOPP_ASSERT is a custom assert provided in trap.h. Its claim to fame is it does not do stupid Posixy things, like crash a program being debugged or surface in release builds. Release builds are very sensitive to assert - if an assert crashes a program, then the sensitive information (like passwords and private keys) are sent to Apple, Canonical, Google, Microsoft, etc through bug reporting/error reporting services. The NSA and GCHQ thanks them :)

It is enabled by CRYPTOPP_DEBUG, which is set in config.h : 125:

#if (defined(DEBUG) || defined(_DEBUG)) && !defined(CRYPTOPP_DEBUG)
# define CRYPTOPP_DEBUG 1
#endif

I believe Visual Studio sets DEBUG and/or _DEBUG for debug builds, so you should have CRYPTOPP_DEBUG in Debug.

[c0ff]

I used nmake file, not the projects. Uncommented debug CXXFLAGS and got the assert, as LDFLAGS were left unchanged.

[noloader]

I used nmake file, not the projects

Oh man, +1. I thought I was the only guy who used it. I love that nmake file. If you ever have anything to add to it, then feel free to ping me. noloader, gmail account.

If you are a windows guy, you might be interested in this open bug report: Issue 159, Need cryptest.cmd for Windows testing. The thrust of the report is, we want to torture test Windows like we do Unix and Linux.

The 159 issue can use nmake or MSBuild. MSBuild and overriding project file settings may be the way to go. Also see our MSBuild wiki page. I've even been thinking about Python since Windows command scripting is so lame.

[noloader]

@c0ff, I removed about 6 intermediate comments from the thread. I accidentally whacked the one where you said the commit broke things, but the one that cites it is still there. Sorry about that.

(This will be whacked eventually).

[c0ff]

@noloader No problem.
I've taken a look at #159 - nope, sorry. I'm more of an ex-Linux kind of guy and I try no to touch ms-specific stuff. At my job we use scons/python-based build system for everything, including CryptoPP. We even generate vcproject files with it - much easier to make configuration changes this way.