Skip to content

Commit 8fb88eb

Browse files
committed
fix(jlib): Error when compressing uncompressible data with zstd
- Ensure maxKeep doesn't exceed toCompress when keeping data uncompressed instead of compressing to avoid incorrect delta calculations. - Add inlen to bounds checks to prevent uint32_t wrap on uncompressed data - Enforce maxOutputSize limits appropriately on open() initialization - Lazily resize maxOutputSize memory limits during final close() flush - Skip unsafe undo pointer dereference if extra blocks were not created - Factor uncompressed payload into dynamic limits during adjustLimit()
1 parent 42d03cf commit 8fb88eb

1 file changed

Lines changed: 49 additions & 20 deletions

File tree

system/jlib/jlzbase.cpp

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ void CBlockCompressor::initCommon(size32_t initialSize)
4848

4949
void CBlockCompressor::open(void *buf,size32_t max, size32_t fixedRowSize, bool _allowPartialWrites)
5050
{
51-
assertex(buf && max);
51+
assertex(buf && max && (max > 2 * sizeof(size32_t)));
5252
originalMax = max;
5353
maxOutputSize = max;
5454
allowPartialWrites = _allowPartialWrites;
@@ -60,7 +60,8 @@ void CBlockCompressor::open(void *buf,size32_t max, size32_t fixedRowSize, bool
6060

6161
void CBlockCompressor::open(MemoryBuffer &mb, size32_t initialSize, size32_t fixedRowSize)
6262
{
63-
if (!initialSize)
63+
//Check if initialSize is not supplied, or is too small
64+
if (initialSize < sizeof(size32_t)*2)
6465
initialSize = FCMP_BUFFER_SIZE; // 1MB
6566

6667
originalMax = 0;
@@ -89,6 +90,11 @@ void CBlockCompressor::close()
8990

9091
//Any remaining data is copied uncompressed.
9192
size32_t totlen = outlen+sizeof(size32_t)+inlen;
93+
if (outBufMb && (totlen > maxOutputSize))
94+
{
95+
maxOutputSize = totlen;
96+
outbuf = (byte *)outBufMb->ensureCapacity(maxOutputSize);
97+
}
9298
assertex(maxOutputSize>=totlen);
9399
size32_t *tsize = (size32_t *)(outbuf+outlen);
94100
*tsize = inlen;
@@ -112,7 +118,7 @@ size32_t CBlockCompressor::write(const void *buf,size32_t len)
112118
// One example compressed at a ratio of 1:10,000!
113119
// Check before the loop to respect allowPartialWrites contract (all-or-nothing).
114120
const size32_t safetyThreshold = 0xF0000000; // 3.75GB
115-
offset_t projectedSize = (offset_t)totalWritten + len;
121+
offset_t projectedSize = (offset_t)totalWritten + inlen + len;
116122
if (projectedSize >= safetyThreshold)
117123
{
118124
full = true;
@@ -166,17 +172,20 @@ size32_t CBlockCompressor::write(const void *buf,size32_t len)
166172
//write() will be called again with the remainder of the data.
167173
if (extraWritten != toCopy)
168174
{
169-
if (allowPartialWrites || written == 0)
175+
if (allowPartialWrites)
170176
return written;
171177

172-
// Disallow partial writes, but a block of data including part of the row has already been compressed
173-
// We need to undo the first compression to restore the previous good data.
174-
byte * prevout = outbuf + savedOutlen;
175-
size32_t compressedSize = *(size32_t *)prevout;
176-
177-
//Only need to expand the first block that was compressed by this write call
178-
size32_t expanded = expandDirect(maxInputSize, inbuf, compressedSize, prevout + sizeof(size32_t));
179-
assertex(expanded >= savedInlen);
178+
if (outlen > savedOutlen)
179+
{
180+
// Disallow partial writes, but a block of data including part of the row has already been compressed
181+
// We need to undo the first compression to restore the previous good data.
182+
byte * prevout = outbuf + savedOutlen;
183+
size32_t compressedSize = *(size32_t *)prevout;
184+
185+
//Only need to expand the first block that was compressed by this write call
186+
size32_t expanded = expandDirect(maxInputSize, inbuf, compressedSize, prevout + sizeof(size32_t));
187+
assertex(expanded >= savedInlen);
188+
}
180189

181190
inlen = savedInlen;
182191
outlen = savedOutlen;
@@ -202,13 +211,30 @@ bool CBlockCompressor::adjustLimit(size32_t newLimit)
202211
assertex(!outBufMb); // Only supported when a fixed size buffer is provided
203212
assertex(newLimit <= originalMax);
204213

205-
//Reject the limit change if it is too small for the data already committed.
206-
size32_t reservedSpace = outlen + sizeof(size32_t) * 2;
207-
if (newLimit < reservedSpace)
208-
return false;
214+
// Case 1: Does the data fit including the uncompressed portion?
215+
size32_t requiredSpace = outlen + sizeof(size32_t) + inlen;
216+
if (newLimit >= requiredSpace)
217+
{
218+
maxOutputSize = newLimit;
219+
return true;
220+
}
209221

210-
maxOutputSize = newLimit;
211-
return true;
222+
// Case 2: We would exceed the limit. Attempt to compress the pending data
223+
// to see if we can shrink the footprint enough to fit.
224+
if (inlen > 0)
225+
{
226+
flushCompress(0); // This uses the current (larger) maxOutputSize to do its work
227+
228+
requiredSpace = outlen + sizeof(size32_t) + inlen;
229+
if (newLimit >= requiredSpace)
230+
{
231+
maxOutputSize = newLimit;
232+
return true;
233+
}
234+
}
235+
236+
// Still too large even after compression attempt
237+
return false;
212238
}
213239

214240
//Try and compress inlen + extra bytes of data - inlen is guaranteed to fit uncompressed.
@@ -275,8 +301,11 @@ size32_t CBlockCompressor::flushCompress(size32_t extra)
275301
{
276302
spaceLeft += sizeof(size32_t); //Can squeeze in 4 more bytes because there is no compressed size
277303
assertex(spaceLeft >= inlen);
278-
size_t delta = spaceLeft - inlen;
279-
inlen = spaceLeft;
304+
size_t maxKeep = spaceLeft;
305+
if (maxKeep > toCompress)
306+
maxKeep = toCompress;
307+
size_t delta = maxKeep - inlen;
308+
inlen = maxKeep;
280309
return delta;
281310
}
282311

0 commit comments

Comments
 (0)