diff --git a/NEWS b/NEWS index cd9ddd3994cc..35a92c1c1dbf 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.7.0alpha1 +- Bz2: + . Fixed GH-24170 (compress.bzip2:// truncates concatenated bzip2 streams). + (Alessio Attilio) + - Core: . A flat unkeyed list assignment from an array literal with one value per target, such as [$a, $b] = [$b, $a], no longer builds the array when the diff --git a/UPGRADING b/UPGRADING index 49d1ebc8258c..1cea53fb9439 100644 --- a/UPGRADING +++ b/UPGRADING @@ -19,6 +19,11 @@ PHP 8.7 UPGRADE NOTES 1. Backward Incompatible Changes ======================================== +- Bz2: + . The compress.bzip2:// wrapper and bzopen() now read all concatenated bzip2 + streams instead of stopping after the first stream. Trailing data without + a bzip2 header is still ignored. + - Curl: . curl_multi_exec() now throws a TypeError when still_running is not an int. . Nested arrays in CURLOPT_POSTFIELDS now throw a TypeError, instead of being diff --git a/ext/bz2/bz2.c b/ext/bz2/bz2.c index dd1b87d2bf61..6f761626ece3 100644 --- a/ext/bz2/bz2.c +++ b/ext/bz2/bz2.c @@ -57,11 +57,54 @@ ZEND_GET_MODULE(bz2) struct php_bz2_stream_data_t { BZFILE *bz_file; + FILE *file; + bool stream_end; + bool read_failed; zend_resource *stream_res; }; /* {{{ BZip2 stream implementation */ +static bool php_bz2iop_next_stream(php_stream *stream) +{ + struct php_bz2_stream_data_t *self = stream->abstract; + char buffer[BZ_MAX_UNUSED]; + void *unused; + int unused_len, error; + + BZ2_bzReadGetUnused(&error, self->bz_file, &unused, &unused_len); + if (error != BZ_OK) { + self->read_failed = true; + stream->eof = 1; + return false; + } + memcpy(buffer, unused, unused_len); + while (unused_len < 4) { + int ch = fgetc(self->file); + if (ch == EOF) { + self->read_failed = ferror(self->file) != 0; + break; + } + buffer[unused_len++] = ch; + } + if (unused_len < 4 || memcmp(buffer, "BZh", 3) != 0 + || buffer[3] < '1' || buffer[3] > '9') { + stream->eof = 1; + return false; + } + + BZFILE *next = BZ2_bzReadOpen(&error, self->file, 0, strchr(stream->mode, 's') != NULL, buffer, unused_len); + if (error != BZ_OK) { + self->read_failed = true; + stream->eof = 1; + return false; + } + BZ2_bzReadClose(&error, self->bz_file); + self->bz_file = next; + self->stream_end = false; + return true; +} + static ssize_t php_bz2iop_read(php_stream *stream, char *buf, size_t count) { struct php_bz2_stream_data_t *self = (struct php_bz2_stream_data_t *)stream->abstract; @@ -72,12 +115,30 @@ static ssize_t php_bz2iop_read(php_stream *stream, char *buf, size_t count) size_t remain = count - ret; int to_read = (int)(remain <= INT_MAX ? remain : INT_MAX); - just_read = BZ2_bzread(self->bz_file, buf, to_read); + if (stream->eof) { + return ret ? (ssize_t) ret : (self->read_failed ? -1 : 0); + } + if (self->file) { + int error; + if (self->stream_end && !php_bz2iop_next_stream(stream)) { + return ret ? (ssize_t) ret : (self->read_failed ? -1 : 0); + } + just_read = BZ2_bzRead(&error, self->bz_file, buf + ret, to_read); + self->stream_end = error == BZ_STREAM_END; + if (error != BZ_OK && error != BZ_STREAM_END) { + just_read = -1; + } else if (just_read == 0 && self->stream_end) { + continue; + } + } else { + just_read = BZ2_bzread(self->bz_file, buf + ret, to_read); + } if (just_read < 1) { /* it is not safe to keep reading after an error, see #72613 */ stream->eof = 1; if (just_read < 0) { + self->read_failed = true; if (ret) { return ret; } @@ -126,7 +187,13 @@ static int php_bz2iop_close(php_stream *stream, int close_handle) int ret = EOF; if (close_handle) { - BZ2_bzclose(self->bz_file); + if (self->file) { + int error; + BZ2_bzReadClose(&error, self->bz_file); + fclose(self->file); + } else { + BZ2_bzclose(self->bz_file); + } } /* The inner stream may have been closed by the user, only its resource is held */ @@ -164,14 +231,32 @@ const php_stream_ops php_stream_bz2io_ops = { /* {{{ Bzip2 stream openers */ -/* bzlib closes the descriptor it is given, so it gets a duplicate */ -static BZFILE *php_bz2_bzdopen(php_socket_t fd, const char *mode) +static BZFILE *php_bz2_readopen(FILE *file, const char *mode) +{ + int error; + BZFILE *bz = BZ2_bzReadOpen(&error, file, 0, strchr(mode, 's') != NULL, NULL, 0); + if (!bz) { + fclose(file); + } + return bz; +} + +static BZFILE *php_bz2_bzdopen(php_socket_t fd, const char *mode, FILE **file) { int dup_fd = dup((int) fd); if (dup_fd == -1) { return NULL; } + if (mode[0] == 'r') { + *file = fdopen(dup_fd, "rb"); + if (!*file) { + close(dup_fd); + return NULL; + } + return php_bz2_readopen(*file, mode); + } + BZFILE *bz = BZ2_bzdopen(dup_fd, mode); if (!bz) { close(dup_fd); @@ -193,6 +278,9 @@ PHP_BZ2_API php_stream *_php_stream_bz2open_from_BZFILE(BZFILE *bz, GC_ADDREF(innerstream->res); } self->bz_file = bz; + self->file = NULL; + self->stream_end = false; + self->read_failed = false; return php_stream_alloc_rel(&php_stream_bz2io_ops, self, 0, mode); } @@ -207,6 +295,7 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, php_stream *retstream = NULL, *stream = NULL; char *path_copy = NULL; BZFILE *bz_file = NULL; + FILE *file = NULL; if (strncasecmp("compress.bzip2://", path, 17) == 0) { path += 17; @@ -229,7 +318,14 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, } /* try and open it directly first */ - bz_file = BZ2_bzopen(path_copy, mode); + if (mode[0] == 'r') { + file = VCWD_FOPEN(path_copy, "rb"); + if (file) { + bz_file = php_bz2_readopen(file, mode); + } + } else { + bz_file = BZ2_bzopen(path_copy, mode); + } if (opened_path && bz_file) { *opened_path = zend_string_init(path_copy, strlen(path_copy), 0); @@ -246,7 +342,7 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, if (stream) { php_socket_t fd; if (SUCCESS == php_stream_cast(stream, PHP_STREAM_AS_FD, (void **) &fd, REPORT_ERRORS)) { - bz_file = php_bz2_bzdopen(fd, mode); + bz_file = php_bz2_bzdopen(fd, mode, &file); } } @@ -261,6 +357,7 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper, if (bz_file) { retstream = _php_stream_bz2open_from_BZFILE(bz_file, mode, stream STREAMS_REL_CC); if (retstream) { + ((struct php_bz2_stream_data_t *) retstream->abstract)->file = file; return retstream; } @@ -358,6 +455,7 @@ PHP_FUNCTION(bzopen) BZFILE *bz; /* The compressed file stream */ php_stream *stream = NULL; + FILE *handle = NULL; if (zend_parse_parameters(ZEND_NUM_ARGS(), "zs", &file, &mode, &mode_len) == FAILURE) { RETURN_THROWS(); @@ -432,12 +530,13 @@ PHP_FUNCTION(bzopen) RETURN_FALSE; } - bz = php_bz2_bzdopen(fd, mode); + bz = php_bz2_bzdopen(fd, mode, &handle); if (!bz) { RETURN_FALSE; } stream = php_stream_bz2open_from_BZFILE(bz, mode, stream); + ((struct php_bz2_stream_data_t *) stream->abstract)->file = handle; } else { zend_argument_type_error(1, "must be of type string or file-resource, %s given", zend_zval_value_name(file)); RETURN_THROWS(); diff --git a/ext/bz2/tests/gh24170.phpt b/ext/bz2/tests/gh24170.phpt new file mode 100644 index 000000000000..05578696e5c9 --- /dev/null +++ b/ext/bz2/tests/gh24170.phpt @@ -0,0 +1,89 @@ +--TEST-- +GH-24170 (bzip2 streams read all concatenated members) +--EXTENSIONS-- +bz2 +--FILE-- + $part) { + $compressed .= bzcompress($part, $i + 1); +} +file_put_contents($file, $compressed); +$expected = implode('', $parts); +var_dump(file_get_contents('compress.bzip2://' . $file) === $expected); +$copy = $file . '.out'; +var_dump(copy('compress.bzip2://' . $file, $copy)); +var_dump(file_get_contents($copy) === $expected); +unlink($copy); + +foreach ([1, 17, 8192, 900000, 1000000] as $size) { + $fp = fopen('compress.bzip2://' . $file, 'rb'); + $offset = 0; + $matches = true; + while (!feof($fp)) { + $chunk = fread($fp, $size); + $matches = $matches && $chunk === substr($expected, $offset, strlen($chunk)); + $offset += strlen($chunk); + } + var_dump($matches && $offset === strlen($expected)); + var_dump(fread($fp, 1)); + fclose($fp); +} + +foreach ([false, true] as $resource) { + $inner = $resource ? fopen($file, 'rb') : null; + $fp = bzopen($inner ?? $file, 'r'); + if ($inner) { + fclose($inner); + } + $result = ''; + while (!feof($fp)) { + $result .= bzread($fp, 8192); + } + var_dump($result === $expected); + var_dump(bzerrno($fp)); + var_dump(bzread($fp, 1)); + bzclose($fp); +} + +file_put_contents($file, str_repeat(bzcompress(''), 400) . bzcompress('end')); +var_dump(file_get_contents('compress.bzip2://' . $file)); +file_put_contents($file, str_repeat(bzcompress(''), 400)); +var_dump(file_get_contents('compress.bzip2://' . $file)); +unlink($file); +?> +--CLEAN-- + +--EXPECT-- +bool(true) +bool(true) +bool(true) +bool(true) +string(0) "" +bool(true) +string(0) "" +bool(true) +string(0) "" +bool(true) +string(0) "" +bool(true) +string(0) "" +bool(true) +int(0) +string(0) "" +bool(true) +int(0) +string(0) "" +string(3) "end" +string(0) "" diff --git a/ext/bz2/tests/gh24170_boundaries.phpt b/ext/bz2/tests/gh24170_boundaries.phpt new file mode 100644 index 000000000000..9d6b79dc14ac --- /dev/null +++ b/ext/bz2/tests/gh24170_boundaries.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-24170 (concatenated bzip2 headers split across the compressed input buffer) +--EXTENSIONS-- +bz2 +--FILE-- + +--CLEAN-- + +--EXPECT-- +int(4997) +bool(true) +bool(true) +int(4998) +bool(true) +bool(true) +int(4999) +bool(true) +bool(true) +int(5000) +bool(true) +bool(true) diff --git a/ext/bz2/tests/gh24170_errors.phpt b/ext/bz2/tests/gh24170_errors.phpt new file mode 100644 index 000000000000..b17f1d3dc447 --- /dev/null +++ b/ext/bz2/tests/gh24170_errors.phpt @@ -0,0 +1,53 @@ +--TEST-- +GH-24170 (concatenated bzip2 streams handle trailing data and damaged members) +--EXTENSIONS-- +bz2 +--FILE-- + +--CLEAN-- + +--EXPECT-- +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +string(4) "data" +bool(false) +bool(true) +bool(true) +bool(false) +string(4) "data" +bool(false) +bool(true) +bool(true) +bool(false) +string(4) "data" +bool(false) +bool(true) +bool(true) +bool(false)