IDAT read buffering correction

The sequential read code failed to read to the end of the IDAT stream in about
1/820 cases, resulting in a spurious warning.  The
png_set_compression_buffer_size API also would not work (or do bad things) if
the size of a zlib uInt was less than 32 bits.

This includes a quiet API change to alter png_set_compression_buffer_size to use
a png_alloc_size_t, not png_size_t and implement the correct checks.

Signed-off-by: John Bowler <jbowler@acm.org>

png.h

@@ -1113,11 +1113,21 @@ PNG_EXPORTA(5, png_structp, png_create_write_struct,
     png_error_ptr warn_fn),
     PNG_ALLOCATED);
 
-PNG_EXPORT(6, size_t, png_get_compression_buffer_size,
+/* These APIs control the sie of the buffer used when reading IDAT chunks in the
+ * sequential read code and the size of the IDAT chunks produced when writing.
+ * They have no effect on the progressive read code.  In both read and write
+ * cases it will be necessary to allocate at least this amount of buffer space.
+ * The default value is PNG_IDAT_READ_SIZE on read and PNG_ZBUF_SIZE on write.
+ *
+ * The valid range is 1..0x7FFFFFFF on write and 1..max(uInt) on read, where
+ * uInt is the type declared by zlib.h.  On write setting the largest value will
+ * typically cause the PNG image data to be written in one chunk; this gives the
+ * smallest PNG and has little or no effect on applications that read the PNG.
+ */
+PNG_EXPORT(6, png_alloc_size_t, png_get_compression_buffer_size,
     (png_const_structrp png_ptr));
-
 PNG_EXPORT(7, void, png_set_compression_buffer_size, (png_structrp png_ptr,
-    size_t size));
+    png_alloc_size_t size));
 
 /* Moved from pngconf.h in 1.4.0 and modified to ensure setjmp/longjmp
  * match up.

pngget.c

@@ -1144,7 +1144,7 @@ png_get_user_chunk_ptr(png_const_structrp png_ptr)
 }
 #endif
 
-png_size_t PNGAPI
+png_alloc_size_t PNGAPI
 png_get_compression_buffer_size(png_const_structrp png_ptr)
 {
    if (png_ptr == NULL)

pngread.c

@@ -308,17 +308,18 @@ png_start_read_image(png_structrp png_ptr)
 static void
 png_read_IDAT(png_structrp png_ptr)
 {
-   /* Read more input data, up to png_struct::IDAT_size, stop at the
-    * end of the IDAT stream:
+   /* Read more input data, up to png_struct::IDAT_size, stop at the end of the
+    * IDAT stream.  pngset.c checks png_struct::IDAT_size to ensure that it will
+    * fit in a uInt.
     */
+   const uInt buffer_size = (uInt)/*SAFE*/png_ptr->IDAT_size;
    uInt IDAT_size = 0;
    png_bytep buffer =
-      png_read_buffer(png_ptr, png_ptr->IDAT_size, 0/*error*/);
+      png_read_buffer(png_ptr, buffer_size, 0/*error*/);
 
    png_ptr->zstream.next_in = buffer;
 
-   while (png_ptr->chunk_name == png_IDAT &&
-          IDAT_size < png_ptr->IDAT_size)
+   while (png_ptr->chunk_name == png_IDAT && IDAT_size < buffer_size)
    {
       png_uint_32 l = png_ptr->chunk_length;
 
@@ -338,11 +339,11 @@ png_read_IDAT(png_structrp png_ptr)
 
       /* Read from the IDAT chunk into the buffer, up to png_struct::IDAT_size:
        */
-      if (l > png_ptr->IDAT_size - IDAT_size) /* SAFE: while check */
-         l = png_ptr->IDAT_size - IDAT_size;
+      if (l > buffer_size - IDAT_size) /* SAFE: while check */
+         l = buffer_size - IDAT_size;
 
       png_crc_read(png_ptr, buffer+IDAT_size, l);
-      IDAT_size += /*SAFE*/l;
+      IDAT_size += (uInt)/*SAFE*/l;
       png_ptr->chunk_length -= l;
    }
 
@@ -612,7 +613,15 @@ png_read_end(png_structrp png_ptr, png_inforp info_ptr)
    {
       if (png_ptr->zowner == png_IDAT)
       {
-         /* Normal case: read to the end of the IDAT chunks. */
+         /* Normal case: read to the end of the IDAT chunks.  In about
+          * 5/PNG_IDAT_READ_SIZE cases (typically that's 1:820) zlib will have
+          * returned all the image data but not read up to the end of the
+          * Adler32 because the end of the stream had not been read.  Make sure
+          * it gets read here:
+          */
+         if (png_ptr->zstream.avail_in == 0)
+            png_read_IDAT(png_ptr);
+
          while (!png_read_finish_IDAT(png_ptr)) {
             /* This will adjust zstream.next/avail_in appropriately and if
              * necessary read the next chunk.  After this avail_in may still

pngset.c

@@ -1596,16 +1596,17 @@ png_set_rows(png_const_structrp png_ptr, png_inforp info_ptr,
 #endif
 
 void PNGAPI
-png_set_compression_buffer_size(png_structrp png_ptr, png_size_t size)
+png_set_compression_buffer_size(png_structrp png_ptr, png_alloc_size_t size)
 {
    if (png_ptr == NULL)
       return;
 
-   if (size == 0 || size > PNG_UINT_31_MAX)
+   if (size == 0 ||
+       size > (png_ptr->read_struct ? ZLIB_IO_MAX : PNG_UINT_31_MAX))
       png_error(png_ptr, "invalid compression buffer size");
 
 #  if (defined PNG_SEQUENTIAL_READ_SUPPORTED) || defined PNG_WRITE_SUPPORTED
-      png_ptr->IDAT_size = (uInt)/*SAFE*/size;
+      png_ptr->IDAT_size = (png_uint_32)/*SAFE*/size;
 #  endif /* SEQUENTIAL_READ || WRITE */
 }
 

pngstruct.h

@@ -576,7 +576,7 @@ struct png_struct_def
 #endif
 
 #if defined(PNG_SEQUENTIAL_READ_SUPPORTED) || defined(PNG_WRITE_SUPPORTED)
-   uInt IDAT_size; /* limit on IDAT read and write IDAT size */
+   png_uint_32 IDAT_size;         /* limit on IDAT read and write IDAT size */
 #endif /* SEQUENTIAL_READ || WRITE */
 
    /* ERROR HANDLING */