unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* image size limit?
@ 2005-10-11 20:32 Chong Yidong
  2005-10-11 21:19 ` Kevin Rodgers
  2005-10-12 16:24 ` Richard M. Stallman
  0 siblings, 2 replies; 24+ messages in thread
From: Chong Yidong @ 2005-10-11 20:32 UTC (permalink / raw)


Can we get this discussion going again?  From FOR-RELEASE:

  Put a max-limit on the size of images, e.g. based on the display
  size.  This is to avoid allocating insane amounts of memory due to
  bogus image size specifications.  Note: rather than clipping images
  that are too big (this may be non-trivial to do correctly in all
  cases -- and thus non-trivial to test), it may be better just to
  avoid displaying such images for emacs 22.

I am not an expert, but from looking through image.c, it seems that
there is only so much we can do to prevent memory over-allocation
problems.  Generally, we have to call an external library to load an
image file, which does its own memory management thing.  By the time
we know the width and height, the external library has already loaded
the file into memory (or signalled an error).

For example, gif_load looks something like this:

  gif = (GifFileType *)fn_DGifOpenFileName (SDATA (file));
  ....
  /* Read entire contents.  */
  rc = fn_DGifSlurp (gif);
  ...
  image_top = gif->SavedImages[ino].ImageDesc.Top;
  ...
  x_create_x_image_and_pixmap (f, width, height, ...);

We have no control over what DGifSlurp does.

One place where we can set a limit is in x_create_x_image_and_pixmap,
where we malloc a pixmap to store the image contents.  The data
supplied to us by the external library is copied into this pixmap.  We
could signal an error if width and height are too large.  However,
this seems like closing the barn door after the horses have left --
the external library will already have allocated a big chunk of
memory.

Thoughts?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-11 20:32 image size limit? Chong Yidong
@ 2005-10-11 21:19 ` Kevin Rodgers
  2005-10-12 16:24 ` Richard M. Stallman
  1 sibling, 0 replies; 24+ messages in thread
From: Kevin Rodgers @ 2005-10-11 21:19 UTC (permalink / raw)


Chong Yidong wrote:
 > One place where we can set a limit is in x_create_x_image_and_pixmap,
 > where we malloc a pixmap to store the image contents.  The data
 > supplied to us by the external library is copied into this pixmap.  We
 > could signal an error if width and height are too large.  However,
 > this seems like closing the barn door after the horses have left --
 > the external library will already have allocated a big chunk of
 > memory.

But freeing that memory immediately (if the library provides such a
function) and signalling the error would certainly be better than
allocating the memory for the pixmap and proceeding to display the
image.  It's more like closing the barn door after half of the horses
have left.

-- 
Kevin Rodgers

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-11 20:32 image size limit? Chong Yidong
  2005-10-11 21:19 ` Kevin Rodgers
@ 2005-10-12 16:24 ` Richard M. Stallman
  2005-10-12 17:40   ` Romain Francoise
  1 sibling, 1 reply; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-12 16:24 UTC (permalink / raw)
  Cc: emacs-devel

    One place where we can set a limit is in x_create_x_image_and_pixmap,
    where we malloc a pixmap to store the image contents.  The data
    supplied to us by the external library is copied into this pixmap.  We
    could signal an error if width and height are too large.  However,
    this seems like closing the barn door after the horses have left --
    the external library will already have allocated a big chunk of
    memory.

Will it free that memory if Emacs decides to abort the operation?
If so, I think that still counts as a solution.  If not, I think
it is a bug in the library--so we should ask them to fix it.

Meanwhile, if these libraries do not have the feature of limiting the
memory they can use, I think they ought to have it.  That is a
necessary part of defending against invalid data.  A nonsensical image
that swallows all of memory is the equivalent of a denial-of-service
attack.  Good apps defend against that, and good libraries should be
designed to help apps defend against that.

Would you like to write to the developers of these libraries,
asking them nicely to add such a feature?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-12 16:24 ` Richard M. Stallman
@ 2005-10-12 17:40   ` Romain Francoise
  2005-10-13 20:13     ` Richard M. Stallman
  0 siblings, 1 reply; 24+ messages in thread
From: Romain Francoise @ 2005-10-12 17:40 UTC (permalink / raw)


For some image types, it is a simple matter of passing to the library
the limits Emacs wants to set.  For libpng, we could use
png_set_user_limits() to set our maximum width/height.  (libpng also has
a built-in default limit of 1 million pixels in both dimensions.)

