The current behavior of LZMA_FINISH in the decoder is a little
confusing because it requires calling lzma_code a few times without
providing more input to trigger a LZMA_BUF_ERROR. This patch replaces
return LZMA_OK lines with:
return action == LZMA_FINISH && *out_pos != out_size ? LZMA_BUF_ERROR : LZMA_OK;
This checks if the action is LZMA_FINISH and the output is not full,
then return the LZMA_BUF_ERROR. If the output is full, LZMA_OK is
needed because the caller needs to provide more room for the coder to
write out.
I believe this solves the issue in all cases and does not return
LZMA_BUF_ERROR when it shouldn't.
Jia Tan
From c7ebdb16a4f14fed641d82e97331eb39c4f57d1d Mon Sep 17 00:00:00 2001
From: jiat75 <jiat0...@gmail.com>
Date: Thu, 21 Apr 2022 21:09:57 +0800
Subject: [PATCH] LZMA_FINISH will now trigger LZMA_BUF_ERROR on truncated xz files right away
In the single threaded and multithreaded decoder, LZMA_FINISH
will cause BUF_ERROR right away instead of requiring multiple calls to
lzma_code without providing more input.
---
src/liblzma/common/common.c | 1 -
src/liblzma/common/stream_decoder.c | 19 ++++++++++++++-----
src/liblzma/common/stream_decoder_mt.c | 19 ++++++++++++++-----
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/src/liblzma/common/common.c b/src/liblzma/common/common.c
index 346fc7af..3ddaf199 100644
--- a/src/liblzma/common/common.c
+++ b/src/liblzma/common/common.c
@@ -352,7 +352,6 @@ lzma_code(lzma_stream *strm, lzma_action action)
default:
// All the other errors are fatal; coding cannot be continued.
- assert(ret != LZMA_BUF_ERROR);
strm->internal->sequence = ISEQ_ERROR;
break;
}
diff --git a/src/liblzma/common/stream_decoder.c b/src/liblzma/common/stream_decoder.c
index fdd8ff2f..06d4136b 100644
--- a/src/liblzma/common/stream_decoder.c
+++ b/src/liblzma/common/stream_decoder.c
@@ -118,7 +118,8 @@ stream_decode(void *coder_ptr, const lzma_allocator *allocator,
// Return if we didn't get the whole Stream Header yet.
if (coder->pos < LZMA_STREAM_HEADER_SIZE)
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
coder->pos = 0;
@@ -161,7 +162,8 @@ stream_decode(void *coder_ptr, const lzma_allocator *allocator,
case SEQ_BLOCK_HEADER: {
if (*in_pos >= in_size)
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
if (coder->pos == 0) {
// Detect if it's Index.
@@ -184,7 +186,8 @@ stream_decode(void *coder_ptr, const lzma_allocator *allocator,
// Return if we didn't get the whole Block Header yet.
if (coder->pos < coder->block_options.header_size)
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
coder->pos = 0;
@@ -256,6 +259,10 @@ stream_decode(void *coder_ptr, const lzma_allocator *allocator,
in, in_pos, in_size, out, out_pos, out_size,
action);
+ if (ret == LZMA_OK)
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
+
if (ret != LZMA_STREAM_END)
return ret;
@@ -275,7 +282,8 @@ stream_decode(void *coder_ptr, const lzma_allocator *allocator,
// lzma_index_hash_decode() since it would return
// LZMA_BUF_ERROR, which we must not do here.
if (*in_pos >= in_size)
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
// Decode the Index and compare it to the hash calculated
// from the sizes of the Blocks (if any).
@@ -296,7 +304,8 @@ stream_decode(void *coder_ptr, const lzma_allocator *allocator,
// Return if we didn't get the whole Stream Footer yet.
if (coder->pos < LZMA_STREAM_HEADER_SIZE)
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
coder->pos = 0;
diff --git a/src/liblzma/common/stream_decoder_mt.c b/src/liblzma/common/stream_decoder_mt.c
index e8939254..1519e570 100644
--- a/src/liblzma/common/stream_decoder_mt.c
+++ b/src/liblzma/common/stream_decoder_mt.c
@@ -1053,7 +1053,8 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
// Return if we didn't get the whole Stream Header yet.
if (coder->pos < LZMA_STREAM_HEADER_SIZE)
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
coder->pos = 0;
@@ -1153,7 +1154,8 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
break;
}
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
}
if (ret == LZMA_INDEX_DETECTED) {
@@ -1513,7 +1515,8 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
// Return if the input didn't contain the whole Block.
if (coder->thr->in_filled < coder->thr->in_size) {
assert(*in_pos == in_size);
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
}
// The whole Block has been copied to the thread-specific
@@ -1577,6 +1580,10 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
coder->progress_in += *in_pos - in_old;
coder->progress_out += *out_pos - out_old;
+ if (ret == LZMA_OK)
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
+
if (ret != LZMA_STREAM_END)
return ret;
@@ -1610,7 +1617,8 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
// lzma_index_hash_decode() since it would return
// LZMA_BUF_ERROR, which we must not do here.
if (*in_pos >= in_size)
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
// Decode the Index and compare it to the hash calculated
// from the sizes of the Blocks (if any).
@@ -1635,7 +1643,8 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
// Return if we didn't get the whole Stream Footer yet.
if (coder->pos < LZMA_STREAM_HEADER_SIZE)
- return LZMA_OK;
+ return action == LZMA_FINISH && *out_pos != out_size
+ ? LZMA_BUF_ERROR : LZMA_OK;
coder->pos = 0;
--
2.25.1
