[sane-devel] [PATCH] fix xsane PNM decoder
m. allan noah
kitno455 at gmail.com
Tue Oct 23 20:45:59 UTC 2012
The author of XSane is not a member of this list. You should contact
him directly, to ensure he sees your patch.
allan
On Tue, Oct 23, 2012 at 9:31 AM, Yann Droneaud <yann at droneaud.fr> wrote:
> - be sure to read return codes not to enter in infinite loop
> - be resilient to malformed or unrecognized PNM file
> - report errors in debug mode
>
> Signed-off-by: Yann Droneaud <yann at droneaud.fr>
> ---
> src/xsane-save.c | 300 +++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 223 insertions(+), 77 deletions(-)
>
> Hi,
>
> While trying to create/modify a multipage project, my XSanes window became
> unresponsive. This weird behavor was related to my modifications:
> I made changes on the images using GIMP behind XSane's back.
>
> This is related in bug #869175
> https://bugzilla.redhat.com/show_bug.cgi?id=869175
>
> After digging inside the source code, I've found that XSane was relying on
> a specific PNM format[1][2][3] with crafted headers.
> If those headers were not present, the decoder get stuck in an infinite loop.
>
> It would be great if XSane allows me to use an external tool to modify the
> file inside a multipage project.
>
> While I understand the need for the extra information stored in the XSane PNM
> files, I wander if they shouldn't be put in a associate meta data file,
> especially, in the case of multipage project, in the project description,
> so that outside modification can be detected.
>
> In the mean time, I improved the PNM parser to make the XSane header really
> optional and not block the tool when they are not present.
> This way I can do any horrible thing on the images before creating the final
> multipage PDF.
>
> Here's the patch.
>
> Regards.
> [1] http://netpbm.sourceforge.net/doc/ppm.html
> [2] http://netpbm.sourceforge.net/doc/pnm.html
> [3] http://en.wikipedia.org/wiki/Netpbm_format
>
> ---
> Yann Droneaud
>
> diff --git a/src/xsane-save.c b/src/xsane-save.c
> index f53f5e6..a4b24a7 100644
> --- a/src/xsane-save.c
> +++ b/src/xsane-save.c
> @@ -26,6 +26,7 @@
> #include "xsane-back-gtk.h"
> #include "xsane-front-gtk.h"
> #include "xsane-save.h"
> +#include <ctype.h>
> #include <time.h>
> #include <sys/wait.h>
>
> @@ -421,107 +422,224 @@ void xsane_update_counter_in_filename(char **filename, int skip, int step, int m
>
> void xsane_read_pnm_header(FILE *file, Image_info *image_info)
> {
> - int max_val, filetype_nr;
> - char buf[TEXTBUFSIZE];
> - int items_done;
> + int filetype_nr;
> + int width;
> + int height;
> + int max_val;
> + char buf[TEXTBUFSIZE];
> + int items_done;
> + char *ret;
> + int c;
> + int xsane_header_parsing = 0;
> + int xsane_header_count = 0;
> +
> + memset(image_info, 0, sizeof(*image_info));
>
> - fgets(buf, sizeof(buf)-1, file);
> - DBG(DBG_info, "filetype header :%s", buf);
> + ret = fgets(buf, sizeof(buf)-1, file);
> + if (ret == NULL)
> + {
> + DBG(DBG_error, "can't read PNM header: %s\n", strerror(errno));
> + goto error;
> + }
>
> if (buf[0] == 'P')
> {
> - filetype_nr = atoi(buf+1); /* get filetype number */
> + /* get filetype number and check PNM header format */
> + items_done = sscanf(buf, "P%d %d %d %d", &filetype_nr, &width, &height, &max_val);
> + if (items_done != 1)
> + {
> + DBG(DBG_error, "unsupported PNM header format\n");
> + goto error;
> + }
> +
> + DBG(DBG_info, "filetype header :P%d\n", filetype_nr);
> +
> + if (filetype_nr > 6)
> + {
> + DBG(DBG_error, "unrecognized PNM type\n");
> + goto error;
> + }
>
> image_info->resolution_x = 72.0;
> image_info->resolution_y = 72.0;
> image_info->reduce_to_lineart = FALSE;
> image_info->enable_color_management = FALSE;
>
> - while (strcmp(buf, "# XSANE data follows\n"))
> + /* parse header:
> + *
> + * # comment
> + * width height
> + * max
> + *
> + */
> + while (1)
> {
> - fgets(buf, sizeof(buf)-1, file);
> -
> - if (!strncmp(buf, "# resolution_x =", 20))
> - {
> - sscanf(buf+20, "%lf", &image_info->resolution_x);
> - }
> - else if (!strncmp(buf, "# resolution_y =", 20))
> - {
> - sscanf(buf+20, "%lf", &image_info->resolution_y);
> - }
> - else if (!strncmp(buf, "# threshold =", 20))
> - {
> - sscanf(buf+20, "%lf", &image_info->threshold);
> - }
> - else if (!strncmp(buf, "# gamma =", 20))
> - {
> - sscanf(buf+20, "%lf", &image_info->gamma);
> - }
> - else if (!strncmp(buf, "# gamma IRGB =", 20))
> - {
> - sscanf(buf+20, "%lf %lf %lf %lf",
> - &image_info->gamma,
> - &image_info->gamma_red,
> - &image_info->gamma_green,
> - &image_info->gamma_blue);
> - }
> - else if (!strncmp(buf, "# brightness =", 20))
> - {
> - sscanf(buf+20, "%lf", &image_info->brightness);
> - }
> - else if (!strncmp(buf, "# brightness IRGB =", 20))
> - {
> - sscanf(buf+20, "%lf %lf %lf %lf",
> - &image_info->brightness,
> - &image_info->brightness_red,
> - &image_info->brightness_green,
> - &image_info->brightness_blue);
> - }
> - else if (!strncmp(buf, "# contrast =", 20))
> - {
> - sscanf(buf+20, "%lf", &image_info->contrast);
> - }
> - else if (!strncmp(buf, "# contrast IRGB =", 20))
> + /* read just one char */
> + c = fgetc(file);
> + if (c == EOF)
> {
> - sscanf(buf+20, "%lf %lf %lf %lf",
> - &image_info->contrast,
> - &image_info->contrast_red,
> - &image_info->contrast_green,
> - &image_info->contrast_blue);
> + DBG(DBG_error, "error while reading PNM header: %s\n", strerror(errno));
> + goto error;
> }
> - else if (!strncmp(buf, "# color-management=", 20))
> - {
> - sscanf(buf+20, "%d", &image_info->enable_color_management);
> - }
> - else if (!strncmp(buf, "# cms-function =", 20))
> - {
> - sscanf(buf+20, "%d", &image_info->cms_function);
> - }
> - else if (!strncmp(buf, "# cms-intent =", 20))
> +
> + /* push back to be used by fgets() or fscanf() below */
> + ungetc(c, file);
> +
> + /* end of header */
> + if (c != '#')
> {
> - sscanf(buf+20, "%d", &image_info->cms_intent);
> + break;
> }
> - else if (!strncmp(buf, "# cms-bpc =", 20))
> +
> + /* read the comment line */
> + ret = fgets(buf, sizeof(buf)-1, file);
> + if (ret == NULL)
> {
> - sscanf(buf+20, "%d", &image_info->cms_bpc);
> + DBG(DBG_error, "error while reading PNM header: %s\n", strerror(errno));
> + goto error;
> }
> - else if (!strncmp(buf, "# icm-profile =", 20))
> +
> + if (!xsane_header_parsing)
> {
> - sscanf(buf+20, "%s", image_info->icm_profile);
> + if (!strcmp(buf, "# XSane settings:\n"))
> + {
> + /* start of xsane heaser */
> + xsane_header_parsing = 1;
> + xsane_header_count++;
> + }
> + else if (!strcmp(buf, "# XSANE data follows\n"))
> + {
> + DBG(DBG_info, "premature end of XSANE PNM header\n");
> + }
> }
> - else if (!strncmp(buf, "# reduce to lineart", 20))
> + else /* xsane_header_parsing */
> {
> - image_info->reduce_to_lineart = TRUE;
> + if (!strcmp(buf, "# XSANE data follows\n"))
> + {
> + /* end of xsane header */
> + xsane_header_parsing = 0;
> + }
> + else if (!strcmp(buf, "# XSane settings:\n"))
> + {
> + DBG(DBG_info, "XSANE header inside XSANE header !\n");
> + }
> + else if (!strncmp(buf, "# resolution_x =", 20))
> + {
> + sscanf(buf+20, "%lf", &image_info->resolution_x);
> + }
> + else if (!strncmp(buf, "# resolution_y =", 20))
> + {
> + sscanf(buf+20, "%lf", &image_info->resolution_y);
> + }
> + else if (!strncmp(buf, "# threshold =", 20))
> + {
> + sscanf(buf+20, "%lf", &image_info->threshold);
> + }
> + else if (!strncmp(buf, "# gamma =", 20))
> + {
> + sscanf(buf+20, "%lf", &image_info->gamma);
> + }
> + else if (!strncmp(buf, "# gamma IRGB =", 20))
> + {
> + sscanf(buf+20, "%lf %lf %lf %lf",
> + &image_info->gamma,
> + &image_info->gamma_red,
> + &image_info->gamma_green,
> + &image_info->gamma_blue);
> + }
> + else if (!strncmp(buf, "# brightness =", 20))
> + {
> + sscanf(buf+20, "%lf", &image_info->brightness);
> + }
> + else if (!strncmp(buf, "# brightness IRGB =", 20))
> + {
> + sscanf(buf+20, "%lf %lf %lf %lf",
> + &image_info->brightness,
> + &image_info->brightness_red,
> + &image_info->brightness_green,
> + &image_info->brightness_blue);
> + }
> + else if (!strncmp(buf, "# contrast =", 20))
> + {
> + sscanf(buf+20, "%lf", &image_info->contrast);
> + }
> + else if (!strncmp(buf, "# contrast IRGB =", 20))
> + {
> + sscanf(buf+20, "%lf %lf %lf %lf",
> + &image_info->contrast,
> + &image_info->contrast_red,
> + &image_info->contrast_green,
> + &image_info->contrast_blue);
> + }
> + else if (!strncmp(buf, "# color-management=", 20))
> + {
> + sscanf(buf+20, "%d", &image_info->enable_color_management);
> + }
> + else if (!strncmp(buf, "# cms-function =", 20))
> + {
> + sscanf(buf+20, "%d", &image_info->cms_function);
> + }
> + else if (!strncmp(buf, "# cms-intent =", 20))
> + {
> + sscanf(buf+20, "%d", &image_info->cms_intent);
> + }
> + else if (!strncmp(buf, "# cms-bpc =", 20))
> + {
> + sscanf(buf+20, "%d", &image_info->cms_bpc);
> + }
> + else if (!strncmp(buf, "# icm-profile =", 20))
> + {
> + sscanf(buf+20, "%s", image_info->icm_profile);
> + }
> + else if (!strncmp(buf, "# reduce to lineart", 20))
> + {
> + image_info->reduce_to_lineart = TRUE;
> + }
> + else
> + {
> + DBG(DBG_info, "unrecognized XSANE PNM header: %s", buf);
> + }
> }
> }
>
> - items_done = fscanf(file, "%d %d", &image_info->image_width, &image_info->image_height);
> + if (xsane_header_parsing)
> + {
> + DBG(DBG_info, "unfinished XSANE PNM header\n");
> + }
> + if (xsane_header_count == 0)
> + {
> + DBG(DBG_info, "no XSANE PNM header\n");
> + }
> + else if (xsane_header_count > 1)
> + {
> + DBG(DBG_info, "too many XSANE PNM headers\n");
> + }
>
> - image_info->depth = 1;
> + /* read image size */
> + if (filetype_nr == 1 || filetype_nr == 4) /* P4 = lineart */
> + {
> + items_done = fscanf(file, "%d %d", &width, &height);
> + if (items_done != 2)
> + {
> + DBG(DBG_error, "unrecognized PNM width/height: %s", buf);
> + goto error;
> + }
>
> - if (filetype_nr != 4) /* P4 = lineart */
> + image_info->image_width = width;
> + image_info->image_height = height;
> + image_info->depth = 1;
> + }
> + else
> {
> - items_done = fscanf(file, "%d", &max_val);
> + items_done = fscanf(file, "%d %d %d", &width, &height, &max_val);
> + if (items_done != 3)
> + {
> + DBG(DBG_error, "unrecognized PNM width/height: %s", buf);
> + goto error;
> + }
> +
> + image_info->image_width = width;
> + image_info->image_height = height;
>
> if (max_val == 255)
> {
> @@ -531,10 +649,20 @@ void xsane_read_pnm_header(FILE *file, Image_info *image_info)
> {
> image_info->depth = 16;
> }
> + else
> + {
> + DBG(DBG_error, "unrecognized PNM maximum value: %d\n", max_val);
> + goto error;
> + }
> }
>
> - fgetc(file); /* read exactly one newline character */
> -
> + c = fgetc(file); /* read exactly one whitespace character */
> +
> + if (c == EOF || !isspace(c))
> + {
> + DBG(DBG_error, "unrecognized PNM header\n");
> + goto error;
> + }
>
> image_info->channels = 1;
>
> @@ -546,6 +674,8 @@ void xsane_read_pnm_header(FILE *file, Image_info *image_info)
> #ifdef SUPPORT_RGBA
> else if (buf[0] == 'S') /* RGBA format */
> {
> + DBG(DBG_info, "filetype header :%s", buf);
> +
> items_done = fscanf(file, "%d %d\n%d", &image_info->image_width, &image_info->image_height, &max_val);
> fgetc(file); /* read exactly one newline character */
>
> @@ -563,10 +693,26 @@ void xsane_read_pnm_header(FILE *file, Image_info *image_info)
> image_info->channels = 4;
> }
> #endif
> + else
> + {
> + DBG(DBG_error, "error: unrecognized filetype header :%s", buf);
> + goto error;
> + }
>
> DBG(DBG_info, "xsane_read_pnm_header: width=%d, height=%d, depth=%d, colors=%d, resolution_x=%f, resolution_y=%f\n",
> image_info->image_width, image_info->image_height, image_info->depth, image_info->channels,
> image_info->resolution_x, image_info->resolution_y);
> +
> + return;
> +
> + error:
> +
> + rewind(file);
> + memset(image_info, 0, sizeof(*image_info));
> +
> + DBG(DBG_info, "xsane_read_pnm_header: failed to parse image\n");
> +
> + return;
> }
>
> /* ---------------------------------------------------------------------------------------------------------------------- */
> --
> 1.7.11.7
>
>
> --
> sane-devel mailing list: sane-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/sane-devel
> Unsubscribe: Send mail with subject "unsubscribe your_password"
> to sane-devel-request at lists.alioth.debian.org
--
"The truth is an offense, but not a sin"
More information about the sane-devel
mailing list