Upload to S3 Failing

[Thomas-McKanna]

Upload to S3 Failing

[ColdHeat]

This could be related to #2432 but I am unsure since I have not seen S3 uploads breaking. Can you perhaps set your timezone to UTC to see if that allows uploads to work? That will help confirm the source of the issue.

[Thomas-McKanna]

@ColdHeat I did some digging. It looks like the boto3 upload_fileobj function silently closes the file object you give it after upload (see boto/boto3#929).

This unfortunate undocumented behavior, along with the addition of

CTFd/CTFd/utils/uploads/__init__.py

Line 50 in 3536c4c

sha1sum = hash_file(fp=file_obj)

in the last release it causing the issue, since this line occurs after the called to upload_fileobj. This explains why the returned error is "I/O operation on closed file.".

I guess this would only be a problem if using S3 to store files for CTFd.

[ColdHeat]

Thank you for digging into this and figuring out the issue. This is an issue in a feature for 3.7 which explains why I haven't seen it before.

You're welcome to PR a fix but I will tackle this as part of 3.7 as well.

[ColdHeat]

Likely the simplest fix is to calculate the SHA before uploading because we don't close the file and re-seek to the beginning as well.

[Thomas-McKanna]

@ColdHeat I can confirm that moving the hash calculated to before the file upload fixes the issue. I can submit a PR. Any idea when this might be pushed to a new hotfix version (maybe 3.6.2?)? This issue is currently causing me some trouble in preparing for an upcoming event.

[ColdHeat]

Thanks for the PR I stepped out before I could put one up.

I think I would rather wait for 3.7.0. There isn't a significant amount of difference between 3.6.1 and the current commit: 3.6.1...master so if you having issues you could just specifically pin to the 3.6.1 image or to that tag/commit.

[Thomas-McKanna]

Ok, I'll make it work somehow. I was having some issues with pinning to a tag/commit because it seemed to throw off the database.

[ColdHeat]

I think now that your fix is merged you should be able to run off of that since the shasum change is the only change that modified the database.

[Thomas-McKanna]

@ColdHeat Yep that worked. Thank you!