Redmine originally used @request.body@, but changed to use @request.raw_post@ in r9652 in order to fix an error regarding fastcgi (#10832). The attached big_files_upload.diff reverts r9652.

I think the patch can be merged to the core if the patch works fine with fastcgi.

--------------------------------------------------------------------------------
Hi @Go MAEDA,
in my opinion, the fix in #10832 is wrong. It breaks the concept of streaming file uploads https://github.com/redmine/redmine/blob/master/app/models/attachment.rb#L126

however, the proposed patch breaks FCGI. I've attached a test case.

unlike other request handlers, CGI uses a rewindable input, that doesn't support the #size method. In theory, the only way how to determine the file size is to read the content first. The content should be streamed into a tempfile not into a memory.
https://github.com/rack/rack/blob/master/lib/rack/rewindable_input.rb

the second option is to rely on the CONTENT-LENGTH header, but it might not be accurate or it could be faked.

after some investigation, I found that Rails actually relies on the header
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/http/request.rb#L327

this means that FCGI is currently broken if an invalid CONTENT-LENGTH is provided. Note that most web-browsers and curl do send this header by default.

I'm wondering why anyone wants to use FCGI these days, they're definitely better options. We should try to fix it if possible, but the original patch #10832 broke other webservers that behave correctly.
--------------------------------------------------------------------------------
I think that the problem with the size can be solved as suggested by Pavel by getting the size after data are stored into the filesystem. See the patch.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
A test is broken after applying attachment:big_files_upload.diff and attachment:file_size.diff.

<pre>
Failure:
Redmine::ApiTest::AttachmentsTest#test_POST_/uploads.xml_should_return_errors_if_file_is_too_big [test/integration/api_test/attachments_test.rb:207]:
Expected response to be a <422: Unprocessable Entity>, but was a <201: Created>
Response body: <?xml version="1.0" encoding="UTF-8"?><upload><id>24</id><token>24.1d1801f753ccd9fa57966c46f360585caf83337a394a5f238d4e4e7d6005788d</token></upload>.
Expected: 422
Actual: 201

bin/rails test test/integration/api_test/attachments_test.rb:204
</pre>
--------------------------------------------------------------------------------
there're validations for size before the file is actually uploaded.

let's choose a simpler approach. These patches should be commited:
attachment:"big_files_upload.diff"
attachment:"10-patches.rb.patch"
attachment:"attachments_test.rb.patch"

it passes all tests.
--------------------------------------------------------------------------------
@Go MAEDA may I ask for a review? it's a simple change.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
+1, that may really help installations on VPS or other small-memory machines. The rack patch for FCGI support has been merged already (not released unfortunately, appears it'll be in Rack 3.0.0)
--------------------------------------------------------------------------------
Pavel Rosický wrote:
> let's choose a simpler approach. These patches should be commited:
> attachment:"big_files_upload.diff"
> attachment:"10-patches.rb.patch"
> attachment:"attachments_test.rb.patch"

Thank you. I have merged the three patches and fixed RuboCop offenses: attachment:33752.patch
--------------------------------------------------------------------------------
Committed the patch. Thank you.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

relates,Closed,35715,File upload fails when run with uWSGI