thumbor-community / aws

Thumbor AWS extensions
MIT License
155 stars 70 forks source link

Update bucket.py to prevent error #120

Closed thehappydinoa closed 6 years ago

thehappydinoa commented 6 years ago

Fixes

File "project/venv/lib/python2.7/site-packages/tornado/gen.py", line 307, in wrapper
    yielded = next(result)
  File "project/venv/lib/python2.7/site-packages/thumbor-6.5.1-py2.7-macosx-10.13-x86_64.egg/thumbor/handlers/imaging.py", line 31, in check_image
    exists = yield gen.maybe_future(self.context.modules.storage.exists(kw['image'][:self.context.config.MAX_ID_LENGTH]))
  File "project/venv/lib/python2.7/site-packages/tornado/concurrent.py", line 483, in wrapper
    future.result()
  File "project/venv/lib/python2.7/site-packages/tornado/concurrent.py", line 238, in result
    raise_exc_info(self._exc_info)
  File "project/venv/lib/python2.7/site-packages/tornado/concurrent.py", line 471, in wrapper
    result = f(*args, **kwargs)
  File "project/venv/lib/python2.7/site-packages/tc_aws/aws/storage.py", line 109, in exists
    self.storage.get(file_abspath, callback=return_data)
  File "project/venv/lib/python2.7/site-packages/tornado/concurrent.py", line 483, in wrapper
    future.result()
  File "project/venv/lib/python2.7/site-packages/tornado/concurrent.py", line 238, in result
    raise_exc_info(self._exc_info)
  File "project/venv/lib/python2.7/site-packages/tornado/concurrent.py", line 471, in wrapper
    result = f(*args, **kwargs)
  File "project/venv/lib/python2.7/site-packages/tc_aws/aws/bucket.py", line 71, in get
    Key=self._clean_key(path),
  File "project/venv/lib/python2.7/site-packages/tc_aws/aws/bucket.py", line 146, in _clean_key
    if '/' == key[0]:
IndexError: string index out of range
Bladrak commented 6 years ago

Hello and thanks for contributing :) Could you add a test case to cover this use case? (What was the URL for instance?)

thehappydinoa commented 6 years ago

If the key is "" then it won't have an index of 0.

On Wed, Jul 4, 2018, 9:07 AM Hugo Briand notifications@github.com wrote:

Hello and thanks for contributing :) Could you add a test case to cover this use case? (What was the URL for instance?)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thumbor-community/aws/pull/120#issuecomment-402473880, or mute the thread https://github.com/notifications/unsubscribe-auth/AcxAIufb4HtWc1Z-VNOJd_v8cKguofBWks5uDL30gaJpZM4U_1IT .

Bladrak commented 6 years ago

Sure, but what URL are you testing this with?

thehappydinoa commented 6 years ago

I am unsure, I was having trouble with tornado bc it decided to not send future_exception so it was none. Or something like that.

On Fri, Jul 6, 2018, 4:13 AM Hugo Briand notifications@github.com wrote:

Sure, but what URL are you testing this with?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thumbor-community/aws/pull/120#issuecomment-402961557, or mute the thread https://github.com/notifications/unsubscribe-auth/AcxAIhuQaZiOUwUvTVtInNN8nSTmpO1cks5uDxw1gaJpZM4U_1IT .

Bladrak commented 6 years ago

I'll need at least some steps to reproduce the issue, if not an automated test, to be able to merge this PR

thehappydinoa commented 6 years ago

I was using AWS Cloudfront and running thumbor on heroku. I am really not sure what caused it.

Bladrak commented 6 years ago

I'm sorry but I can't merge a PR without a clear issue, a way to reproduce it, and a test to cover it.