Defect #35715
closed
File upload fails when run with uWSGI
Added by Cyprien D over 2 years ago.
Updated over 2 years ago.
Description
Hello,
Attachments are broken since I have upgrade my Redmine from 4.2.1 to 4.2.2.
POST fail with a 500 error and this error is logged:
Started POST "/uploads.js?attachment_id=2&filename=placeholder.jpeg&content_type=image%2Fjpeg" for X.X.X.X at 2021-08-05 17:48:28 +0200
Processing by AttachmentsController#upload as JS
Parameters: {"attachment_id"=>"2", "filename"=>"placeholder.jpeg", "content_type"=>"image/jpeg"}
Current user: cyprien (id=121)
Completed 500 Internal Server Error in 5ms (ActiveRecord: 0.9ms)
NoMethodError (undefined method `size' for #<Uwsgi_IO:0x00005566ba40c8d0>):
app/models/attachment.rb:123:in `file='
app/controllers/attachments_controller.rb:108:in `upload'
lib/redmine/sudo_mode.rb:61:in `sudo_mode'
It seems to come from this change introduced to 4.2.2
diff redmine-4.2.1/app/controllers/attachments_controller.rb redmine-4.2.2/app/controllers/attachments_controller.rb
108c108
< @attachment = Attachment.new(:file => request.raw_post)
---
> @attachment = Attachment.new(:file => request.body)
If I switch back from request.body to request.raw_post the error is fixed
Files
I'm not sure if uwsgi is officially supported. The best way would be if they implement the method in their adapter.
https://github.com/unbit/uwsgi/blob/master/plugins/rack/rack_plugin.c
we could use a fallback, but it'll load the whole file into a memory again on adapters that don't support the feature. A compromise?
diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
index 8e28194a3..f62c35976 100644
--- a/app/controllers/attachments_controller.rb
+++ b/app/controllers/attachments_controller.rb
@@ -105,7 +105,7 @@ class AttachmentsController < ApplicationController
return
end
- @attachment = Attachment.new(:file => request.body)
+ @attachment = Attachment.new(:file => request.body.respond_to?(:size) ? request.body : request.raw_post)
@attachment.author = User.current
@attachment.filename = params[:filename].presence || Redmine::Utils.random_hex(16)
@attachment.content_type = params[:content_type].presence
Go MAEDA what do you think?
- Related to Defect #33752: Uploading a big file fails with NoMemoryError added
uwsgi is certainly an unusual application server... However, as restoring basic support for it should be rather simple, and it did work before, I'm in favor of providing a fallback as you proposed.
I'd just recommend to extract the check to a separate method with some comments describing why we are doing this. This could look like:
class AttachmentController
#...
private
# Get an IO-like object for the request body which is usable to create a new
# attachment. We try to avoid having to read the whole body into memory.
def raw_request_body
if request.body.respond_to?(:size)
request.body
else
request.raw_body
end
end
end
Pavel and Holger, thank you for looking into the issue and writing the patch. I will commit 35715.patch.
Setting the target version to 4.2.3.
thanks, I agree, but the target version should be 4.1.5 because the original change went to 4.1.4
and there's also a typo in the new patch, it should be request.raw_post instead of request.raw_body
- Target version changed from 4.2.3 to 4.1.5
Pavel Rosický wrote:
and there's also a typo in the new patch, it should be request.raw_post instead of request.raw_body
Fixed the patch. And I added a test that simulates a request body without the size method. Thank you for pointing it out.
- Subject changed from Attachments fails - 500 Internal Server Error on uploads.js POST to File upload fails when run with uWSGI
- Status changed from New to Resolved
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the fix.
Thank you all for reporting and fixing the issue.
- Status changed from Resolved to Closed
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by 
Updated by