From 2d73de1d360e0b7bc66b4db9e6b3a0e441927230 Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 18 Apr 2019 21:21:35 -0400 Subject: [PATCH] Fix zero-length reads causing problems for IO filters that did not expect them. The IoRead object was passing zero-length buffers into the filter processing code but not all the filters were happy about getting them. In particular, the gzip compression filter failed if it was given no input directly after it had flushed all of its buffers. This made the problem rather intermittent even though a zero-length buffer was being passed to the filter at the end of every file. It also explains why tweaking compress-level or buffer-size allowed the file to go through. Since this error was happening after all processing had completed, there does not appear to be any risk that successfully processed files were corrupted. Reported by brunre01, jwpit, Tomasz Kontusz, guruguruguru. --- doc/xml/release.xml | 31 +++++++++++++++++++++++++++++++ src/common/io/read.c | 3 ++- test/src/module/common/ioTest.c | 14 +++++++++++++- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 614e85e50..7b7cef42d 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -15,6 +15,17 @@ + + + + + + + + +

Fix zero-length reads causing problems for IO filters that did not expect them.

+ +

Fix reliability of error reporting from local/remote processes.

 @@ -6692,6 +6703,11 @@ baburdick + + brunre01 + brunre01 + + Bruno Friedmann tigerfoot @@ -6791,6 +6807,11 @@ gregscds + + guruguruguru + guruguruguru + + Hans-Jürgen Schönig @@ -6863,6 +6884,11 @@ jungle-boogie + + jwpit + jwpit + + Keith Fiske keithf4 @@ -7013,6 +7039,11 @@ gintoddic + + Tomasz Kontusz + ktosiek + + Thomas Flatley seadba diff --git a/src/common/io/read.c b/src/common/io/read.c index e877bf6f6..4d93f2cae 100644 --- a/src/common/io/read.c +++ b/src/common/io/read.c @@ -155,7 +155,8 @@ ioReadInternal(IoRead *this, Buffer *buffer, bool block) this->input = NULL; // Process the input buffer (or flush if NULL) - ioFilterGroupProcess(this->filterGroup, this->input, buffer); + if (this->input == NULL || bufUsed(this->input) > 0) + ioFilterGroupProcess(this->filterGroup, this->input, buffer); // Stop if not blocking -- we don't need to fill the buffer as long as we got some data if (!block && bufUsed(buffer) > bufferUsedBegin) diff --git a/test/src/module/common/ioTest.c b/test/src/module/common/ioTest.c index 8735dd94c..4544a969f 100644 --- a/test/src/module/common/ioTest.c +++ b/test/src/module/common/ioTest.c @@ -255,11 +255,23 @@ testRun(void) TEST_RESULT_VOID(ioReadFree(read), " free read object"); TEST_RESULT_VOID(ioReadFree(NULL), " free null read object"); + // Read a zero-length buffer to be sure it is not passed on to the filter group // ------------------------------------------------------------------------------------------------------------------------- IoBufferRead *bufferRead = NULL; ioBufferSizeSet(2); buffer = bufNew(2); - Buffer *bufferOriginal = bufNewZ("123"); + Buffer *bufferOriginal = bufNew(0); + + TEST_ASSIGN(bufferRead, ioBufferReadNew(bufferOriginal), "create empty buffer read object"); + TEST_RESULT_BOOL(ioReadOpen(ioBufferReadIo(bufferRead)), true, " open"); + TEST_RESULT_BOOL(ioReadEof(ioBufferReadIo(bufferRead)), false, " not eof"); + TEST_RESULT_SIZE(ioRead(ioBufferReadIo(bufferRead), buffer), 0, " read 0 bytes"); + TEST_RESULT_BOOL(ioReadEof(ioBufferReadIo(bufferRead)), true, " now eof"); + + // ------------------------------------------------------------------------------------------------------------------------- + ioBufferSizeSet(2); + buffer = bufNew(2); + bufferOriginal = bufNewZ("123"); MEM_CONTEXT_TEMP_BEGIN() {