Unexpected behaviour on loading truncated images

[bozaro]

Now call NewImageFromBuffer for truncated jpeg and png load successfully and produce some warning. It was unexpected for me.

Warnings are useless in this case: in a multithreaded application, they cannot be associated with a specific function call. I expected an error like io.ErrUnexpectedEOF.

This behavior can be controlled through the flag "fail" (with the exception of https://github.com/libvips/libvips/issues/1946).

For example:

index e3f3f17..169e371 100644
--- a/vips/foreign.c
+++ b/vips/foreign.c
@@ -5,9 +5,9 @@ int load_image_buffer(void *buf, size_t len, int imageType, VipsImage **out) {
        int code = 1;
 
        if (imageType == JPEG) {
-               code = vips_jpegload_buffer(buf, len, out, NULL);
+               code = vips_jpegload_buffer(buf, len, out, "fail", TRUE, NULL);
        } else if (imageType == PNG) {
-               code = vips_pngload_buffer(buf, len, out, NULL);
+               code = vips_pngload_buffer(buf, len, out, "fail", TRUE,  NULL);
        } else if (imageType == WEBP) {
                code = vips_webpload_buffer(buf, len, out, NULL);
        } else if (imageType == TIFF) {

I admit the current behavior might be helpful. And the value of the "fail" flag should be controlled by the user.

I see the following solutions to the problem:

• Add LoadingOptions parameter like func NewImageFromBuffer(buf []byte, opts *LoadingOptions) (*ImageRef, error) { (but this is breaking change);
• Add LoadingOptions parameter like func NewImageFromBuffer(buf []byte, opts ...LoadingOptions) (*ImageRef, error) { with MergeLoadingOptions method like https://github.com/mongodb/mongo-go-driver/blob/master/mongo/options/dboptions.go (but this is minor breaking change);
• Add global variables for fail flag (but this is ugly solution, especially in a multi-threaded application).

I would like to know which path is preferable before submitting the PR with changes.

P. S. https://twitter.com/kernio/status/879944532808278016

[tonimelisma]

Hey @bozaro thanks for this. There is already a more generic PR being worked on here: https://github.com/davidbyttow/govips/pull/111

The idea is to create a similar ImportParams struct and associated functions as we have for ExportParams, and a new Import function in addition to the current one which would also accept the ImportParams parameter.

Seeing as there hasn't been any action on the PR if you have the energy to create a similar PR that would be hugely appreciated. The specs on how to approach this are all described in the PR discussions. Do you think you this would be too much work?

[bozaro]

I create PoC for ImportParams. Sorry for formatting: I can't find sutable C autoformatter.

[tonimelisma]

Hey @bozaro is the fail parameter now probably handled with the new import parameters? Can we close the issue?

[bozaro]

This issue was fixed by #166 and can be closed. But original issue is more complex, then expected: https://github.com/libvips/libvips/issues/2309