-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Token in querystring crashs when use in conjunction with some CDNs #149
Comments
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. |
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.
Even urlencoding the query string 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? |
Yeah, if you wanna PR that I'm down for it |
Will do! |
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_key
to false inconfig/croppa.php
, but in this case we have to worry about the max number of crops, security, abuse, etc.The text was updated successfully, but these errors were encountered: