bolt / thumbs

Thumbnail handler for Bolt Images
12 stars 16 forks source link

[RFC] Limit thumb sizes to config presets (2.x) #28

Closed GDmac closed 8 years ago

GDmac commented 8 years ago

This adds the option to limit the amount of image sizes that are allowed to be generated. It looks if config thumbnails presets are set, and then limits the allowed sizes to those presets.

The presets take the following format (defined in app/config/config.yml under thumbnails)

thumbnails:
  presets:
    mythumb : { size: [ 160,  120] }
    myimage : { size: [1000,  750] }
    dynamic_height : { size: [400,  0] }

The sub-array format is choosen in anticipation of https://github.com/bolt/bolt/issues/3703 which might/will allow other parameters (cropping, watermarking) and which might/will allow usage of presets in twig templates as parameters for record.image|thumb(preset_name)

GwendolenLynch commented 8 years ago

Added to Tuesday's agenda

bobdenotter commented 8 years ago

This does not include having the 'key' in the URL, like /thumbs/myimage/foo.jpg, correct?

GDmac commented 8 years ago

Affirmative, no key in url. Only test that width-x-height size from url is found in allowed presets. With BC fallback (allow all) if presets are not defined.

Note: preset key in url is currently NOT in the proposal as i see it ! presets can in the proposal only be used in Twig (record.image|thumb(preset_name)) and then translate to a thumb url. e.g. from above presets, /path/image.jpg|thumb(mythumb) translates to thumbs/160x120c/path/image.jpg (edit2: is having keys in the url a requested feature? /thumbs/myimage/foo.jpg )

CarsonF commented 8 years ago

This is a new feature on a stable, released branch :-1:

GDmac commented 8 years ago

IMO this is a "security" fix against unlimited thumb requests generating files. You call it a "new feature", that is a somewhat formal approach to the issue.

bobdenotter commented 8 years ago

It's really not a security issue. If you are legitimately worried about someone bringing down your server by requesting a ton of thumbnails, then adding this feature will not prevent that. If this is a problem for the site, use varnish, cloudflare or some other CDN to offload the images.

cdowdy commented 8 years ago

It's really not a security issue

@bobdenotter I agree maybe not security but definitely a ddos vector.

Really this is no different than drupals DDos vector[1] they patched ~3 years ago where on demand thumbnail generation can bring a site to a halt through so many requests, fill up the disk space, or max out its CPU.
It is also why liip imagine's bundle implemented hmac[2] like I believe it was @rossriley mentioned in the forums.

Varnish won't "fix" this. These would be dynamic urls, created on request and Varnish wouldn't know about them to cache them, because they don't exist until the request, until after they are generated.

or some other CDN to offload the images.

how does an "editor" or "frontend designer" offload thumbnails to a CDN through bolt currently?

[1] https://www.drupal.org/SA-CORE-2013-002

[2] Liip Imagine's "signer" https://github.com/liip/LiipImagineBundle/blob/master/Imagine/Cache/Signer.php#L23-L32

GwendolenLynch commented 8 years ago

Closed by consensus as #not-for-2.2