BKWLD / croppa

Image thumbnail creation through specially formatted URLs for Laravel.
MIT License
494 stars 91 forks source link

Token in querystring crashs when use in conjunction with some CDNs #149

Open SomosAMambo opened 7 years ago

SomosAMambo commented 7 years ago

Guys,

Some CDNs (Cloudflare, Photon, etc) ignores querystring variables when requesting the original files. So:

http://domain.com/uploads/02/12/helder_turismo-200x200.jpg?token=b41ba0e0913219fa9cfea7d0e212f289

Becomes:

http://domain.com/uploads/02/12/helder_turismo-200x200.jpg Which causes a token mismatch error.

My suggestion is to incorporate the token to filename, like the dimension:

http://domain.com/uploads/02/12/helder_turismo-200x200-b41ba0e0913219fa9cfea7d0e212f289.jpg

This could be bad for image SEO. If this is a concern, the token could be a "folder":

http://domain.com/uploads/02/12/b41ba0e0913219fa9cfea7d0e212f289/200x200/helder_turismo.jpg

We know that we always can set the signing_keyto false in config/croppa.php, but in this case we have to worry about the max number of crops, security, abuse, etc.

weotch commented 7 years ago

Huh, that hasn't been our experience with Cloudflare. We're on the free plan on our own site and URIs like http://www.bukwild.com/uploads/02/12/BKWLD-TEAM-GRADED-CROP2-1920x864-1366x768.png?token=b797dc07220f36c0b4ab7ec555292f72 are working:

You can see that Cloudflare is serving in the headers:

Still I'm open to this.

SomosAMambo commented 7 years ago

For some reason our stage is breaking with Cloudflare. Anyway, we have implemented a regex that replaces the response changing images url to Wordpress Photon.

domain.com/upload/01/03/test.jpg becomes i3.wp.com/domain.com/upload/01/03/test.jpg

Even urlencoding the query string ?token=000... to %3Ftoken%3D000... their API can't forward the request with it.

See: https://wordpress.org/support/topic/maintain-querystring-with-photon-image/

So... for cases like this, we imagine that adding the token to the path or file name is a good option. Maybe like an option via config?

weotch commented 7 years ago

Yeah, if you wanna PR that I'm down for it

SomosAMambo commented 7 years ago

Will do!