tripviss / image-resizer

On-the-fly image resizing using Node.js and libvips. Heroku Ready!
MIT License
74 stars 45 forks source link

Crop cut "Bad extract area" #29

Open francesconero opened 7 years ago

francesconero commented 7 years ago

Sharp returns a "Bad extract area" when extracting "out of bounds" portions of images. This means that requesting a bad cut makes the server return a 500 code. This is not really ideal for a production environment.

Wouldn't it be better to simply return a sensible default (like ignoring the crop or switching to a different crop mode) while issuing a 400 (as in a "Bad request" was made)?

teohhanhui commented 7 years ago

What about returning 404 to indicate that the requested size is not available?

On Mon, 14 Nov 2016, 19:58 Francesco Nero, notifications@github.com wrote:

Sharp returns a "Bad extract area" when extracting "out of bounds" portions of images. This means that requesting a bad cut makes the server return a 500 code. This is not really ideal for a production environment.

Wouldn't it be better to simply return a sensible default (like ignoring the crop or switching to a different crop mode) while issuing a 400 (as in a "Bad request" was made)?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tripviss/image-resizer/issues/29, or mute the thread https://github.com/notifications/unsubscribe-auth/AAhf60rLwO0IZ_QMgB5_gY3b9vsNxGhkks5q-EzzgaJpZM4KxPfG .

francesconero commented 7 years ago

Makes sense as well. Would you provide an alternative image or just return the status code without content?

Sorry if kind of off-topic: when using a resizing crop mode (like cfill) the crop focus (x and y) are interpreted as referring to the resized image and not the original, correct? This makes it a little clunky client side as in first you need to figure out the resized image dimensions (the crop usually has a different aspect ratio so you can't use the crop dimensions) and then calculate the crop focus on the resized image dimensions.

What do you think about requiring x y parameters to be provided as referring to the original image size dimensions?

Sorry for the convoluted explanation, I hope you understood what I mean.

teohhanhui commented 7 years ago

404 should not return an image.

I agree that the cropping needs to be reworked, especially cpad which is just wrong.