For other libraries, the application can set the maximum amount of
memory to allocate, see for example libjpeg's `max_memory_to_use' and
`max_alloc_chunk'.

Getting things right for all image types will probably be tricky...

-- 
Romain Francoise <romain@orebokech.com> | I just thought I'd go out
it's a miracle -- http://orebokech.com/ | with a little bit more style.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-12 17:40   ` Romain Francoise
@ 2005-10-13 20:13     ` Richard M. Stallman
  2005-10-14 13:02       ` Chong Yidong
  2005-10-16 22:27       ` Chong Yidong
  0 siblings, 2 replies; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-13 20:13 UTC (permalink / raw)
  Cc: emacs-devel

    For some image types, it is a simple matter of passing to the library
    the limits Emacs wants to set.  For libpng, we could use
    png_set_user_limits() to set our maximum width/height.  (libpng also has
    a built-in default limit of 1 million pixels in both dimensions.)

    For other libraries, the application can set the maximum amount of
    memory to allocate, see for example libjpeg's `max_memory_to_use' and
    `max_alloc_chunk'.

    Getting things right for all image types will probably be tricky...

If we handle them one by one, it won't be terribly tricky.
We could have one limit for memory size of an image, and another limit
for the width and height as a ratio to those of the window.
Then for each image type it would use whichever of those can be used.

Would you like to implement this for some image types for which it is easy
to do?

Then we could ask the developers of the other libraries to add
similar features.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-13 20:13     ` Richard M. Stallman
@ 2005-10-14 13:02       ` Chong Yidong
  2005-10-15 16:13         ` Richard M. Stallman
  2005-10-16 22:27       ` Chong Yidong
  1 sibling, 1 reply; 24+ messages in thread
From: Chong Yidong @ 2005-10-14 13:02 UTC (permalink / raw)
  Cc: Romain Francoise, emacs-devel

> If we handle them one by one, it won't be terribly tricky.
> We could have one limit for memory size of an image, and another limit
> for the width and height as a ratio to those of the window.
> Then for each image type it would use whichever of those can be used.

Here's another idea.  We impose a limit on the size of the image file
that Emacs is willing to read, say 50 megabytes.  This should be easy
to implement and independent of image type.

At the same time, we implement image width and height limits in
x_create_x_image_and_pixmap, to deal with malicious images that
specify gigantic width and height sizes, even though the file size
isn't that big.

That should catch most of the problematic images, and we don't have to
fiddle with the individual image libraries.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-14 13:02       ` Chong Yidong
@ 2005-10-15 16:13         ` Richard M. Stallman
  2005-10-15 18:33           ` Chong Yidong
  0 siblings, 1 reply; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-15 16:13 UTC (permalink / raw)
  Cc: romain, emacs-devel

    Here's another idea.  We impose a limit on the size of the image file
    that Emacs is willing to read, say 50 megabytes.  This should be easy
    to implement and independent of image type.

That might be a good idea, but it won't address this problem.
In this problem case, the file itself is not too large,
but the image size is too large due to invalid data.

    At the same time, we implement image width and height limits in
    x_create_x_image_and_pixmap, to deal with malicious images that
    specify gigantic width and height sizes, even though the file size
    isn't that big.

Will that succeed in handling the problem case we got?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-15 16:13         ` Richard M. Stallman
@ 2005-10-15 18:33           ` Chong Yidong
  2005-10-16 17:36             ` Richard M. Stallman
  0 siblings, 1 reply; 24+ messages in thread
From: Chong Yidong @ 2005-10-15 18:33 UTC (permalink / raw)
  Cc: romain, emacs-devel

>     At the same time, we implement image width and height limits in
>     x_create_x_image_and_pixmap, to deal with malicious images that
>     specify gigantic width and height sizes, even though the file size
>     isn't that big.
>
> Will that succeed in handling the problem case we got?

It will avoid allocating too much memory in Emacs.  Whether libungif
tries to allocate too much memory prior to this, depends on the
internal implementation of libungif.  Worst that could happen is that
libungif believes the invalid width and height data, tries to malloc a
big chunk of memory, malloc fails, and DGifSlurp() frees the memory
and returns with an error, which we catch.

I can't find any libungif function for controlling how much memory
libungif uses.

As for libpng, the png_set_user_limits() function was only added in
version 1.0.16rc1, from 2004, so if we use that we'll lose
compatibility with older versions.

I don't think it's worth delaying the release for this.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-15 18:33           ` Chong Yidong
@ 2005-10-16 17:36             ` Richard M. Stallman
  0 siblings, 0 replies; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-16 17:36 UTC (permalink / raw)
  Cc: romain, emacs-devel

    As for libpng, the png_set_user_limits() function was only added in
    version 1.0.16rc1, from 2004, so if we use that we'll lose
    compatibility with older versions.

I think it is ok for the new Emacs to use a feature of libpng that
is one year old.  Would you like to write the code?

Perhaps configure should verify that that entry point exists, and
call it only if it is available.

    >     At the same time, we implement image width and height limits in
    >     x_create_x_image_and_pixmap, to deal with malicious images that
    >     specify gigantic width and height sizes, even though the file size
    >     isn't that big.
    >
    > Will that succeed in handling the problem case we got?

    It will avoid allocating too much memory in Emacs.  Whether libungif
    tries to allocate too much memory prior to this, depends on the
    internal implementation of libungif.  Worst that could happen is that
    libungif believes the invalid width and height data, tries to malloc a
    big chunk of memory, malloc fails, and DGifSlurp() frees the memory
    and returns with an error, which we catch.

I am not sure that is the worst that can happen, but if it does happen,
I agree it is better than what happens now.  So would you like to implement
that?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-13 20:13     ` Richard M. Stallman
  2005-10-14 13:02       ` Chong Yidong
@ 2005-10-16 22:27       ` Chong Yidong
  2005-10-17 17:30         ` Richard M. Stallman
  1 sibling, 1 reply; 24+ messages in thread
From: Chong Yidong @ 2005-10-16 22:27 UTC (permalink / raw)
  Cc: emacs-devel

Ok, I figured out how to do it.  This patch imposes height and width
limits based on the variable max-image-size.  It should cause load_gif
and the other image loading functions to return before the third-party
libraries are called upon to load the image data, so we shouldn't get
memory-overallocation at any point.

I've tested it for png, jpeg, tiff, and gif.  It should work for xbm,
pbm and ghostscript, but I haven't checked.  It doesn't impose limits
on xpm images, because I don't understand the xpm code too well.

If no one objects to this, I'll install it sometime next week.


*** emacs/src/image.c.~1.36.~	2005-10-11 11:09:35.000000000 -0400
--- emacs/src/image.c	2005-10-16 18:10:45.000000000 -0400
***************
*** 1097,1102 ****
--- 1097,1105 ----
  		 Image type independent image structures
   ***********************************************************************/
  
+ #define MAX_IMAGE_SIZE 100000
+ EMACS_INT Vmax_image_size;
+ 
  static struct image *make_image P_ ((Lisp_Object spec, unsigned hash));
  static void free_image P_ ((struct frame *f, struct image *img));
  
***************
*** 1708,1713 ****
--- 1711,1722 ----
      if (img->hash == hash && !NILP (Fequal (img->spec, spec)))
        break;
  
+   if (img && img->load_failed_p)
+     {
+       free_image (f, img);
+       img = NULL;
+     }
+ 
    /* If not found, create a new image and cache it.  */
    if (img == NULL)
      {
***************
*** 2992,2998 ****
        expect (XBM_TK_NUMBER);
      }
  
!   if (*width < 0 || *height < 0)
      goto failure;
    else if (data == NULL)
      goto success;
--- 3001,3008 ----
        expect (XBM_TK_NUMBER);
      }
  
!   if (*width <= 0     || *width > Vmax_image_size
!       || *height <= 0 || *height > Vmax_image_size)
      goto failure;
    else if (data == NULL)
      goto success;
***************
*** 5465,5472 ****
  	max_color_idx = 255;
      }
  
!   if (width < 0
!       || height < 0
        || (type != PBM_MONO && max_color_idx < 0))
      goto error;
  
--- 5475,5482 ----
  	max_color_idx = 255;
      }
  
!   if (width <= 0 || width > Vmax_image_size
!       || height <=0 || height > Vmax_image_size
        || (type != PBM_MONO && max_color_idx < 0))
      goto error;
  
***************
*** 5966,5971 ****
--- 5976,5985 ----
    fn_png_get_IHDR (png_ptr, info_ptr, &width, &height, &bit_depth, &color_type,
  		   &interlace_type, NULL, NULL);
  
+   if (width <= 0 || width > Vmax_image_size
+       || height <=0 || height > Vmax_image_size)
+     goto error;
+ 
    /* If image contains simply transparency data, we prefer to
       construct a clipping mask.  */
    if (fn_png_get_valid (png_ptr, info_ptr, PNG_INFO_tRNS))
***************
*** 6726,6731 ****
--- 6740,6752 ----
    width = img->width = cinfo.output_width;
    height = img->height = cinfo.output_height;
  
+   if (width <= 0 || width > Vmax_image_size
+       || height <=0 || height > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       longjmp (mgr.setjmp_buffer, 2);
+     }
+ 
    /* Create X image and pixmap.  */
    if (!x_create_x_image_and_pixmap (f, width, height, 0, &ximg, &img->pixmap))
      longjmp (mgr.setjmp_buffer, 2);
***************
*** 7155,7160 ****
--- 7176,7189 ----
       of width x height 32-bit values.  */
    fn_TIFFGetField (tiff, TIFFTAG_IMAGEWIDTH, &width);
    fn_TIFFGetField (tiff, TIFFTAG_IMAGELENGTH, &height);
+   if (width <= 0    || width > Vmax_image_size
+       || height <=0 || height > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       UNGCPRO;
+       return 0;
+     }
+ 
    buf = (uint32 *) xmalloc (width * height * sizeof *buf);
  
    rc = fn_TIFFReadRGBAImage (tiff, width, height, buf, 0);
***************
*** 7459,7464 ****
--- 7488,7504 ----
  	}
      }
  
+   /* Abort if the stated image size is too big, before reading entire
+      contents. */
+   if (gif->SWidth <= 0 || gif->SWidth > Vmax_image_size
+       || gif->SHeight <=0 || gif->SHeight > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       fn_DGifCloseFile (gif);
+       UNGCPRO;
+       return 0;
+     }
+ 
    /* Read entire contents.  */
    rc = fn_DGifSlurp (gif);
    if (rc == GIF_ERROR)
***************
*** 7492,7497 ****
--- 7532,7546 ----
  			      max (gif->Image.Top + gif->Image.Height,
  				   image_top + image_height));
  
+   if (width <= 0    || width > Vmax_image_size
+       || height <=0 || height > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       fn_DGifCloseFile (gif);
+       UNGCPRO;
+       return 0;
+     }
+ 
    /* Create the X image and pixmap.  */
    if (!x_create_x_image_and_pixmap (f, width, height, 0, &ximg, &img->pixmap))
      {
***************
*** 7944,7949 ****
--- 7993,8005 ----
    in_height = XFASTINT (pt_height) / 72.0;
    img->height = in_height * FRAME_X_DISPLAY_INFO (f)->resy;
  
+   if (img->width <= 0    || img->width > Vmax_image_size
+       || img->height <=0 || img->height > Vmax_image_size)
+     {
+       image_error ("Invalid image size", Qnil, Qnil);
+       return 0;
+     }
+ 
    /* Create the pixmap.  */
    xassert (img->pixmap == NO_PIXMAP);
  
***************
*** 8217,8222 ****
--- 8273,8286 ----
    Vimage_library_alist = Qnil;
    Fput (intern ("image-library-alist"), Qrisky_local_variable, Qt);
  
+   DEFVAR_INT ("max-image-size", &Vmax_image_size,
+     doc: /* Maximum width and height, in pixels, that an image can have.
+ 
+ Emacs will not load an image into memory if its width or height, in
+ pixels, is larger than this value.  Additional limits may be imposed
+ by the external libraries that Emacs uses to read the images. */);
+   Vmax_image_size = MAX_IMAGE_SIZE;
+ 
    Vimage_type_cache = Qnil;
    staticpro (&Vimage_type_cache);

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-16 22:27       ` Chong Yidong
@ 2005-10-17 17:30         ` Richard M. Stallman
  2005-10-17 21:56           ` Chong Yidong
  0 siblings, 1 reply; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-17 17:30 UTC (permalink / raw)
  Cc: emacs-devel

    + #define MAX_IMAGE_SIZE 100000
    + EMACS_INT Vmax_image_size;

