bottlepy / bottle

bottle.py is a fast and simple micro-framework for python web-applications.
http://bottlepy.org/
MIT License
8.38k stars 1.46k forks source link

Broken response headers when using plugin mounted from another sub-application #637

Open dobo90 opened 10 years ago

dobo90 commented 10 years ago

Please take a look at following code snippet:

import bottle
from bottle import response

root = bottle.app()
page = bottle.app.push()

@root.route('/')
def root_func():
    return ''

@page.route('/')
def page_func():
    return ''

def plugin(callback):
    def wrapper(*args, **kwargs):
        body = callback(*args, **kwargs)
        response.set_header('Test-Header', 'Test-Value')
        return body
        # return body if not isinstance(body, bottle.HTTPResponse) else body.body
    return wrapper

root.install(plugin)
root.mount('/page', page, skip=None)

bottle.run(app=root)

When you enter http://127.0.0.1:8080/ the Test-Header is set correctly but on http://127.0.0.1:8080/page header isn't set at all. When wrapper function is called from page_func, body variable is instance of HTTPResposne (it's created by Bootle.mount function), when called from root_page it is str. The workaround is to uncomment line in wrapper function.

When body (in wrapper function) is HTTPRepsonse it doesn't contain added header and Bottle._cast method overwrites overall response headers. I haven't found any solution to fix this issue yet. Does anyone have an idea?

dobo90 commented 10 years ago

It seems that status and cookies are also affected. I have a patch proposal but probably it's a workaround. Anyway it passes all unit tests and works as supposed.

--- bottle.py   2014-06-27 01:18:50.120172879 +0200
+++ bottle/bottle.py    2014-06-27 01:31:20.915117085 +0200
@@ -670,6 +670,8 @@
             try:
                 request.path_shift(path_depth)
                 rs = HTTPResponse([])
+                if isinstance(app, Bottle):
+                    rs.is_mounted = True
                 def start_response(status, headerlist, exc_info=None):
                     if exc_info:
                         _raise(*exc_info)
@@ -1694,12 +1696,14 @@
 class HTTPResponse(Response, BottleException):
     def __init__(self, body='', status=None, headers=None, **more_headers):
         super(HTTPResponse, self).__init__(body, status, headers, **more_headers)
+        self.is_mounted = False

     def apply(self, other):
-        other._status_code = self._status_code
-        other._status_line = self._status_line
-        other._headers = self._headers
-        other._cookies = self._cookies
+        if not self.is_mounted:
+            other._status_code = self._status_code
+            other._status_line = self._status_line
+            other._headers = self._headers
+            other._cookies = self._cookies
         other.body = self.body

Edit: I think this patch is more appropriate:

-- bottle.py   2014-06-27 01:18:50.120172879 +0200
+++ bottlepp.py 2014-06-27 11:14:33.906274174 +0200
@@ -670,6 +670,8 @@ class Bottle(object):
             try:
                 request.path_shift(path_depth)
                 rs = HTTPResponse([])
+                if isinstance(app, Bottle):
+                    rs.omit_apply = True
                 def start_response(status, headerlist, exc_info=None):
                     if exc_info:
                         _raise(*exc_info)
@@ -905,7 +907,8 @@ class Bottle(object):
             out = self.error_handler.get(out.status_code, self.default_error_handler)(out)
             return self._cast(out)
         if isinstance(out, HTTPResponse):
-            out.apply(response)
+            if not hasattr(out, 'omit_apply'):
+                out.apply(response)
             return self._cast(out.body)

         # File-like objects.