diff --git a/CHANGELOG.md b/CHANGELOG.md index cec5ac9a6..b7a887c0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Python, Java and Apple file/image/font reads report stream failures instead + of returning partial data after a ZIP checksum or other read error. + - Android packaging rejects ABI directories missing either native library or containing empty libraries. JNI jar configuration now requires its actual JDK 17 minimum. diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 59f98ba63..507adafcc 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -289,10 +288,7 @@ - (nullable NSString *)diskPath { } - (nullable NSData *)dataWithError:(NSError **)error { - return guarded(error, [&]() -> NSData * { - const std::unique_ptr stream = _handle->stream(); - return to_nsdata(*stream); - }); + return guarded(error, [&]() -> NSData * { return to_nsdata(*_handle); }); } - (BOOL)copyTo:(NSString *)path error:(NSError **)error { @@ -698,9 +694,7 @@ @implementation ODRImageFile - (nullable NSData *)dataWithError:(NSError **)error { return guarded(error, [&]() -> NSData * { - const std::unique_ptr stream = - self.handle.as_image_file().stream(); - return to_nsdata(*stream); + return to_nsdata(self.handle.as_image_file().file()); }); } @@ -809,9 +803,7 @@ @implementation ODRFontFile - (nullable NSData *)dataWithError:(NSError **)error { return guarded(error, [&]() -> NSData * { - const std::unique_ptr stream = - self.handle.as_font_file().stream(); - return to_nsdata(*stream); + return to_nsdata(self.handle.as_font_file().file()); }); } diff --git a/apple/src/ODRInternal.h b/apple/src/ODRInternal.h index 9448e7503..b7dde2cb1 100644 --- a/apple/src/ODRInternal.h +++ b/apple/src/ODRInternal.h @@ -19,6 +19,10 @@ NS_ASSUME_NONNULL_BEGIN +namespace odr { +class File; +} + namespace odr::apple { /// Real UTF-8 <-> UTF-16, the way `odr_jni::to_string` does it. Never route @@ -28,9 +32,9 @@ std::string to_string(NSString *string); NSString *to_nsstring(const std::string &string); NSString *to_nsstring(std::string_view string); -/// Bytes as an `NSData`, drained from a stream where odrcore hands one out. +/// Copies bytes into `NSData`; file reads propagate I/O errors. NSData *to_nsdata(const std::string &bytes); -NSData *to_nsdata(std::istream &stream); +NSData *to_nsdata(const File &file); /// Fills `*error` from the exception currently being handled. Call only from /// inside a `catch`. diff --git a/apple/src/ODRInternal.mm b/apple/src/ODRInternal.mm index 3c75c5f6d..a5789a77b 100644 --- a/apple/src/ODRInternal.mm +++ b/apple/src/ODRInternal.mm @@ -5,9 +5,9 @@ #include #include +#include #include -#include #include NSErrorDomain const ODRErrorDomain = @"app.opendocument.OdrCore.ErrorDomain"; @@ -53,9 +53,9 @@ return [NSData dataWithBytes:bytes.data() length:bytes.size()]; } -NSData *apple::to_nsdata(std::istream &stream) { +NSData *apple::to_nsdata(const File &file) { std::ostringstream buffer; - buffer << stream.rdbuf(); + file.pipe(buffer); return to_nsdata(std::move(buffer).str()); } diff --git a/jni/src/jni_file.cpp b/jni/src/jni_file.cpp index 17fdcab29..ebd2886f9 100644 --- a/jni/src/jni_file.cpp +++ b/jni/src/jni_file.cpp @@ -418,7 +418,7 @@ Java_app_opendocument_core_ImageFile_readNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { std::ostringstream out; - out << decoded(handle).as_image_file().stream()->rdbuf(); + decoded(handle).as_image_file().file().pipe(out); return to_jbytes(env, out.str()); }); } @@ -523,7 +523,7 @@ Java_app_opendocument_core_FontFile_readNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { std::ostringstream out; - out << decoded(handle).as_font_file().stream()->rdbuf(); + decoded(handle).as_font_file().file().pipe(out); return to_jbytes(env, out.str()); }); } diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index 8f5478be2..467de8593 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -359,7 +359,7 @@ void odr_python::bind_file(py::module_ &m) { py::class_(m, "ImageFile") .def("read", [](const odr::ImageFile &file) { std::ostringstream out; - out << file.stream()->rdbuf(); + file.file().pipe(out); return py::bytes(out.str()); }); @@ -421,7 +421,7 @@ void odr_python::bind_file(py::module_ &m) { py::class_(m, "FontFile") .def("read", [](const odr::FontFile &file) { std::ostringstream out; - out << file.stream()->rdbuf(); + file.file().pipe(out); return py::bytes(out.str()); }); diff --git a/python/tests/test_file.py b/python/tests/test_file.py index 4bd944aef..9b699b5cd 100644 --- a/python/tests/test_file.py +++ b/python/tests/test_file.py @@ -1,3 +1,4 @@ +import io import zipfile import pytest @@ -251,3 +252,18 @@ def test_a_markdown_file_holds_a_text_file_too(tmp_path): assert file.is_markdown_file() assert file.as_markdown_file().text_file().text() == "# hello\n" assert file.as_markdown_file().document().document_type() == pyodr.DocumentType.text + + +def test_image_read_rejects_archive_corruption(): + output = io.BytesIO() + with zipfile.ZipFile(output, "w") as zip_file: + zip_file.writestr("image.png", b"\x89PNG\r\n\x1a\n" + b"x" * 1_000_000) + corrupt = bytearray(output.getvalue()) + # past the bytes the format detection reads, so only the full read fails + corrupt[corrupt.index(b"x") + 900_000] ^= 1 + archive = ( + pyodr.open(pyodr.File.from_memory(bytes(corrupt))).as_archive_file().archive() + ) + image = pyodr.open(archive.as_filesystem().open("/image.png")).as_image_file() + with pytest.raises(RuntimeError, match="stream read failed"): + image.read()