Skip to content

nginx: allow large file uploads for /api/v4/files endpoint#184

Open
madest92 wants to merge 1 commit intomattermost:mainfrom
madest92:max-upload-size
Open

nginx: allow large file uploads for /api/v4/files endpoint#184
madest92 wants to merge 1 commit intomattermost:mainfrom
madest92:max-upload-size

Conversation

@madest92
Copy link
Copy Markdown

This prevents 413 (Request Entity Too Large) errors when uploading large files.

Key changes:

  • Added a separate location for the file upload endpoint
  • Removed request size limit at nginx level (client_max_body_size 0)
  • Increased client_body_timeout to support slow/large uploads
  • Simplified proxy configuration for this endpoint

Note:
File size limits are still enforced at the application level via Mattermost settings, where administrators can configure the maximum allowed upload size.
This change only removes nginx as a limiting factor and delegates control to the application.
Maximum File Size

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 25, 2026

📝 Walkthrough

Walkthrough

A new Nginx location block was added for /api/v4/files to handle GET/POST requests, routing to the backend upstream with custom timeout and body size constraints.

Changes

Cohort / File(s) Summary
Nginx Configuration
nginx/conf.d/default.conf
Added exact-match location block for /api/v4/files endpoint with no client body size limit (client_max_body_size 0), extended client body timeout (1200s), proxy read timeout (600s), and proxy headers configuration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding support for large file uploads to the /api/v4/files endpoint by removing nginx size limits.
Description check ✅ Passed The description is directly related to the changeset, explaining the purpose of the nginx configuration changes, key modifications, and application-level controls.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
nginx/conf.d/default.conf (1)

113-126: Consider adding proxy_request_buffering off and proxy_send_timeout for large uploads.

The configuration correctly removes the nginx body size limit and increases timeouts. However, for large file uploads, consider these improvements:

  1. proxy_request_buffering off; – Streams the upload directly to the backend without buffering in nginx memory, reducing memory pressure for large files.

  2. proxy_send_timeout – Currently missing; defaults to 60s. For slow uploads of large files, this timeout could be reached while nginx sends the request body to the backend. Consider setting it to match proxy_read_timeout (600s) or higher.

♻️ Suggested improvement
 location = /api/v4/files {
     client_max_body_size 0;
     client_body_timeout 1200s;
+    proxy_request_buffering off;
     proxy_set_header Connection "";
     proxy_set_header Host $http_host;
     proxy_set_header X-Real-IP $remote_addr;
     proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
     proxy_set_header X-Forwarded-Proto $scheme;
     proxy_set_header X-Frame-Options SAMEORIGIN;
     proxy_set_header Early-Data $ssl_early_data;
+    proxy_send_timeout 600s;
     proxy_read_timeout 600s;
     proxy_http_version 1.1;
     proxy_pass http://backend;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@nginx/conf.d/default.conf` around lines 113 - 126, In the location block
matching "= /api/v4/files" add proxy_request_buffering off to stream request
bodies to the backend (avoid nginx buffering large uploads) and set
proxy_send_timeout (e.g., 600s to match proxy_read_timeout) so nginx won't time
out while sending the request body to the upstream; update the location =
/api/v4/files block to include these two directives alongside the existing
proxy_read_timeout and proxy_http_version settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@nginx/conf.d/default.conf`:
- Line 113: The nginx config currently has an exact match location "location =
/api/v4/files" which raises client_max_body_size for legacy single-file uploads
but does not cover resumable/chunked uploads under "/api/v4/uploads"; if your
Mattermost uses resumable uploads add a corresponding location block for the
uploads endpoints (e.g. a prefix or regex location for "/api/v4/uploads" and
"/api/v4/uploads/*") and set the same client_max_body_size value as used for
"location = /api/v4/files" so chunk initialization (POST /api/v4/uploads) and
chunk POSTs (POST /api/v4/uploads/{upload_id}) are not rejected. Ensure the new
block(s) use the same directives (client_max_body_size and any other relevant
proxy/fastcgi settings) and test upload flows after deploying the change.

---

Nitpick comments:
In `@nginx/conf.d/default.conf`:
- Around line 113-126: In the location block matching "= /api/v4/files" add
proxy_request_buffering off to stream request bodies to the backend (avoid nginx
buffering large uploads) and set proxy_send_timeout (e.g., 600s to match
proxy_read_timeout) so nginx won't time out while sending the request body to
the upstream; update the location = /api/v4/files block to include these two
directives alongside the existing proxy_read_timeout and proxy_http_version
settings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d9d0e957-bc4c-4666-8d44-4818d6f8e68a

📥 Commits

Reviewing files that changed from the base of the PR and between 1423a77 and c6252ba.

📒 Files selected for processing (1)
  • nginx/conf.d/default.conf

@madest92
Copy link
Copy Markdown
Author

@coderabbitai approve

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 25, 2026

✅ Actions performed

Comments resolved and changes approved.

@madest92
Copy link
Copy Markdown
Author

madest92 commented Apr 2, 2026

@hanzei Maybe you can take a look at this PR?

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

Successfully merging this pull request may close these issues.

1 participant