Add the ability to configure client_max_body_size

[daemonsy]

Background

By default, nginx uses a maximum of 1m for client_max_body_size. This means requests above this will receive the 413 Request Entity Too Large error.

In my use case, this happened during a resume upload in our job application tracking app, which uses nginx to proxy requests and avoid CORs.

What this does

• Expose client_max_body_size to a configurable value under the server part of the config. This means that it's a global setting.
• Expose max_body_size in static.json so you can set the value. It's just taken as a string.
• Update docs

Need help

• Happy to add a test case but need some direction on what to test. Awesome if there's an example in the current test suite to follow.

[daemonsy]

Hi maintainers, have you got a chance to look at this yet?

[daemonsy]

Second ping :)

[daemonsy]

Hey @hone do you have a chance to take a look at this yet?

[remithomas]

@daemonsy and @hone I have tested and deployed this PR. This is fully working !
I was encountered the same issue (413 Request Entity Too Large error)

[daemonsy]

Hi Heroku, just curious, is this repository still maintained?

[Dhaulagiri]

@daemonsy this looks good to me. let me see if i can figure out a good way for us to test this otherwise we can

[hone]

@daemonsy Hi, thanks for this PR. We aren't ignoring it, but it's probably not as simple as it seems. Most of the current features in static.json tend to be higher level abstractions vs just 1:1 config settings in nginx. I'd think the nginx config name itself, is slightly unclear what it actually does unless you're already familiar with 413 errors. :) I was also doing some research on how other static hosting services handle this, and I noticed that there's no documentation I could find for aerobatic, github pages, or netlify. I assume these services aren't doing a check, which probably isn't what we want to do here. For me there's also the question of, should this be a global setting for something handled per location and/or would we want to expose that to the user. Having a large max request body size, can lead to DoS on endpoints that it isn't intended. Is there a clean way to specify this per location if that's warranted?

As for writing a test, I think we would need to do an actual file upload to a post endpoint on an app that hits that limit, so for instance a 2mb file and ensure we get a 200 back. The inverse can potentially be done too, which is to set it lower than upload something that we would expect to get a 413.

Ultimately, I couldn't find good prior art from other static hosting services and practically we need something like this to get around 413 errors for people. If you have some thoughts on the above, I would appreciate it. Thanks!

[daemonsy]

Thanks @hone for the detailed write up.

I was actually thinking of exactly the same thing when working on the PR, didn't like the client_max_body_size being a departure to the rest of the fields in static.json, which feel less friendly than the rest of the config options.

If we make an assumption that the static buildpack will always be on NGINX, will it make sense to provide an override mechanism that matches 1:1? e.g. nginx.conf.json

After making this PR, we found a few other tweaks we want to do the the nginx settings, having the nginx specific override makes it easier for people to do devopsy stuff, without making the buildpack harder to use for the clear cases.

[hone]

@daemonsy I've been against it so far, since I think that opens up pandora's box, but I'm not steadfast on it. This is probably out of the scope of this issue, but what other options were you looking at? What's the use case you're using the static buildpack for?

[daemonsy]

Hi @hone I didn't get a notification on your last response, sorry I missed it. We're using it mainly to host JS single page apps, built on React / Ember.

On these apps, the key problem we have is dealing with CORs.

https://m.alphasights.com/using-nginx-on-heroku-to-serve-single-page-apps-and-avoid-cors-5d013b171a45

For more color on how we came to the static buildpack, we used to use divshot because it advertised itself to be a SPA hoster. Most of our apps are on Heroku and after a while, we moved off divshot and wanted Heroku to be the base of all our apps. At first we used superstatic but moved to heroku static buildpack because it's simpler. I think that the market clearly has a need for static app hosting that's geared towards being good at SPAs. That's how I view this buildpack.

On the additional NGINX options, we are only looking at configuring the proxy_read_timeout currently. The other thing we really want to be able to interpolate ENV variables in the headers section, so we can add in CORs headers for assets type scoped to a HOST based on an env variable instead of hardcoding it

[esbanarango]

+1

[daemonsy]

Hi @hone we also have some gateway errors with the API proxying too. With some research, it seems to be caused by NGINX caching the resolution response, this is actually causing us issues on our Heroku single page apps proxying to backend apps.

https://tenzer.dk/nginx-with-dynamic-upstreams/

There's another issue opened up on this repository that describes this better.

#44

[jsdevel]

A solution to this really is needed, as it's something that comes up too often. static.json provides for reverse proxying. The ability to configure said proxies is a must. Right now I'm forking this repo to add this PR on top of master for a hotfix in a production app I'm working on.

[Skaradumau]

Can someone update it?

[jcfinnerup]

Is anything happening here?

[Luisetelo]

hi all, waiting for this PR to be merged too...

cheers!

[karianpour]

I wonder if there is any solution for this problem. I cannot believe that the PR is waiting more than 3 years.

[edmorley]

Hi

This buildpack is now deprecated and we are recommending people move the more actively maintained heroku-buildpack-nginx. For migration advice see here.

As such, I'm closing this PR out since we won't be making further changes to this buildpack.