fix some int32 limits on windows (libvips#3935)

* fix some int32 limits on windows write() on windows uses int for byte counts, leading to errors with very wide (more than 1.5m pixels wide) images * Fix some other occurrences * Add ChangeLog note --------- Co-authored-by: Kleis Auke Wolthuizen <[email protected]>
kleisauke · Jun 18, 2024 · da5b16e · da5b16e
1 parent 0ae442a
commit da5b16e
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 57 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -5,7 +5,8 @@
 - pngload: disable ADLER32/CRC checking in non-fail mode [kleisauke]
 - improve target_clones support check [kleisauke]
 - fix pipe read limit
-- fix a rare crash on windows in highly threaded applications [Julianiolo]
+- fix a rare crash on Windows in highly threaded applications [Julianiolo]
+- vipssave: fix infinite loop on Windows with large images [pdbourke]
 
 12/3/24 8.15.2
 

diff --git a/libvips/deprecated/package.c b/libvips/deprecated/package.c
@@ -323,7 +323,7 @@ static int
 getext_vec(im_object *argv)
 {
 	void **out = (void **) &argv[1];
-	int size;
+	size_t size;
 
 	/* void/char confusion is fine.
 	 */

diff --git a/libvips/foreign/rawsave.c b/libvips/foreign/rawsave.c
@@ -93,16 +93,12 @@ vips_foreign_save_raw_write(VipsRegion *region, VipsRect *area, void *a)
 {
 	VipsForeignSave *save = (VipsForeignSave *) a;
 	VipsForeignSaveRaw *raw = (VipsForeignSaveRaw *) a;
-	int i;
 
-	for (i = 0; i < area->height; i++) {
-		VipsPel *p =
-			VIPS_REGION_ADDR(region, area->left, area->top + i);
-
-		if (vips__write(raw->fd, p,
-				VIPS_IMAGE_SIZEOF_PEL(save->in) * area->width))
+	for (int i = 0; i < area->height; i++)
+		if (vips__write(raw->fd,
+			VIPS_REGION_ADDR(region, area->left, area->top + i),
+			VIPS_IMAGE_SIZEOF_PEL(save->in) * area->width))
 			return -1;
-	}
 
 	return 0;
 }
@@ -207,16 +203,12 @@ vips_foreign_save_raw_fd_write(VipsRegion *region, VipsRect *area, void *a)
 {
 	VipsForeignSave *save = (VipsForeignSave *) a;
 	VipsForeignSaveRawFd *fd = (VipsForeignSaveRawFd *) a;
-	int i;
-
-	for (i = 0; i < area->height; i++) {
-		VipsPel *p =
-			VIPS_REGION_ADDR(region, area->left, area->top + i);
 
-		if (vips__write(fd->fd, p,
-				VIPS_IMAGE_SIZEOF_PEL(save->in) * area->width))
+	for (int i = 0; i < area->height; i++)
+		if (vips__write(fd->fd,
+			VIPS_REGION_ADDR(region, area->left, area->top + i),
+			VIPS_IMAGE_SIZEOF_PEL(save->in) * area->width))
 			return -1;
-	}
 
 	return 0;
 }
@@ -231,8 +223,7 @@ vips_foreign_save_raw_fd_build(VipsObject *object)
 		return -1;
 
 	if (vips_image_pio_input(save->in) ||
-		vips_sink_disc(save->in,
-			vips_foreign_save_raw_fd_write, fd))
+		vips_sink_disc(save->in, vips_foreign_save_raw_fd_write, fd))
 		return -1;
 
 	return 0;

diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h
@@ -222,11 +222,11 @@ int vips__has_extension_block(VipsImage *im);
 /* TODO(kleisauke): VIPS_API is required by vipsheader.
  */
 VIPS_API
-void *vips__read_extension_block(VipsImage *im, int *size);
+void *vips__read_extension_block(VipsImage *im, size_t *size);
 /* TODO(kleisauke): VIPS_API is required by vipsedit.
  */
 VIPS_API
-int vips__write_extension_block(VipsImage *im, void *buf, int size);
+int vips__write_extension_block(VipsImage *im, void *buf, size_t size);
 int vips__writehist(VipsImage *image);
 /* TODO(kleisauke): VIPS_API is required by vipsedit.
  */

diff --git a/libvips/include/vips/util.h b/libvips/include/vips/util.h
@@ -335,8 +335,7 @@ int vips__file_write(void *data, size_t size, size_t nmemb, FILE *stream);
 /* TODO(kleisauke): VIPS_API is required by the magick module.
  */
 VIPS_API
-gint64 vips__get_bytes(const char *filename,
-	unsigned char buf[], gint64 len);
+gint64 vips__get_bytes(const char *filename, unsigned char buf[], gint64 len);
 int vips__fgetc(FILE *fp);
 
 GValue *vips__gvalue_ref_string_new(const char *text);

diff --git a/libvips/iofuncs/generate.c b/libvips/iofuncs/generate.c
@@ -628,15 +628,22 @@ vips_allocate_input_array(VipsImage *out, ...)
 static int
 write_vips(VipsRegion *region, VipsRect *area, void *a)
 {
-	size_t nwritten, count;
+	size_t count;
 	void *buf;
 
 	count = (size_t) region->bpl * area->height;
 	buf = VIPS_REGION_ADDR(region, 0, area->top);
 
 	do {
-		nwritten = write(region->im->fd, buf, count);
-		if (nwritten == (size_t) -1)
+		// write() uses int not size_t on windows, so we need to chunk
+		// ... max 1gb, why not
+		int chunk_size = VIPS_MIN(1024 * 1024 * 1024, count);
+		ssize_t nwritten = write(region->im->fd, buf, chunk_size);
+
+		/* n == 0 isn't strictly an error, but we treat it as
+		 * one to make sure we don't get stuck in this loop.
+		 */
+		if (nwritten <= 0)
 			return errno;
 
 		buf = (void *) ((char *) buf + nwritten);

diff --git a/libvips/iofuncs/image.c b/libvips/iofuncs/image.c
@@ -3194,7 +3194,7 @@ vips_image_write_prepare(VipsImage *image)
 int
 vips_image_write_line(VipsImage *image, int ypos, VipsPel *linebuffer)
 {
-	int linesize = VIPS_IMAGE_SIZEOF_LINE(image);
+	guint64 linesize = VIPS_IMAGE_SIZEOF_LINE(image);
 
 	/* Is this the start of eval?
 	 */
@@ -3215,8 +3215,7 @@ vips_image_write_line(VipsImage *image, int ypos, VipsPel *linebuffer)
 	switch (image->dtype) {
 	case VIPS_IMAGE_SETBUF:
 	case VIPS_IMAGE_SETBUF_FOREIGN:
-		memcpy(VIPS_IMAGE_ADDR(image, 0, ypos),
-			linebuffer, linesize);
+		memcpy(VIPS_IMAGE_ADDR(image, 0, ypos), linebuffer, linesize);
 		break;
 
 	case VIPS_IMAGE_OPENOUT:
@@ -3227,10 +3226,8 @@ vips_image_write_line(VipsImage *image, int ypos, VipsPel *linebuffer)
 		break;
 
 	default:
-		vips_error("VipsImage",
-			_("unable to output to a %s image"),
-			vips_enum_string(VIPS_TYPE_IMAGE_TYPE,
-				image->dtype));
+		vips_error("VipsImage", _("unable to output to a %s image"),
+			vips_enum_string(VIPS_TYPE_IMAGE_TYPE, image->dtype));
 		return -1;
 	}
 

diff --git a/libvips/iofuncs/target.c b/libvips/iofuncs/target.c
@@ -459,9 +459,10 @@ vips_target_write_unbuffered(VipsTarget *target,
 		return 0;
 
 	while (length > 0) {
-		gint64 bytes_written;
-
-		bytes_written = class->write(target, data, length);
+		// write() uses int not size_t on windows, so we need to chunk
+		// ... max 1gb, why not
+		int chunk_size = VIPS_MIN(1024 * 1024 * 1024, length);
+		gint64 bytes_written = class->write(target, data, chunk_size);
 
 		/* n == 0 isn't strictly an error, but we treat it as
 		 * one to make sure we don't get stuck in this loop.

diff --git a/libvips/iofuncs/util.c b/libvips/iofuncs/util.c
@@ -538,11 +538,16 @@ int
 vips__write(int fd, const void *buf, size_t count)
 {
 	do {
-		size_t nwritten = write(fd, buf, count);
+		// write() uses int not size_t on windows, so we need to chunk
+		// ... max 1gb, why not
+		int chunk_size = VIPS_MIN(1024 * 1024 * 1024, count);
+		ssize_t nwritten = write(fd, buf, chunk_size);
 
-		if (nwritten == (size_t) -1) {
-			vips_error_system(errno, "vips__write",
-				"%s", _("write failed"));
+		/* n == 0 isn't strictly an error, but we treat it as
+		 * one to make sure we don't get stuck in this loop.
+		 */
+		if (nwritten <= 0) {
+			vips_error_system(errno, "vips__write", "%s", _("write failed"));
 			return -1;
 		}
 
@@ -727,8 +732,7 @@ vips__file_read(FILE *fp, const char *filename, size_t *length_out)
 	if (len > 1024 * 1024 * 1024) {
 		/* Over a gb? Seems crazy!
 		 */
-		vips_error("vips__file_read",
-			_("\"%s\" too long"), filename);
+		vips_error("vips__file_read", _("\"%s\" too long"), filename);
 		return NULL;
 	}
 
@@ -750,17 +754,15 @@ vips__file_read(FILE *fp, const char *filename, size_t *length_out)
 			if (size > 1024 * 1024 * 1024 ||
 				!(str2 = realloc(str, size))) {
 				free(str);
-				vips_error("vips__file_read",
-					"%s", _("out of memory"));
+				vips_error("vips__file_read", "%s", _("out of memory"));
 				return NULL;
 			}
 			str = str2;
 
 			/* -1 to allow space for an extra NULL we add later.
 			 */
 			read = fread(str + len, sizeof(char),
-				(size - len - 1) / sizeof(char),
-				fp);
+				(size - len - 1) / sizeof(char), fp);
 			len += read;
 		} while (!feof(fp));
 
@@ -778,8 +780,7 @@ vips__file_read(FILE *fp, const char *filename, size_t *length_out)
 		if (read != (size_t) len) {
 			g_free(str);
 			vips_error("vips__file_read",
-				_("error reading from file \"%s\""),
-				filename);
+				_("error reading from file \"%s\""), filename);
 			return NULL;
 		}
 	}

diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c
@@ -506,7 +506,7 @@ vips__has_extension_block(VipsImage *im)
 /* Read everything after the pixels into memory.
  */
 void *
-vips__read_extension_block(VipsImage *im, int *size)
+vips__read_extension_block(VipsImage *im, size_t *size)
 {
 	gint64 psize;
 	void *buf;
@@ -515,8 +515,7 @@ vips__read_extension_block(VipsImage *im, int *size)
 	g_assert(im->file_length > 0);
 	if (im->file_length - psize > 100 * 1024 * 1024) {
 		vips_error("VipsImage",
-			"%s", _("more than 100 megabytes of XML? "
-					"sufferin' succotash!"));
+			"%s", _("more than 100 megabytes of XML? sufferin' succotash!"));
 		return NULL;
 	}
 	if (im->file_length - psize == 0)
@@ -774,7 +773,7 @@ readhist(VipsImage *im)
 }
 
 int
-vips__write_extension_block(VipsImage *im, void *buf, int size)
+vips__write_extension_block(VipsImage *im, void *buf, size_t size)
 {
 	gint64 length;
 	gint64 psize;
@@ -794,7 +793,7 @@ vips__write_extension_block(VipsImage *im, void *buf, int size)
 		return -1;
 
 #ifdef DEBUG
-	printf("vips__write_extension_block: written %d bytes of XML to %s\n",
+	printf("vips__write_extension_block: written %zd bytes of XML to %s\n",
 		size, im->filename);
 #endif /*DEBUG*/
 

diff --git a/tools/vipsheader.c b/tools/vipsheader.c
@@ -135,10 +135,9 @@ print_header(VipsImage *image, gboolean many)
 	else if (strcmp(main_option_field, "getext") == 0) {
 		if (vips__has_extension_block(image)) {
 			void *buf;
-			int size;
+			size_t size;
 
-			if (!(buf =
-						vips__read_extension_block(image, &size)))
+			if (!(buf = vips__read_extension_block(image, &size)))
 				return -1;
 			printf("%s", (char *) buf);
 			g_free(buf);