Skip to content
This repository has been archived by the owner on Mar 13, 2023. It is now read-only.

Settle on a way to handle body size limit on proxy requests #39

Open
deifyed opened this issue Jul 6, 2020 · 3 comments
Open

Settle on a way to handle body size limit on proxy requests #39

deifyed opened this issue Jul 6, 2020 · 3 comments
Labels
bug Something isn't working question Further information is requested

Comments

@deifyed
Copy link
Member

deifyed commented Jul 6, 2020

When sending big requests (files/pictures/videos) through the proxy, Gatekeeper will block the request with an 413 request entity too large error. The limit is per now set to 1mb.

I suggest (without thinking about it really) that we set the limit to infinity, or something really high and let reverse proxies handle request limits. A counter argument is that not everyone uses reverse proxies, but I suggest we implement it when and if its needed. Debugging request limiting is a pain in the. .

I ninjamerged a temp fix here: #38

Any thoughts on how we should handle this?

@deifyed deifyed added bug Something isn't working question Further information is requested labels Jul 6, 2020
@yngvark
Copy link

yngvark commented Jul 7, 2020

let reverse proxies handle request limits

Not quite sure I understand this. The Gatekeeper is a reverse proxy for a destination/upstream/backend, so I'm assuming you mean let the upstream handle request limits - which makes sense to me, so we can set a limit one place instead of two. And it also makes sense to let the upstream decide this, instead of having some value in the Gatekeeper, which may or may not work for all the X upstreams the Gatekeeper serves.

"not everyone uses reverse proxies"

Not sure what this means?

Maybe a potential pitfall with unlimited limit is that it allows posting huge payloads to the gatekeeper, filling its memory. Though I don't think this is an issue, as Express uses streaming by default: https://github.com/villadora/express-http-proxy#streaming, as I guess any sane proxy should.

@yngvark yngvark removed their assignment Jul 7, 2020
@deifyed
Copy link
Member Author

deifyed commented Jul 9, 2020

What I mean by reverse proxies is either an k8s ingress or just an nginx/apache/* instance in front of the Gatekeeper.

Your described pitfall is indeed a problem, so a limit should exist - but I argue it should be in the ingress or any other reverse proxy

@yngvark
Copy link

yngvark commented Jul 9, 2020

Right, I see. I agree, either the reverse proxy or the upstream can set the limit.

@deifyed deifyed removed their assignment Jul 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants