From f0ad921f3884356057f02643d0a628d0ac6e0aeb Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Sun, 30 Aug 2026 15:27:19 -0400 Subject: [PATCH] Fix the response to a geometry that has no pixels A crop offset at the image edge ended the request with 500. An offset past the edge produced an image of one pixel. go-dims responds with 400 status code for both. - Respond with 400 status code for a crop region that lies off the image. - Respond with 400 status code when MagickCropImage fails. The status was 500. - Fill a crop side of zero from the image before the cut. - Clamp a resize and a watermark overlay to one pixel a side. --- docs/docs/operations/transformations/crop.md | 8 ++- .../docs/operations/transformations/resize.md | 3 + src/operations/crop.c | 11 +++- src/operations/geometry.c | 46 +++++++++++++ src/operations/geometry.h | 19 ++++++ src/operations/legacy.c | 24 ++++++- src/operations/operations.h | 18 ++++++ src/operations/resize.c | 3 + src/operations/thumbnail.c | 11 +++- src/operations/watermark.c | 9 +++ .../grid.TestCropOffsetOnTheEdge.golden.png | Bin 0 -> 82 bytes ...rid.TestCropXOffsetOutsideImage.golden.png | Bin 94 -> 0 bytes ...rid.TestCropYOffsetOutsideImage.golden.png | Bin 94 -> 0 bytes .../grid.TestResizeBelowOnePixel.golden.png | Bin 0 -> 80 bytes ...unit.grid.TestCropWithoutHeight.golden.png | Bin 0 -> 2395 bytes ...rid.TestCropXOffsetOutsideImage.golden.png | Bin 188 -> 0 bytes ...rid.TestCropYOffsetOutsideImage.golden.png | Bin 188 -> 0 bytes ...it.grid.TestResizeBelowOnePixel.golden.png | Bin 0 -> 146 bytes test/http/test_crop.c | 44 +++++++++++-- test/http/test_no_error_image.c | 34 ++++++++++ test/http/test_resize.c | 11 ++++ test/http/test_watermark.c | 31 +++++++++ test/unit/test_crop.c | 61 ++++++++++++++++-- test/unit/test_resize.c | 13 ++++ 24 files changed, 332 insertions(+), 14 deletions(-) create mode 100644 test/golden/grid.TestCropOffsetOnTheEdge.golden.png delete mode 100644 test/golden/grid.TestCropXOffsetOutsideImage.golden.png delete mode 100644 test/golden/grid.TestCropYOffsetOutsideImage.golden.png create mode 100644 test/golden/grid.TestResizeBelowOnePixel.golden.png create mode 100644 test/golden/unit.grid.TestCropWithoutHeight.golden.png delete mode 100644 test/golden/unit.grid.TestCropXOffsetOutsideImage.golden.png delete mode 100644 test/golden/unit.grid.TestCropYOffsetOutsideImage.golden.png create mode 100644 test/golden/unit.grid.TestResizeBelowOnePixel.golden.png diff --git a/docs/docs/operations/transformations/crop.md b/docs/docs/operations/transformations/crop.md index 20b8d18..ee81a68 100644 --- a/docs/docs/operations/transformations/crop.md +++ b/docs/docs/operations/transformations/crop.md @@ -12,8 +12,12 @@ crop/100x100 100 by 100 from the centre crop/50%x50%+0+0 the top left quarter ``` -A region larger than the image is reduced to fit. An offset outside the image -produces an empty result. +A region larger than the image is reduced to fit, so a crop that overhangs an +edge keeps the part that is on the image. + +A region that lies entirely off the image is refused with `400`. An offset of +exactly the width or the height leaves nothing, so it is refused the same way. +The offset one pixel inside the edge keeps a strip one pixel wide. Some clients escape a `+` as `%20`, which arrives as a space. `crop` converts a space back to a `+`, so `100x100%20350%200` works. diff --git a/docs/docs/operations/transformations/resize.md b/docs/docs/operations/transformations/resize.md index 2996925..ee809d2 100644 --- a/docs/docs/operations/transformations/resize.md +++ b/docs/docs/operations/transformations/resize.md @@ -18,5 +18,8 @@ resize/50% half the size A JPEG source is resampled at 2x1x1 chroma subsampling. +A side never falls below one pixel. A percentage rounds down, so a small image +scaled by a small percentage would otherwise ask for a side of no pixels. + [`DimsOptimizeResize`](/configuration/output) makes ImageMagick decode a large JPEG at a smaller scale first, which is faster and slightly softer. diff --git a/src/operations/crop.c b/src/operations/crop.c index e569288..cf9d214 100644 --- a/src/operations/crop.c +++ b/src/operations/crop.c @@ -5,6 +5,7 @@ */ #include "operations.h" +#include "geometry.h" apr_status_t dims_crop_operation (dims_request_rec *d, char *args, const char **err) { @@ -41,7 +42,15 @@ dims_crop_operation (dims_request_rec *d, char *args, const char **err) { DestroyExceptionInfo(ex_info); - MAGICK_CHECK(MagickCropImage(d->wand, rec.width, rec.height, rec.x, rec.y), d); + /* A region that lies off the image has nothing to cut out. go-dims answers + * 400 for it, and a signature is portable between the two. */ + if (!dims_geometry_crop_fits(GetImageFromMagickWand(d->wand), &rec)) { + *err = "The crop region lies outside the image"; + return DIMS_BAD_ARGUMENTS; + } + + MAGICK_CROP_CHECK(MagickCropImage(d->wand, rec.width, rec.height, rec.x, rec.y), + d, err, "The crop region lies outside the image"); MAGICK_CHECK(MagickSetImagePage(d->wand, rec.width, rec.height, rec.x, rec.y), d); return DIMS_SUCCESS; diff --git a/src/operations/geometry.c b/src/operations/geometry.c index 2ca0565..ef1d312 100644 --- a/src/operations/geometry.c +++ b/src/operations/geometry.c @@ -26,3 +26,49 @@ dims_parse_size_geometry(const Image *image, const char *geometry, return ParseMetaGeometry(geometry, ®ion->x, ®ion->y, ®ion->width, ®ion->height); } + +int +dims_geometry_crop_fits(const Image *image, RectangleInfo *region) +{ + ssize_t columns = (ssize_t) image->columns; + ssize_t rows = (ssize_t) image->rows; + ssize_t left = (region->x > 0) ? region->x : 0; + ssize_t top = (region->y > 0) ? region->y : 0; + + /* + * A side of zero means the rest of the image. ParseGravityGeometry fills + * that in from the image, ParseAbsoluteGeometry does not, so the callers + * that use the second one arrive here with one. + */ + if (region->width == 0 && columns > left) { + region->width = (size_t) (columns - left); + } + if (region->height == 0 && rows > top) { + region->height = (size_t) (rows - top); + } + + if (region->width == 0 || region->height == 0) { + return 0; + } + + /* Wholly right of, below, left of, or above the image. */ + if (region->x >= columns || region->y >= rows || + region->x + (ssize_t) region->width <= 0 || + region->y + (ssize_t) region->height <= 0) { + return 0; + } + + return 1; +} + +void +dims_geometry_least_one_pixel(RectangleInfo *region) +{ + if (region->width == 0) { + region->width = 1; + } + + if (region->height == 0) { + region->height = 1; + } +} diff --git a/src/operations/geometry.h b/src/operations/geometry.h index a73c71d..07aabf7 100644 --- a/src/operations/geometry.h +++ b/src/operations/geometry.h @@ -20,4 +20,23 @@ MagickStatusType dims_parse_size_geometry(const Image *image, const char *geometry, RectangleInfo *region); +/* + * Completes a crop region and reports whether any of it lies on the image. + * + * A side of zero becomes the rest of the image, measured from the offset. + * Returns 0 when nothing remains, which happens when the offset sits at or + * past an edge. ImageMagick refuses that crop, and go-dims answers 400 for the + * same region, so the caller returns DIMS_BAD_ARGUMENTS and the two agree. + */ +int dims_geometry_crop_fits(const Image *image, RectangleInfo *region); + +/* + * Raises a width or a height of zero to one. + * + * A percentage rounds down, so a small enough image scaled by a small enough + * percentage asks for a side of no pixels. No format holds such an image, and + * ImageMagick refuses to build one. + */ +void dims_geometry_least_one_pixel(RectangleInfo *region); + #endif diff --git a/src/operations/legacy.c b/src/operations/legacy.c index 83b06a5..7ec2ae4 100644 --- a/src/operations/legacy.c +++ b/src/operations/legacy.c @@ -39,11 +39,20 @@ dims_legacy_crop_operation (dims_request_rec *d, char *args, const char **err) { x = (width / 2) - (rec.width / 2); y = (height / 2) - (rec.height / 2); + rec.x = x; + rec.y = y; + + if (!dims_geometry_crop_fits(GetImageFromMagickWand(d->wand), &rec)) { + *err = "The crop region lies outside the image"; + return DIMS_BAD_ARGUMENTS; + } + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, d->r, "legacy_crop will crop to %ldx%ld+%d+%d", rec.width, rec.height, x, y); - MAGICK_CHECK(MagickCropImage(d->wand, rec.width, rec.height, x, y), d); + MAGICK_CROP_CHECK(MagickCropImage(d->wand, rec.width, rec.height, rec.x, rec.y), + d, err, "The crop region lies outside the image"); return DIMS_SUCCESS; } @@ -62,6 +71,8 @@ dims_legacy_thumbnail_operation (dims_request_rec *d, char *args, const char **e return DIMS_FAILURE; } + dims_geometry_least_one_pixel(&rec); + char *format = MagickGetImageFormat(d->wand); if (strcmp(format, "JPEG") == 0) { const double factors[3] = { 2.0, 1.0, 1.0 }; @@ -89,10 +100,19 @@ dims_legacy_thumbnail_operation (dims_request_rec *d, char *args, const char **e x = (width / 2) - (rec.width / 2); y = (height / 2) - (rec.height / 2); + rec.x = x; + rec.y = y; + + if (!dims_geometry_crop_fits(GetImageFromMagickWand(d->wand), &rec)) { + *err = "The crop region lies outside the image"; + return DIMS_BAD_ARGUMENTS; + } + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, d->r, "legacy_thumbnail will crop to %ldx%ld+%d+%d", rec.width, rec.height, x, y); - MAGICK_CHECK(MagickCropImage(d->wand, rec.width, rec.height, x, y), d); + MAGICK_CROP_CHECK(MagickCropImage(d->wand, rec.width, rec.height, rec.x, rec.y), + d, err, "The crop region lies outside the image"); return DIMS_SUCCESS; } diff --git a/src/operations/operations.h b/src/operations/operations.h index 503166c..5aeda5d 100644 --- a/src/operations/operations.h +++ b/src/operations/operations.h @@ -34,4 +34,22 @@ } \ } while (0) +/* + * Runs a crop and returns a bad request when ImageMagick refuses it. + * + * The geometry checks above catch the regions go-dims names, and this catches + * the rest. A region ImageMagick will not cut is the caller's geometry, not a + * fault in the service, so it must not end the request with 500. + */ +#define MAGICK_CROP_CHECK(func, rec, err, message) \ + do { \ + apr_status_t code = func; \ + if (rec->status == DIMS_IMAGEMAGICK_TIMEOUT) { \ + return DIMS_IMAGEMAGICK_TIMEOUT; \ + } else if (code == MagickFalse) { \ + *(err) = (message); \ + return DIMS_BAD_ARGUMENTS; \ + } \ + } while (0) + #endif diff --git a/src/operations/resize.c b/src/operations/resize.c index 61426c1..5de760b 100644 --- a/src/operations/resize.c +++ b/src/operations/resize.c @@ -18,6 +18,8 @@ dims_resize_operation (dims_request_rec *d, char *args, const char **err) { return DIMS_FAILURE; } + dims_geometry_least_one_pixel(&rec); + char *format = MagickGetImageFormat(d->wand); if (strcmp(format, "JPEG") == 0) { const double factors[3] = { 2.0, 1.0, 1.0 }; @@ -32,6 +34,7 @@ dims_resize_operation (dims_request_rec *d, char *args, const char **err) { RectangleInfo sampleRec = rec; sampleRec.width *= d->optimize_resize; sampleRec.height *= d->optimize_resize; + dims_geometry_least_one_pixel(&sampleRec); orig_width = MagickGetImageWidth(d->wand); orig_height = MagickGetImageHeight(d->wand); diff --git a/src/operations/thumbnail.c b/src/operations/thumbnail.c index a4bfb7a..edfd8f1 100644 --- a/src/operations/thumbnail.c +++ b/src/operations/thumbnail.c @@ -19,6 +19,8 @@ dims_thumbnail_operation (dims_request_rec *d, char *args, const char **err) { return DIMS_FAILURE; } + dims_geometry_least_one_pixel(&rec); + char *format = MagickGetImageFormat(d->wand); if (strcmp(format, "JPEG") == 0) { const double factors[3] = { 2.0, 1.0, 1.0 }; @@ -33,6 +35,7 @@ dims_thumbnail_operation (dims_request_rec *d, char *args, const char **err) { RectangleInfo sampleRec = rec; sampleRec.width *= d->optimize_resize; sampleRec.height *= d->optimize_resize; + dims_geometry_least_one_pixel(&sampleRec); orig_width = MagickGetImageWidth(d->wand); orig_height = MagickGetImageHeight(d->wand); @@ -52,7 +55,13 @@ dims_thumbnail_operation (dims_request_rec *d, char *args, const char **err) { return DIMS_FAILURE; } - MAGICK_CHECK(MagickCropImage(d->wand, rec.width, rec.height, rec.x, rec.y), d); + if (!dims_geometry_crop_fits(GetImageFromMagickWand(d->wand), &rec)) { + *err = "The crop region lies outside the image"; + return DIMS_BAD_ARGUMENTS; + } + + MAGICK_CROP_CHECK(MagickCropImage(d->wand, rec.width, rec.height, rec.x, rec.y), + d, err, "The crop region lies outside the image"); } MAGICK_CHECK(MagickSetImagePage(d->wand, rec.width, rec.height, rec.x, rec.y), d); diff --git a/src/operations/watermark.c b/src/operations/watermark.c index 0f13c2b..5bfe1c7 100644 --- a/src/operations/watermark.c +++ b/src/operations/watermark.c @@ -306,6 +306,15 @@ dims_watermark_operation (dims_request_rec *d, char *args, const char **err) { final_height = largest_size; } + /* A small source scales the overlay to less than a pixel, and no image has + * a side of none. */ + if (final_width < 1) { + final_width = 1; + } + if (final_height < 1) { + final_height = 1; + } + if (MagickScaleImage(overlay_wand, final_width, final_height) == MagickFalse) { status = (d->status == DIMS_IMAGEMAGICK_TIMEOUT) ? DIMS_IMAGEMAGICK_TIMEOUT : DIMS_FAILURE; diff --git a/test/golden/grid.TestCropOffsetOnTheEdge.golden.png b/test/golden/grid.TestCropOffsetOnTheEdge.golden.png new file mode 100644 index 0000000000000000000000000000000000000000..06d660c434fc6dc1df5dfdeeab3da6f62a5d2b5e GIT binary patch literal 82 zcmeAS@N?(olHy`uVBq!ia0vp^j6iGzA{e$d^r(Xb@{9aJ7|$;%p9SRbc)B=-a9mGL aSYQj}r7$ptyDMh_Sqz@8elF{r5}E*VJrOVf literal 0 HcmV?d00001 diff --git a/test/golden/grid.TestCropXOffsetOutsideImage.golden.png b/test/golden/grid.TestCropXOffsetOutsideImage.golden.png deleted file mode 100644 index 3f15aeb93177b6cd081081cb2ab38586e0eede30..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 94 zcmeAS@N?(olHy`uVBq!ia0vp^j35jm7|ip2ssJgbk|4iepwN`R>p)WZMSdZS=NFaF j0&=)KT^vI=t|w;z*-Z?Mcb&7o09g#4u6{1-oD!Mp)WZMSdZS=NFaF j0&=)KT^vI=t|w;z*-Z?Mcb&7o09g#4u6{1-oD!M8QcA!p+-hn@W=_G*8)-|zc=-@m@D`?_x(_jcQX+KmDL z?C@}R@c{rKZy}&8SKz)lXCODdbM8J~03;j$08Ry9Qy#%n0FX@pm<|TuXdVDs(M9zq zY~>3m^q89qklkQ(G|4BGX!pQa091GVT?kN6qz!;_kB5uXDazp7z3#vvcMVt48jto9 zD(?S!p2#mWQ`Jl~gAbXUf=?!UA-~pSoq~Om4dm8re_rdgYk%mjVY6DxRm4c1-8;1@vFzs^7d2oE>jd}#c12u7>_)PXpJhaU(1A>NkpN1@vYdo@qyTxCmmADcO& zlt--1t_lagpL4ugJ5?Q6ryAu+vQ_wPqMzj44i)=nSJW33u6dH!(s^ojwx(=zZ_G}j+N zG>kROxraT1_p=y$3=D1Jm&S>^MEau|!50u677>)`2E`&M^~{zmW-fp_|3&jhA%a__ z!#o=`Xc}IbO+Z`bf()W|TcPEV^taFEC@@FI`(;ifq5VbK$X(zY(big zW@f8G;O-@l#FQ$W;y>wcT)pwu=F=6p-}+guvYP7=+Yt=9wa0|!3s33gj>yb37{zI= z!wbs}>x{N{s_KOyRScWL#L^}0oW0BV2;}K+mbtHG(u0+U?)0MS>Lc2WRQ0MfyNa87Aid6=1cT!2 z3c}=l594gp!F_#+RLnD%(sl9z2*@4d`_;F_dBjSc>$| z3Q)b~a81b}gl)k+)Yp*>q4Z_QK`iKRVhhJ zXfxk{MwBJR=UcWUiWCwrXEv?WyZMrZG&hCw_jPi8nY0=+&1X24Eh4@*#_x(-%B-QKoDe+ z)ZcZ;?02Lb>bZH`+2_QOmW$+;kBy2H-O4?2>1;(FFM|B|R!aU!xIIzw3hNrKm7;B&#s#@T8{=%I>WZzp#!PM>_P z45bDP46hOx7_4S6Fo+k-*%fF5lwc|e@(TuPpYnGdh?nH;?!v%$w@Y{lkjGiz5n0T@ zz%2~Ij105pNB{)|k`w*Ppq2u;%wXK()YA!MaCy2ohHzX@&H%ET7#i<7XMF*(7(8A5 KT-G@yGywoAY9=KB diff --git a/test/golden/unit.grid.TestCropYOffsetOutsideImage.golden.png b/test/golden/unit.grid.TestCropYOffsetOutsideImage.golden.png deleted file mode 100644 index fc510d4604d1ab72671dded164df259cef8fe3d4..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 188 zcmeAS@N?(olHy`uVBq!ia0vp^j35jm7|ip2ssJg4WRD z45bDP46hOx7_4S6Fo+k-*%fF5lwc|e@(TuPpYnGdh?nH;?!v%$w@Y{lkjGiz5n0T@ zz%2~Ij105pNB{)|k`w*Ppq2u;Ai&JPb?u)zkiq5Y;uyklJvjr&ZenP>>zwrkB=70! K=d#Wzp$Pykpe95B diff --git a/test/golden/unit.grid.TestResizeBelowOnePixel.golden.png b/test/golden/unit.grid.TestResizeBelowOnePixel.golden.png new file mode 100644 index 0000000000000000000000000000000000000000..da58f6c6ffa8c418d296e03e65fb9146ac226fc4 GIT binary patch literal 146 zcmeAS@N?(olHy`uVBq!ia0vp^j3CSbBp9sfW`_bPg=CK)Uj~LMH3o);76yi2K%s^g z3=E|P3=FRlfU0INFo+k-*%fF5lweBoc6VX;-`;;_Kaj^+;1OBOz`!jG!i)^F=12eq jxjbDQLpZJ{|6pNY_`>knJvT}RD9YgJ>gTe~DWM4fJSiZW literal 0 HcmV?d00001 diff --git a/test/http/test_crop.c b/test/http/test_crop.c index 4f38f3d..c5445d6 100644 --- a/test/http/test_crop.c +++ b/test/http/test_crop.c @@ -35,18 +35,53 @@ test_crop_region_larger_than_image(void) -1, -1); } +/* + * A region entirely outside the image has nothing to cut out, so the command + * is refused and this server answers with its error image. The status the + * refusal carries is on the server that has no error image, in + * test_no_error_image.c. + */ +static void +assert_refused(const char *commands, const char *what) +{ + dims_response *fallback = dims_request_ops(commands, "missing.png"); + dims_response *refused = dims_request_ops(commands, "grid.png"); + + CHECK(fallback->body_len == refused->body_len && + memcmp(fallback->body, refused->body, refused->body_len) == 0, + "%s must be refused", what); + + dims_response_free(fallback); + dims_response_free(refused); +} + static void test_crop_x_offset_outside_image(void) { - dims_run_golden("TestCropXOffsetOutsideImage", "grid.png", "crop/256x256+768+768", - -1, -1); + assert_refused("crop/256x256+768+768", "an x offset past the image"); } static void test_crop_y_offset_outside_image(void) { - dims_run_golden("TestCropYOffsetOutsideImage", "grid.png", "crop/256x256+0+768", - -1, -1); + assert_refused("crop/256x256+0+768", "a y offset past the image"); +} + +/* + * An offset of exactly the width leaves a region no pixels wide. The offsets + * on either side of it are accepted, so only this one value used to end the + * request with 500. + * + * Not a port. go-dims covers the offsets further out, not the one on the edge. + */ +static void +test_crop_offset_on_the_edge(void) +{ + assert_refused("crop/50x50+512+0", "an x offset of exactly the width"); + + /* One column remains, so the crop keeps a strip one pixel wide. */ + dims_run_golden("TestCropOffsetOnTheEdge", "grid.png", "crop/50x50+511+0", + 1, 50); } const dims_test dims_tests_crop[] = { @@ -56,5 +91,6 @@ const dims_test dims_tests_crop[] = { { "TestCropRegionLargerThanImage", test_crop_region_larger_than_image, NULL }, { "TestCropXOffsetOutsideImage", test_crop_x_offset_outside_image, NULL }, { "TestCropYOffsetOutsideImage", test_crop_y_offset_outside_image, NULL }, + { "TestCropOffsetOnTheEdge", test_crop_offset_on_the_edge, NULL }, DIMS_TEST_END }; diff --git a/test/http/test_no_error_image.c b/test/http/test_no_error_image.c index b12765e..0be1c70 100644 --- a/test/http/test_no_error_image.c +++ b/test/http/test_no_error_image.c @@ -186,6 +186,38 @@ test_watermark_short_argument_list_is_a_bad_request(void) free(url); } +/* + * A crop region entirely off the image, and one whose offset is exactly the + * width. Both leave nothing to cut out. go-dims refuses the first pair, and + * the second used to answer 500 while the offsets on either side of it were + * accepted. + */ +static void +test_crop_outside_the_image_is_a_bad_request(void) +{ + char *url = dims_fixture_url("grid.png"); + char *path; + + path = dims_sign_dims4("crop/256x256+768+768", url, NULL, NULL); + CHECK_INT(status_of(path), 400, "an x offset past the image"); + free(path); + + path = dims_sign_dims4("crop/256x256+0+768", url, NULL, NULL); + CHECK_INT(status_of(path), 400, "a y offset past the image"); + free(path); + + path = dims_sign_dims4("crop/50x50+512+0", url, NULL, NULL); + CHECK_INT(status_of(path), 400, "an x offset of exactly the width"); + free(path); + + /* One column remains, so this one is a crop rather than a refusal. */ + path = dims_sign_dims4("crop/50x50+511+0", url, NULL, NULL); + CHECK_INT(status_of(path), 200, "one pixel inside the edge"); + free(path); + + free(url); +} + const dims_test dims_tests_no_error_image[] = { { "TestUnknownClientIsAServerError", test_unknown_client_is_a_server_error, NULL }, { "TestWrongSignatureIsABadRequest", test_wrong_signature_is_a_bad_request, NULL }, @@ -196,6 +228,8 @@ const dims_test dims_tests_no_error_image[] = { test_quality_out_of_range_is_a_bad_request, NULL }, { "TestWatermarkShortArgumentListIsABadRequest", test_watermark_short_argument_list_is_a_bad_request, NULL }, + { "TestCropOutsideTheImageIsABadRequest", + test_crop_outside_the_image_is_a_bad_request, NULL }, { "TestBadGeometryReportsTheModuleStatus", test_bad_geometry_reports_the_module_status, NULL }, DIMS_TEST_END diff --git a/test/http/test_resize.c b/test/http/test_resize.c index b2a2c0e..5799101 100644 --- a/test/http/test_resize.c +++ b/test/http/test_resize.c @@ -63,6 +63,16 @@ test_resize_fill_with_crop(void) 50, 100); } +/* + * A percentage that rounds a side to no pixels answers with a single pixel + * rather than ending the request. + */ +static void +test_resize_below_one_pixel(void) +{ + dims_run_golden("TestResizeBelowOnePixel", "grid.png", "resize/0.1%", 1, 1); +} + const dims_test dims_tests_resize[] = { { "TestResize", test_resize, NULL }, { "TestResizeOnlySmaller", test_resize_only_smaller, NULL }, @@ -72,5 +82,6 @@ const dims_test dims_tests_resize[] = { { "TestResizeIgnoreAspectRatioWidthOnly", test_resize_ignore_aspect_ratio_width_only, NULL }, { "TestResizeFill", test_resize_fill, NULL }, { "TestResizeFillWithCrop", test_resize_fill_with_crop, NULL }, + { "TestResizeBelowOnePixel", test_resize_below_one_pixel, NULL }, DIMS_TEST_END }; diff --git a/test/http/test_watermark.c b/test/http/test_watermark.c index 56073b5..9e11364 100644 --- a/test/http/test_watermark.c +++ b/test/http/test_watermark.c @@ -212,7 +212,36 @@ test_watermark_cached_overlay_is_revalidated(void) free(url); } +/* + * The overlay is scaled to a fraction of the larger side of the source, so a + * source of one pixel scales it to less than a pixel. No image has a side of + * none, so the overlay keeps one and the request is answered. + */ +static void +test_watermark_on_a_one_pixel_source(void) +{ + char *url = dims_fixture_url("grid.png"); + char *overlay = dims_fixture_url("overlay.png"); + char *encoded = dims_urlencode(overlay); + char extra[1024]; + char *path; + dims_response *response; + + snprintf(extra, sizeof(extra), "overlay=%s", encoded); + path = dims_sign_dims4("resize/1x1!/watermark/0.2,0.8,ne", url, extra, "overlay"); + response = dims_get(path); + + CHECK_INT(response->status, 200, "a watermark on a one pixel source"); + + dims_response_free(response); + free(path); + free(encoded); + free(overlay); + free(url); +} + const dims_test dims_tests_watermark[] = { + { "TestWatermark", test_watermark, NULL }, { "TestWatermarkShortArgumentsAreStable", test_watermark_short_arguments_are_stable, NULL }, @@ -222,5 +251,7 @@ const dims_test dims_tests_watermark[] = { test_watermark_overlay_off_allowlist_is_refused, NULL }, { "TestWatermarkCachedOverlayIsRevalidated", test_watermark_cached_overlay_is_revalidated, NULL }, + { "TestWatermarkOnAOnePixelSource", + test_watermark_on_a_one_pixel_source, NULL }, DIMS_TEST_END }; diff --git a/test/unit/test_crop.c b/test/unit/test_crop.c index a1ace21..a351856 100644 --- a/test/unit/test_crop.c +++ b/test/unit/test_crop.c @@ -20,10 +20,61 @@ CROP(TestCropPercent, "50%x50%+50%+0", 256, 256) CROP(TestCropPercentWithAbsolute, "50%x50%+50%+256", 256, 256) CROP(TestCropRegionLargerThanImage, "512x512+0+256", 512, 256) -/* A region entirely outside the image collapses to one pixel rather than - * failing. Recorded, not endorsed. */ -CROP(TestCropXOffsetOutsideImage, "256x256+768+768", 1, 1) -CROP(TestCropYOffsetOutsideImage, "256x256+0+768", 1, 1) +/* + * A region entirely outside the image has nothing to cut out, so the command + * is refused. go-dims answers the same two arguments with an error, and a + * request that carries them gets 400 from either project. + */ +static void +TestCropXOffsetOutsideImage_case(void) +{ + CHECK_INT(dims_operation_status("grid.png", dims_crop_operation, + "256x256+768+768"), + DIMS_BAD_ARGUMENTS, "an x offset past the image"); +} + +static void +TestCropYOffsetOutsideImage_case(void) +{ + CHECK_INT(dims_operation_status("grid.png", dims_crop_operation, + "256x256+0+768"), + DIMS_BAD_ARGUMENTS, "a y offset past the image"); +} + +/* + * An offset of exactly the width leaves a region no pixels wide. ImageMagick + * refuses that crop, and the offsets on either side of it are accepted, so + * only this one value used to end the request with 500. + * + * This case and the one below it are not ports. go-dims covers the offsets + * further out, not the one on the edge. + */ +static void +TestCropOffsetOnTheEdge_case(void) +{ + CHECK_INT(dims_operation_status("grid.png", dims_crop_operation, + "50x50+512+0"), + DIMS_BAD_ARGUMENTS, "an x offset of exactly the width"); + CHECK_INT(dims_operation_status("grid.png", dims_crop_operation, + "50x50+0+512"), + DIMS_BAD_ARGUMENTS, "a y offset of exactly the height"); + + /* One column remains at the offset before it, so that one is a crop. */ + CHECK_INT(dims_operation_status("grid.png", dims_crop_operation, + "50x50+511+0"), + DIMS_SUCCESS, "one pixel inside the edge"); +} + +/* + * A width or a height of zero means the rest of the image, which ImageMagick + * fills in. Neither empties the region. + */ +static void +TestCropWithoutHeight_case(void) +{ + dims_run_operation("TestCropWithoutHeight", "grid.png", dims_crop_operation, + "256x+0+0", 256, 512); +} /* * The command replaces a space with a plus before parsing, because some user @@ -43,6 +94,8 @@ const dims_test dims_tests_unit_crop[] = { { "TestCropRegionLargerThanImage", TestCropRegionLargerThanImage_case, NULL }, { "TestCropXOffsetOutsideImage", TestCropXOffsetOutsideImage_case, NULL }, { "TestCropYOffsetOutsideImage", TestCropYOffsetOutsideImage_case, NULL }, + { "TestCropOffsetOnTheEdge", TestCropOffsetOnTheEdge_case, NULL }, + { "TestCropWithoutHeight", TestCropWithoutHeight_case, NULL }, { "TestCropAcceptsSpacesForPlus", test_crop_accepts_spaces_for_plus, NULL }, DIMS_TEST_END }; diff --git a/test/unit/test_resize.c b/test/unit/test_resize.c index bf5369b..7f84eed 100644 --- a/test/unit/test_resize.c +++ b/test/unit/test_resize.c @@ -33,6 +33,18 @@ test_resize_rejects_bad_geometry(void) CHECK(code != DIMS_SUCCESS, "an unparsable geometry must not succeed"); } +/* + * A percentage rounds down, so a small image scaled by a small percentage asks + * for a side of no pixels. No image has one, so the side becomes a single + * pixel and the command succeeds. + */ +static void +TestResizeBelowOnePixel_case(void) +{ + dims_run_operation("TestResizeBelowOnePixel", "grid.png", dims_resize_operation, + "0.1%", 1, 1); +} + const dims_test dims_tests_unit_resize[] = { { "TestResize", TestResize_case, NULL }, { "TestResizeOnlySmaller", TestResizeOnlySmaller_case, NULL }, @@ -41,6 +53,7 @@ const dims_test dims_tests_unit_resize[] = { { "TestResizeIgnoreAspectRatio", TestResizeIgnoreAspectRatio_case, NULL }, { "TestResizeIgnoreAspectRatioWidthOnly", TestResizeIgnoreAspectRatioWidthOnly_case, NULL }, { "TestResizeFill", TestResizeFill_case, NULL }, + { "TestResizeBelowOnePixel", TestResizeBelowOnePixel_case, NULL }, { "TestResizeRejectsBadGeometry", test_resize_rejects_bad_geometry, NULL }, DIMS_TEST_END };