100000 as a limit of linear dimension is too large.
A square image 100000-1 on each side will be 10 billion pixels,
far too large to fit in a 32 bit machine.
Such a limit should be more like 3000.  (That would allow
almost 10 million pixels, which is still very big.)

But I don't think this limit should be absolute.  I think it should be
specified as a multiple of the frame height and width, and it should
be given as a floating point number.  I'd suggest 2.0 as the default
for this ratio.

The error messages should say "Image too large"
rather than "Invalid image size".

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-17 17:30         ` Richard M. Stallman
@ 2005-10-17 21:56           ` Chong Yidong
  2005-10-18  3:39             ` Richard M. Stallman
  2005-10-19  9:02             ` Eli Zaretskii
  0 siblings, 2 replies; 24+ messages in thread
From: Chong Yidong @ 2005-10-17 21:56 UTC (permalink / raw)
  Cc: emacs-devel

> But I don't think this limit should be absolute.  I think it should be
> specified as a multiple of the frame height and width, and it should
> be given as a floating point number.  I'd suggest 2.0 as the default
> for this ratio.

What frame should we then use? The selected frame?

That's not really logical, the frame that issues the call to load an
image (which is where we check the image size) may not be the frame
that ends up displaying it.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-17 21:56           ` Chong Yidong
@ 2005-10-18  3:39             ` Richard M. Stallman
  2005-10-18 14:33               ` Stefan Monnier
  2005-10-19  9:02             ` Eli Zaretskii
  1 sibling, 1 reply; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-18  3:39 UTC (permalink / raw)
  Cc: emacs-devel

    > But I don't think this limit should be absolute.  I think it should be
    > specified as a multiple of the frame height and width, and it should
    > be given as a floating point number.  I'd suggest 2.0 as the default
    > for this ratio.

    What frame should we then use? The selected frame?

The frame that the image is to be displayed in.

    That's not really logical, the frame that issues the call to load an
    image (which is where we check the image size) may not be the frame
    that ends up displaying it.

How could they be different?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-18  3:39             ` Richard M. Stallman
@ 2005-10-18 14:33               ` Stefan Monnier
  2005-10-19  8:35                 ` Kim F. Storm
  2005-10-19 13:27                 ` Miles Bader
  0 siblings, 2 replies; 24+ messages in thread
From: Stefan Monnier @ 2005-10-18 14:33 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel

>> But I don't think this limit should be absolute.  I think it should be
>> specified as a multiple of the frame height and width, and it should
>> be given as a floating point number.  I'd suggest 2.0 as the default
>> for this ratio.

My Emacs frames are typically 80 columns of 13x6, i.e. about 500 pixel wide,
but I often (at least once a week) watch digital photos in those frames
and more often than not those picteures are 1600 or 2048 pixel wide.
Admittedly, it's not very convenient to watch those pictures within Emacs
right now (the vertical/horizontal scrolling is far from perfect), but often
I end up temporarilly maximizing the frame (after displaying the picture) to
1600 pixel wide.

All this to say that I think choosing the maximum image size based on
display-pixel-width and display-pixel-height would be preferable than using
frame size.


        Stefan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-18 14:33               ` Stefan Monnier
@ 2005-10-19  8:35                 ` Kim F. Storm
  2005-10-19 12:20                   ` David Kastrup
  2005-10-19 13:27                 ` Miles Bader
  1 sibling, 1 reply; 24+ messages in thread
From: Kim F. Storm @ 2005-10-19  8:35 UTC (permalink / raw)
  Cc: Chong Yidong, rms, emacs-devel


RMS:
> But I don't think this limit should be absolute.  I think it should be
> specified as a multiple of the frame height and width, and it should
> be given as a floating point number.	I'd suggest 2.0 as the default
> for this ratio.

Stefan:
> All this to say that I think choosing the maximum image size based on
> display-pixel-width and display-pixel-height would be preferable than using
> frame size.
>

If you use image slicing, you can in principle show a small area of a
much larger image.  

I don't see how that relates to the dimensions of the frame or display.

But it definitely sounds better to scale according to display size
rather than frame size (but round up to minimum size e.g. 4096x4096).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-17 21:56           ` Chong Yidong
  2005-10-18  3:39             ` Richard M. Stallman
