TurboGears / backlash

Standalone WebOb port of the Werkzeug Debugger with Python3 support born as a WebError replacement for TurboGears2
MIT License
13 stars 11 forks source link

errors middleware: fix propagation of early .close() of response iterator to the wrapped app #19

Closed kiilerix closed 4 years ago

kiilerix commented 4 years ago

TraceErrorsMiddleware.__call__ returns _report_errors_while_consuming_iter() , wrapping the original app response iterator. Only when exhausted, this iterator would invoke .close() on the wrapped app response iterator.

An iterator .close() method will inject GeneratorExit, but _report_errors_while_consuming_iter didn't handle that exception and did thus in all cases not forward close, violating PEP 333:

If the iterable returned by the application has a close() method, the server or gateway must call that method upon completion of the current request, whether the request was completed normally, or terminated early due to an error (this is to support resource release by the application).

That was a problem, for example if the client web browser cancelled the request and closed the socket before the response had been sent. When the web/WSGI server fails to write chunks from the response iterator, it is right in closing the response iterator before exhausting it.

A severe (real world) consequence of that was when TurboGears DBSessionRemoverMiddleware is used and its _stream_response wasn't closed. The DB session was thus removed but leaked into the next request where it would be reused instead of auto-creating a new one. In best case the request would fail, in worst case data would leak data from one user to another user. (That problem could be avoided if session creation were explicit instead of automatic on-demand.)

Fixed by moving app_iter.close() to a finally clause where it will be run when the iterator receives a GeneratorExit before being exhausted.

kiilerix commented 4 years ago

DebuggedApplication.debug_application seems to have the same problem and the same pattern ... and needs the same fix.

amol- commented 4 years ago

Thanks for catching this!

amol- commented 4 years ago

addressed for DebuggedApplication too in https://github.com/TurboGears/backlash/commit/2d8607ddbd57c982274655519e542e6085e72f38

patrickdepinguin commented 4 years ago

Thanks a lot, do you plan to make a release with these changes?