plangrid / flask-rebar

Flask-Rebar combines flask, marshmallow, and swagger for robust REST services.
MIT License
231 stars 38 forks source link

Flask-Rebar causes 500 error instead of 413 when MAX_CONTENT_LENGTH exceeded #163

Open twosigmajab opened 4 years ago

twosigmajab commented 4 years ago
# repro.py
from flask import Flask, request                                                        
from flask_rebar import Rebar                                                           

app = Flask(__name__)                                                                   

MAX_CONTENT_LENGTH = 8                                                                  
app.config.from_mapping({"MAX_CONTENT_LENGTH": MAX_CONTENT_LENGTH})                     

rebar = Rebar()                                                                         
registry = rebar.create_handler_registry()                                              
# Comment out the next line and the bug no longer occurs:                                                                                                                
rebar.init_app(app)                                                                     

@app.route("/", methods=["GET", "POST"])                                                
def index():                                                                            
    request.data                                                                        
    return (f"""                                                                        
        Without Flask-Rebar, POSTing more than {MAX_CONTENT_LENGTH}                     
        bytes to this endpoint gives 413 as expected.                                   

        With Flask-Rebar, the catch-all errorhandler it installs                        
        causes an erroneous 500 in this case.                                           
         """,                                                                           
        {"content-type": "text/plain"},                                                 
    )                                                                                   

# With Flask-Rebar we get this buggy 500 Internal Server Error:
➜ curl -d'123456789' -v http://localhost:5000/
> POST / HTTP/1.1
> Content-Length: 9
...
* upload completely sent off: 9 out of 9 bytes
...
[2020-02-02 00:39:39,478] ERROR in app: Exception on / [POST]
Traceback (most recent call last):
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/flask/app.py", line 1949, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/flask/app.py", line 1935, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/jab/tmp/rebar-413-bug/repro.py", line 18, in index
    request.data
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/werkzeug/local.py", line 348, in __getattr__
    return getattr(self._get_current_object(), name)
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/werkzeug/utils.py", line 90, in __get__
    value = self.func(obj)
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/werkzeug/wrappers/base_request.py", line 425, in data
    return self.get_data(parse_form_data=True)
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/werkzeug/wrappers/base_request.py", line 455, in get_data
    self._load_form_data()
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/flask/wrappers.py", line 88, in _load_form_data
    RequestBase._load_form_data(self)
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/werkzeug/wrappers/base_request.py", line 317, in _load_form_data
    data = parser.parse(
  File "/home/jab/tmpvenv/lib/python3.8/site-packages/werkzeug/formparser.py", line 225, in parse
    raise exceptions.RequestEntityTooLarge()
werkzeug.exceptions.RequestEntityTooLarge: 413 Request Entity Too Large: The data value transmitted exceeds the capacity limit.
172.23.170.66 - - [02/Feb/2020 00:39:39] "POST / HTTP/1.1" 500 -
* HTTP 1.0, assume close after body
< HTTP/1.0 500 INTERNAL SERVER ERROR
< Content-Type: application/json
< Content-Length: 50
< Server: Werkzeug/0.16.1 Python/3.8.0
< Date: Sun, 02 Feb 2020 00:39:39 GMT
< 
{"message":"Sorry, there was an internal error."}

# Without Flask-Rebar we get the correct 413 response as expected:
➜ curl -d'123456789' -v http://localhost:5000/
> POST / HTTP/1.1
> Content-Length: 9
...
* upload completely sent off: 9 out of 9 bytes
...
172.23.170.66 - - [02/Feb/2020 00:42:21] "POST / HTTP/1.1" 413 -
* HTTP 1.0, assume close after body
< HTTP/1.0 413 REQUEST ENTITY TOO LARGE
< Content-Type: text/html
< Content-Length: 196
< Server: Werkzeug/0.16.1 Python/3.8.0
< Date: Sun, 02 Feb 2020 00:42:21 GMT
< 
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>413 Request Entity Too Large</title>
<h1>Request Entity Too Large</h1>
<p>The data value transmitted exceeds the capacity limit.</p>
...
airstandley commented 4 years ago

I vote we fix this by simply adding a werkzeug.exceptions.HTTPException error handler that generates the correct 'Error' schema response if the exception if it is not handled by Rebar.uncaught_exception_handlers

I've felt for awhile that this would be more correct behaviour than the patchy handling we have for 301, 308, 400, 404, and 405 HTTPException exceptions.