@ 2005-10-19  9:02             ` Eli Zaretskii
  1 sibling, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2005-10-19  9:02 UTC (permalink / raw)
  Cc: emacs-devel

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Mon, 17 Oct 2005 17:56:39 -0400
> Cc: emacs-devel@gnu.org
> 
> > But I don't think this limit should be absolute.  I think it should be
> > specified as a multiple of the frame height and width, and it should
> > be given as a floating point number.  I'd suggest 2.0 as the default
> > for this ratio.
> 
> What frame should we then use? The selected frame?
> 
> That's not really logical, the frame that issues the call to load an
> image (which is where we check the image size) may not be the frame
> that ends up displaying it.

How about the using display size in pixels (display-pixel-height and
display-pixel-width)?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-19  8:35                 ` Kim F. Storm
@ 2005-10-19 12:20                   ` David Kastrup
  2005-10-19 12:45                     ` Chong Yidong
  2005-10-19 12:51                     ` Kim F. Storm
  0 siblings, 2 replies; 24+ messages in thread
From: David Kastrup @ 2005-10-19 12:20 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel, Stefan Monnier, rms

storm@cua.dk (Kim F. Storm) writes:

> RMS:
>> But I don't think this limit should be absolute.  I think it should be
>> specified as a multiple of the frame height and width, and it should
>> be given as a floating point number.	I'd suggest 2.0 as the default
>> for this ratio.
>
> Stefan:
>> All this to say that I think choosing the maximum image size based on
>> display-pixel-width and display-pixel-height would be preferable than using
>> frame size.
>
> If you use image slicing, you can in principle show a small area of a
> much larger image.  
>
> I don't see how that relates to the dimensions of the frame or display.
>
> But it definitely sounds better to scale according to display size
> rather than frame size (but round up to minimum size e.g. 4096x4096).

It sounds to me like the limits should be configurable, with a
somewhat conservative default.  Applications where larger dimensions
might be appropriate (image viewers with provisions for panning, i.e.)
can allow them in their own buffers using buffer-local settings of the
variables limiting the size.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-19 12:20                   ` David Kastrup
@ 2005-10-19 12:45                     ` Chong Yidong
  2005-10-19 20:17                       ` Richard M. Stallman
  2005-10-19 12:51                     ` Kim F. Storm
  1 sibling, 1 reply; 24+ messages in thread
From: Chong Yidong @ 2005-10-19 12:45 UTC (permalink / raw)
  Cc: emacs-devel, Stefan Monnier, rms, Kim F. Storm

David Kastrup <dak@gnu.org> writes:

>> But it definitely sounds better to scale according to display size
>> rather than frame size (but round up to minimum size e.g. 4096x4096).
>
> It sounds to me like the limits should be configurable, with a
> somewhat conservative default.  Applications where larger dimensions
> might be appropriate (image viewers with provisions for panning, i.e.)
> can allow them in their own buffers using buffer-local settings of the
> variables limiting the size.

Scaling according to display size isn't entirely problem-free either,
because Emacs frames can be put on different X displays ;-)

I think scaling according to frame size is not a great solution, but
it's OK because of the way image loading works.  If the frame is
initially too small, Emacs won't load the image.  But each time you
increase the size of the frame, Emacs checks again, and loads the
image if the size relative to the frame is now OK.  Once an image is
loaded, it's put in the image cache, so it will always be displayed
regardless of the frame size.

The max-image-size code is checked into CVS.  I've set a conservative
value of 6.0 times the frame width and height.  After all, the
original rationale for this feature is to avoid over-allocating memory
in case a malicious image demands a gigantic image widths/heights of
millions of pixels.  In ordinary situations, it shouldn't get in the
way of the user.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-19 12:20                   ` David Kastrup
  2005-10-19 12:45                     ` Chong Yidong
@ 2005-10-19 12:51                     ` Kim F. Storm
  2005-10-19 20:17                       ` Richard M. Stallman
  1 sibling, 1 reply; 24+ messages in thread
From: Kim F. Storm @ 2005-10-19 12:51 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel, Stefan Monnier, rms

David Kastrup <dak@gnu.org> writes:

> It sounds to me like the limits should be configurable, with a
> somewhat conservative default.  Applications where larger dimensions
> might be appropriate (image viewers with provisions for panning, i.e.)
> can allow them in their own buffers using buffer-local settings of the
> variables limiting the size.

I think it makes sense for some purposes to allow max image size to be
specified in absolute pixels.  Here is a patch to do that:


*** image.c	19 Oct 2005 10:20:42 +0200	1.37
--- image.c	19 Oct 2005 11:50:56 +0200	
***************
*** 1163,1179 ****
       int width;
       int height;
  {
!   if (width <= 0 || height <=0)
!     return 0;
  
!   if (FLOATP (Vmax_image_size) && f
!       && ((width > (int)(XFLOAT_DATA (Vmax_image_size)
!   			 * FRAME_PIXEL_WIDTH (f)))
!   	  || (height > (int)(XFLOAT_DATA (Vmax_image_size)
!   			     * FRAME_PIXEL_HEIGHT (f)))))
      return 0;
  
!   return 1;
  }
  
  /* Prepare image IMG for display on frame F.  Must be called before
--- 1163,1191 ----
       int width;
       int height;
  {
!   int w, h;
  
!   if (width <= 0 || height <= 0)
      return 0;
  
!   if (INTEGERP (Vmax_image_size))
!     w = h = XINT (Vmax_image_size);
!   else if (FLOATP (Vmax_image_size))
!     {
!       if (f != NULL)
! 	{
! 	  w = FRAME_PIXEL_WIDTH (f);
! 	  h = FRAME_PIXEL_HEIGHT (f);
! 	}
!       else
! 	w = h = 1024;  /* Arbitrary size for unknown frame. */
!       w = (int) (XFLOAT_DATA (Vmax_image_size) * w);
!       h = (int) (XFLOAT_DATA (Vmax_image_size) * h);
!     }
!   else
!     return 1;
! 
!   return (width <= w && height <= h);
  }
  
  /* Prepare image IMG for display on frame F.  Must be called before
***************
*** 8289,8300 ****
    Fput (intern ("image-library-alist"), Qrisky_local_variable, Qt);
  
    DEFVAR_LISP ("max-image-size", &Vmax_image_size,
!     doc: /* Maximum size of an image, relative to the selected frame.
  
! This is a floating point number that is multiplied by the width and
  height of the selected frame, to give the maximum width and height for
! images.  Emacs will not load an image into memory if its width or
! height exceeds this limit. */);
    Vmax_image_size = make_float (MAX_IMAGE_SIZE);
  
    Vimage_type_cache = Qnil;
--- 8301,8319 ----
    Fput (intern ("image-library-alist"), Qrisky_local_variable, Qt);
  
    DEFVAR_LISP ("max-image-size", &Vmax_image_size,
!     doc: /* Maximum size of images.
! Emacs will not load an image into memory if its pixel width or
! pixel height exceeds this limit.
! 
! If the value is an integer it specifies the absolute maximum pixel
! width and pixel height for images.
  
! If the value is a floating point number it specifies a limit relative
! to the selected frame, i.e. the value is multiplied by the width and
  height of the selected frame, to give the maximum width and height for
! images.
! 
! Otherwise, no limit is placed on images.   */);
    Vmax_image_size = make_float (MAX_IMAGE_SIZE);
  
    Vimage_type_cache = Qnil;


*** display.texi	19 Oct 2005 10:20:40 +0200	1.191
--- display.texi	19 Oct 2005 13:12:43 +0200	
***************
*** 4061,4070 ****
  @defvar max-image-size
  @tindex max-image-size
  This variable is used to define the maximum size of image that Emacs
! will load.  If its value is a floating point number, that number is
! multiplied by the width and height of the selected frame, in pixels,
! to give the maximum image width and height.  Emacs will refuse to load
! and display any image that is larger than this.
  
  The purpose of this variable is to prevent unreasonably large images
  from accidentally being loaded into Emacs.  It only takes effect the
--- 4061,4074 ----
  @defvar max-image-size
  @tindex max-image-size
  This variable is used to define the maximum size of image that Emacs
! will load.  Emacs will refuse to load (and display) any image that is
! larger than this limit.
! 
! If its value is an integer, it specifies the absolute maximum width
! and height, in pixels, for images.  If its value is a floating point
! number, that number is multiplied by the width and height of the
! selected frame, in pixels, to give the maximum image width and height.
! Otherwise, no limit is placed on images.
  
  The purpose of this variable is to prevent unreasonably large images
  from accidentally being loaded into Emacs.  It only takes effect the


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-18 14:33               ` Stefan Monnier
  2005-10-19  8:35                 ` Kim F. Storm
@ 2005-10-19 13:27                 ` Miles Bader
  2005-10-20  4:54                   ` Richard M. Stallman
  1 sibling, 1 reply; 24+ messages in thread
From: Miles Bader @ 2005-10-19 13:27 UTC (permalink / raw)
  Cc: Chong Yidong, rms, emacs-devel

2005/10/18, Stefan Monnier <monnier@iro.umontreal.ca>:
> My Emacs frames are typically 80 columns of 13x6, i.e. about 500 pixel wide,
> but I often (at least once a week) watch digital photos in those frames
> and more often than not those picteures are 1600 or 2048 pixel wide.

Yeah, me too -- in many cases I just wanna check do a quick check, and
seeing just the upper-left corner is good enough, so I do this with
pictures up to about  3840 x 3072 pixels (that's triple my display
size in each dimension).  Emacs scrolling actually works reasonably
well if you only care about the left edge... :-)

I think it's a good idea to have a limit, just make it rather bigger
than any common picture size, e.g., 10,000 x 10,000.  Limiting it to
some multiple of the display size seems like pointless extra work --
Emacs on a modern machine is quite capable of handling even pretty big
pictures, it's just the _insane_ sizes from corrupted or malicious
data that cause problems.

-Miles
--
Do not taunt Happy Fun Ball.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-19 12:45                     ` Chong Yidong
@ 2005-10-19 20:17                       ` Richard M. Stallman
  0 siblings, 0 replies; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-19 20:17 UTC (permalink / raw)
  Cc: emacs-devel, monnier, storm

    The max-image-size code is checked into CVS.  I've set a conservative
    value of 6.0 times the frame width and height.

I am concerned that that is too large.  On large displays, an Emacs
frame could be 1000 pixels in each dimension.  6000 pixels on each
side is 36 million pixels.  Isn't that enough to cause you a lot of
trouble?  It seems to me it should be 3.0, not 6.0.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-19 12:51                     ` Kim F. Storm
@ 2005-10-19 20:17                       ` Richard M. Stallman
  2005-10-19 21:55                         ` Kim F. Storm
  0 siblings, 1 reply; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-19 20:17 UTC (permalink / raw)
  Cc: cyd, monnier, emacs-devel

    I think it makes sense for some purposes to allow max image size to be
    specified in absolute pixels.  Here is a patch to do that:

That is ok.  But instead of this text

    ! If its value is an integer, it specifies the absolute maximum width
    ! and height, in pixels, for images.  If its value is a floating point
    ! number, that number is multiplied by the width and height of the
    ! selected frame, in pixels, to give the maximum image width and height.
    ! Otherwise, no limit is placed on images.

it would be clearer to write this:

    ! If the value is an integer, it directly specifies the maximum
    ! image height and width, measured in pixels.  If it is a floating
    ! point number, it specifies the maximum image height and width
    ! as a ratio to the frame height and width.  If the value is
    ! non-numeric, there is no explicit limit on the size of images.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-19 20:17                       ` Richard M. Stallman
@ 2005-10-19 21:55                         ` Kim F. Storm
  0 siblings, 0 replies; 24+ messages in thread
From: Kim F. Storm @ 2005-10-19 21:55 UTC (permalink / raw)
  Cc: cyd, monnier, emacs-devel

"Richard M. Stallman" <rms@gnu.org> writes:

> it would be clearer to write this:
>
>     ! If the value is an integer, it directly specifies the maximum
>     ! image height and width, measured in pixels.  If it is a floating
>     ! point number, it specifies the maximum image height and width
>     ! as a ratio to the frame height and width.  If the value is
>     ! non-numeric, there is no explicit limit on the size of images.

