Skip to content
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

Unable to generate short url for signed request #18

Open
ryanwi opened this issue May 12, 2016 · 5 comments
Open

Unable to generate short url for signed request #18

ryanwi opened this issue May 12, 2016 · 5 comments

Comments

@ryanwi
Copy link
Contributor

ryanwi commented May 12, 2016

Does the shortener support signed requests?

Given a standard thumbor signed request, this works fine:

http://localhost:8888/oNfnJ36sAQ-CEMVDFTgZ-DfpZ0Y=/360x220/smart/https://82bda53d6c07527f63d4-bb56d6c11261cc2ec250960b8872f9f2.ssl.cf1.rackcdn.com/roster_full_photos/168/original/e44118bf-cf2e-4777-8810-dea67f3e413f.jpg

But, shortening the url fails:

curl -X "POST" "http://localhost:8888/shortener/oNfnJ36sAQ-CEMVDFTgZ-DfpZ0Y=/360x220/smart/https://82bda53d6c07527f63d4-bb56d6c11261cc2ec250960b8872f9f2.ssl.cf1.rackcdn.com/roster_full_photos/168/original/e44118bf-cf2e-4777-8810-dea67f3e413f.jpg"

With the logs as follows:

2016-05-12 14:44:28 thumbor:WARNING Malformed URL: /shortener/oNfnJ36sAQ-CEMVDFTgZ-DfpZ0Y=/360x220/smart/https://82bda53d6c07527f63d4-bb56d6c11261cc2ec250960b8872f9f2.ssl.cf1.rackcdn.com/roster_full_photos/168/original/e44118bf-cf2e-4777-8810-dea67f3e413f.jpg
2016-05-12 14:44:28 tornado.access:WARNING 400 POST /shortener/oNfnJ36sAQ-CEMVDFTgZ-DfpZ0Y=/360x220/smart/https://82bda53d6c07527f63d4-bb56d6c11261cc2ec250960b8872f9f2.ssl.cf1.rackcdn.com/roster_full_photos/168/original/e44118bf-cf2e-4777-8810-dea67f3e413f.jpg (127.0.0.1) 1360.13ms

From what I can tell, the call to yield self.check_image(options) will fail because the method does not account for shortener being in the url. It will look like this:

(Pdb) url = self.request.path
(Pdb) url
'/shortener/oNfnJ36sAQ-CEMVDFTgZ-DfpZ0Y=/360x220/smart/https://82bda53d6c07527f63d4-bb56d6c11261cc2ec250960b8872f9f2.ssl.cf1.rackcdn.com/roster_full_photos/168/original/e44118bf-cf2e-4777-8810-dea67f3e413f.jpg'
(Pdb) url_to_validate = url.replace('/%s/' % options['hash'], '').replace('/%s/' % quote(options['hash']), '')
(Pdb) url_to_validate
'/shortener360x220/smart/https://82bda53d6c07527f63d4-bb56d6c11261cc2ec250960b8872f9f2.ssl.cf1.rackcdn.com/roster_full_photos/168/original/e44118bf-cf2e-4777-8810-dea67f3e413f.jpg'
(Pdb) url_signature = options['hash']
(Pdb) valid = signer.validate(unquote(url_signature), url_to_validate)
(Pdb) valid
False
@masom
Copy link
Contributor

masom commented May 13, 2016

The shortener should support signed requests, you might want to send the URL as a POST parameter instead of being in the URL.

Technically speaking the image source should be URL encoded due to the presence of a http:// section.

https://github.com/thumbor-community/shortener/blob/master/tc_shortener/handlers/shortener.py#L65 should let you do a JSON POST to /shortener with a payload containing a url parameter.

@masom
Copy link
Contributor

masom commented May 13, 2016

curl -X "POST" --data "url=/oNfnJ36sAQ-CEMVDFTgZ-DfpZ0Y=/360x220/smart/https://82bda53d6c07527f63d4-bb56d6c11261cc2ec250960b8872f9f2.ssl.cf1.rackcdn.com/roster_full_photos/168/original/e44118bf-cf2e-4777-8810-dea67f3e413f.jpg" "http://localhost:8888/shortener"

There should have been vows/tests covering that. Sorry about the lack of documentation.

@masom
Copy link
Contributor

masom commented May 13, 2016

Something seems to be broken, investigating.

@ryanwi
Copy link
Contributor Author

ryanwi commented May 13, 2016

Thanks for looking. Sending the url param in the POST body did not seem to be working, is that what you're investigating?

@masom
Copy link
Contributor

masom commented May 13, 2016

Yeah in part. I'm currently adding new tests and fixing a few issues you recently found.

For example the shortened url when preserving the file name will use the path basename instead of the full URL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants