ipx icon indicating copy to clipboard operation
ipx copied to clipboard

feat: add limits for modifiers and dimensions

Open ausir0726 opened this issue 1 year ago โ€ข 5 comments

๐Ÿ”— Linked issue

#45

โ“ Type of change

  • [ ] ๐Ÿ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • [ ] ๐Ÿž Bug fix (a non-breaking change that fixes an issue)
  • [ ] ๐Ÿ‘Œ Enhancement (improving an existing functionality like performance)
  • [x] โœจ New feature (a non-breaking change that adds functionality)
  • [ ] ๐Ÿงน Chore (updates to the build process or auxiliary tools and libraries)
  • [ ] โš ๏ธ Breaking change (fix or feature that would cause existing functionality to change)

๐Ÿ“š Description

Based on the issues mentioned, using encrypted URLs as a deterrent against abuse should be considered. However, inspired by Cloudinary's approach (where the server stops serving images beyond 8000px, which likely meets current screen usage), and taking into account the limitations on images in the ImageKit.io documentation, we have decided to impose restrictions on the usage of modifiers. Additionally, requests for width or height must conform to the maximum limits. This configuration serves as the minimum guarantee to prevent server abuse when URL encryption is not applied.

Furthermore, an example of using IPX_DOMAINS is added because it defaults to an empty array, but the input value is in the form of a comma-separated string, leading to confusion due to the different data types in use.

ImageKit.io doc about limits image : https://docs.imagekit.io/limits-and-troubleshooting/limits

๐Ÿ“ Checklist

  • [x] I have linked an issue or discussion.
  • [x] I have updated the documentation accordingly.

ausir0726 avatar Jul 26 '23 07:07 ausir0726

Taking into consideration the deployment issues of existing users and to avoid any breaking changes, the default values will retain all settings. By limiting the maximum pixels, potential damage caused by attacks can be minimized.

ausir0726 avatar Jul 26 '23 08:07 ausir0726

Codecov Report

Merging #152 (5af59c2) into main (606400a) will decrease coverage by 0.52%. The diff coverage is 43.24%.

@@            Coverage Diff             @@
##             main     #152      +/-   ##
==========================================
- Coverage   56.91%   56.39%   -0.52%     
==========================================
  Files          10       10              
  Lines         940      977      +37     
  Branches       41       42       +1     
==========================================
+ Hits          535      551      +16     
- Misses        405      426      +21     
Files Changed Coverage ฮ”
src/ipx.ts 79.65% <43.24%> (-6.95%) :arrow_down:

codecov[bot] avatar Jul 27 '23 14:07 codecov[bot]

Is there any way I can continue to promote this PR? Because we're suffering from memory crashes due to mass production of images.

ausir0726 avatar Aug 08 '23 06:08 ausir0726

I guess until @pi0 can have a look at it, you can use your fork version @ausir0726

atinux avatar Aug 08 '23 13:08 atinux

I wonder if validating modifiers via a Joi, zod or typebox schema would be the better approach?

jameswragg avatar Nov 23 '23 16:11 jameswragg