search

jmcnamara / libxlsxwriter

A C library for creating Excel XLSX files.
https://libxlsxwriter.github.io
Other
1.49k stars 332 forks source link

image_buffer error in version 1.1.5 #388

Closed slw287r closed 1 year ago

slw287r commented 1 year ago

I encountered the follow warning and failed to insert cairo picture from memory on CentOS Linux:

[WARNING]: worksheet image insertion: couldn't read image type for: image_buffer.

I switched on and off of USE_MEM_FILE and got the same error.

option(USE_MEM_FILE "Use fmemopen()/open_memstream() in place of temporary files" ON)

It worked on macOS for version 1.1.5 and CentOS Linux for version 1.1.3 (1.1.4 not tested).

My demo scripts (a.c):

#include <xlsxwriter.h>
#include <cairo/cairo.h>

#define WIDTH 1500
#define HEIGHT 300

lxw_image_options img_opt = {
    .x_scale = 0.1 / 1.5,
    .y_scale = 0.1 / 1.5,
    .object_position = LXW_OBJECT_MOVE_AND_SIZE_AFTER,
    .description = ""
};

typedef struct {
    unsigned char *data;
    size_t length;
} png_t;

static cairo_status_t png_write_callback(void *closure, const unsigned char *data,
        unsigned int length)
{
    png_t *png = (png_t *)closure;
    size_t new_length = png->length + length;
    unsigned char *new_data;
    if ((new_data = realloc(png->data, new_length)) == NULL)
        return CAIRO_STATUS_WRITE_ERROR;
    memcpy(&new_data[png->length], data, length);
    png->data = new_data;
    png->length = new_length;
    return CAIRO_STATUS_SUCCESS;
}

int main()
{
    png_t *png = calloc(1, sizeof(png_t));
    cairo_surface_t *sf = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, WIDTH, HEIGHT);
    cairo_t *cr = cairo_create(sf);
    cairo_set_antialias(cr, CAIRO_ANTIALIAS_BEST);
    cairo_set_source_rgb(cr, 0, 0, 0);
    cairo_select_font_face(cr, "Sans", CAIRO_FONT_SLANT_NORMAL, CAIRO_FONT_WEIGHT_NORMAL);
    cairo_set_font_size(cr, 200.0);
    cairo_move_to(cr, WIDTH / 10, HEIGHT / 1.25);
    cairo_show_text(cr, "FOOBAR");
    cairo_surface_write_to_png_stream(sf, png_write_callback, png);
    cairo_destroy(cr);
    cairo_surface_destroy(sf);

    lxw_workbook *wb = workbook_new("foo.xlsx");
    lxw_worksheet *ws = workbook_add_worksheet(wb, "bar");
    worksheet_insert_image_buffer_opt(ws, 0, 0, png->data, png->length, &img_opt);
    lxw_error e = workbook_close(wb);
    if (e) {
        fprintf(stderr, "Error closing workbook, %d = %s\n", e, lxw_strerror(e));
        return e;
    }
    free((char *)png->data);
    free(png);
    return 0;
}

Compile and run:

gcc -o a a.c -lcairo -lxlsxwriter
./a # create foo.xlsx

Normal foo.xlsx

jmcnamara commented 1 year ago

What is the issue? The image is in the file you attached and I compiled and confirmed that it works with libxlsxwriter version 1.1.5 on Ubuntu 22.04.

screenshot

slw287r commented 1 year ago

I uploaded the normal xlsx from version 1.1.3 above. Please find the abnormal xlsx without the intended image generated via version 1.1.5 below.

My OS info: Linux 3.10.0-693.5.2.el7.x86_64 #1 SMP Fri Oct 20 20:32:50 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

foo_bad.xlsx

jmcnamara commented 1 year ago

Could you add this this debug code to your example and run it on the system/version where is doesn't work as expected.

int main()
{
    png_t *png = calloc(1, sizeof(png_t));
    cairo_surface_t *sf = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, WIDTH, HEIGHT);
    cairo_t *cr = cairo_create(sf);
    cairo_set_antialias(cr, CAIRO_ANTIALIAS_BEST);
    cairo_set_source_rgb(cr, 0, 0, 0);
    cairo_select_font_face(cr, "Sans", CAIRO_FONT_SLANT_NORMAL, CAIRO_FONT_WEIGHT_NORMAL);
    cairo_set_font_size(cr, 200.0);
    cairo_move_to(cr, WIDTH / 10, HEIGHT / 1.25);
    cairo_show_text(cr, "FOOBAR");
    cairo_surface_write_to_png_stream(sf, png_write_callback, png);
    cairo_destroy(cr);
    cairo_surface_destroy(sf);

    // Add this to your code.
    printf("Version = %s\n", lxw_version());
    printf("Length  = %zu\n", png->length);
    printf("Data    = %c%c%c\n", png->data[1], png->data[2], png->data[3]);

    lxw_workbook *wb = workbook_new("foo.xlsx");
    lxw_worksheet *ws = workbook_add_worksheet(wb, "bar");
    worksheet_insert_image_buffer_opt(ws, 0, 0, png->data, png->length, &img_opt);
    lxw_error e = workbook_close(wb);
    if (e) {
        fprintf(stderr, "Error closing workbook, %d = %s\n", e, lxw_strerror(e));
        return e;
    }
    free((char *)png->data);
    free(png);
    return 0;
}

You should get ouput like this:

Version = 1.1.5
Length  = 13543
Data    = PNG
slw287r commented 1 year ago

Here is what I get:

% ./a
Version = 1.1.5
Length  = 12489
Data    = PNG
[WARNING]: worksheet image insertion: couldn't read image type for: image_buffer.

and for v1.1.3, no warning and normal xlsx:

% ./a
Version = 1.1.3
Length  = 12489
Data    = PNG
jmcnamara commented 1 year ago

Thanks. In the 1.1.5 example are you using USE_MEM_FILE=ON?

slw287r commented 1 year ago
jmcnamara commented 1 year ago

@mohd-akram could you have a look at this issue. I can't reproduce it on the mac or Ubuntu systems that I have but I suspect that the issue may be the rewind() below similar to issues you fixed in the second part of your patchset:

https://github.com/jmcnamara/libxlsxwriter/blob/main/src/worksheet.c#L10396

That [WARNING] occurs when the library can't read any image markers in the data but the "PNG" marker is clearly there so I suspect the stream isn't being rewound.

mohd-akram commented 1 year ago

This is a bit of a strange issue. The image buffer code wasn't really touched since it already used buffers. The CentOS version is quite old and I suspect it might be a glibc issue. Can you print what's the glibc version (ldd --version)? Also, try with version 1.1.3 and the USE_FMEMOPEN option set to on, I suspect it didn't work then either.

mohd-akram commented 1 year ago

Tried this in a centos:7 podman container (glibc 2.17).

@jmcnamara Seems to be the bug mentioned here under notes. Changing w+b to wb+ removes the warning for me (I haven't tried opening the XLSX file). This will also need to be changed in worksheet_set_background_buffer.

@slw287r Try making that change and see if it works for you.

jmcnamara commented 1 year ago

@mohd-akram Thanks. Nice detective work.

slw287r commented 1 year ago

wb+ works for me

Tried this in a centos:7 podman container (glibc 2.17).

@jmcnamara Seems to be the bug mentioned here under notes. Changing w+b to wb+ removes the warning for me (I haven't tried opening the XLSX file). This will also need to be changed in worksheet_set_background_buffer.

@slw287r Try making that change and see if it works for you.

jmcnamara commented 1 year ago

@slw287r Thanks for the test/report. Fixed on main.

Closing.