Much better indeed.  I have committed the changes.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: image size limit?
  2005-10-19 13:27                 ` Miles Bader
@ 2005-10-20  4:54                   ` Richard M. Stallman
  0 siblings, 0 replies; 24+ messages in thread
From: Richard M. Stallman @ 2005-10-20  4:54 UTC (permalink / raw)
  Cc: cyd, monnier, emacs-devel

    Yeah, me too -- in many cases I just wanna check do a quick check, and
    seeing just the upper-left corner is good enough, so I do this with
    pictures up to about  3840 x 3072 pixels (that's triple my display
    size in each dimension).

That is surely an unusual usage.  I think it is ok if this requires
you to set the value of max-image-size.

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2005-10-20  4:54 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-10-11 20:32 image size limit? Chong Yidong
2005-10-11 21:19 ` Kevin Rodgers
2005-10-12 16:24 ` Richard M. Stallman
2005-10-12 17:40   ` Romain Francoise
2005-10-13 20:13     ` Richard M. Stallman
2005-10-14 13:02       ` Chong Yidong
2005-10-15 16:13         ` Richard M. Stallman
2005-10-15 18:33           ` Chong Yidong
2005-10-16 17:36             ` Richard M. Stallman
2005-10-16 22:27       ` Chong Yidong
2005-10-17 17:30         ` Richard M. Stallman
2005-10-17 21:56           ` Chong Yidong
2005-10-18  3:39             ` Richard M. Stallman
2005-10-18 14:33               ` Stefan Monnier
2005-10-19  8:35                 ` Kim F. Storm
2005-10-19 12:20                   ` David Kastrup
2005-10-19 12:45                     ` Chong Yidong
2005-10-19 20:17                       ` Richard M. Stallman
2005-10-19 12:51                     ` Kim F. Storm
2005-10-19 20:17                       ` Richard M. Stallman
2005-10-19 21:55                         ` Kim F. Storm
2005-10-19 13:27                 ` Miles Bader
2005-10-20  4:54                   ` Richard M. Stallman
2005-10-19  9:02             ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).