pyramid icon indicating copy to clipboard operation
pyramid copied to clipboard

It should be possible to set Content-Type to text/html when using renderers

Open lieryan opened this issue 11 years ago • 12 comments

Currently, this code will be served to the browser as text/plain thus the HTML are not rendered by the browser:

from wsgiref.simple_server import make_server
from pyramid.config import Configurator

def hello_world(request):
    request.response.content_type = "text/html"
    return "<p>Hello World</p>"

config = Configurator()
config.add_route('hello', '/')
config.add_view(hello_world, route_name='hello', renderer='string')
app = config.make_wsgi_app()
make_server('', 8000, app).serve_forever()

A little bit of investigative work shows that the issue is here:

File: /pyramid/renderers.py
155 def string_renderer_factory(info):
156     def _render(value, system):
157         if not isinstance(value, string_types):
158             value = str(value)
159         request = system.get('request')
160         if request is not None:
161             response = request.response
162             ct = response.content_type
163             if ct == response.default_content_type:
164                 response.content_type = 'text/plain'
165         return value
166     return _render

Here response.default_content_type == 'text/html' and the string renderer replaces the specified content_type with its default of text/plain. I think this is unintuitive/unexpected behavior, instead when request.response.content_type is explicitly set to 'text/html', the renderer should be not change it.

I didn't test the json renderer, but since there is a similar code over there, I'd assume it has the same bug as well.

lieryan avatar May 15 '14 18:05 lieryan

We should try to fix this if we can. My initial idea is to modify pyramid.response.Response to override default_content_type to be some sort of dummy object that can be tested for via response.content_type is response.default_content_type (using is).

mmerickel avatar May 21 '14 15:05 mmerickel

response.content_type is retrieved from headers, so it seems to be difficult to judge whether content_type is overrided or not...

ledmonster avatar Nov 01 '14 07:11 ledmonster

Similar bug with json renderer.

Somehow my default_content_type is None and json renderer won't attach the correct 'Content-Type: application/json' header to response.

        request = system.get('request')
        if request is not None:
            response = request.response
            ct = response.content_type
            if ct == response.default_content_type:
                response.content_type = 'application/json'
        default = self._make_default(request)
        return self.serializer(value, default=default, **self.kw)

jaseemabid avatar Dec 31 '14 07:12 jaseemabid

I think to preserve bw-compat the easiest way is to make a custom DEFAULT_CONTENT_TYPE object inheriting from str that we can do an is comparison on.

mmerickel avatar Jan 08 '15 18:01 mmerickel

I believe I am experiencing a related issue: I am coding my own conditional response (based on etags) and whenever I do:

raise exception_response(result)

my webob test suite complains:

  File "/home/myproject/env/local/lib/python2.7/site-packages/webtest/lint.py", line 483, in check_content_type
    "which must not return content.") % code)
AssertionError: Content-Type header found in a 304 response, which must not return content.

I am still debugging it though.

marplatense avatar Jan 08 '15 18:01 marplatense

This issue only pertains to how renderers affect responses. Your example has nothing to do with that unless you describe further the problem. A 304 repsonse should not be going through a renderer at all.

mmerickel avatar Jan 08 '15 19:01 mmerickel

I attempted a fix here, but webob does its own mangling of the string enough that it's not possible to keep the same object throughout. We need a different approach.

https://github.com/Pylons/pyramid/compare/fix.default-content-type

mmerickel avatar Feb 06 '15 16:02 mmerickel

My fix is here: https://github.com/Pylons/pyramid/pull/1565

It works, but I just feel that there has to be a better way.

digitalresistor avatar Feb 07 '15 07:02 digitalresistor

Some discussion with @mcdonc came up with a possible fix for this. It basically involves adding a response.content_type_changed property to the response object. Upon access of the property it would compare to some initial value in the response.__init__ to determine if the value had changed. I haven't thought it through further than this but adding the property allows us to internalize the logic in the response class making things simpler.

This is obviously what you've started with implicit_content_type but we should be able to fix it to do the computation later on the getter to account for someone doing response.headers['content-type'] = ....

mmerickel avatar Mar 14 '15 00:03 mmerickel

What if someone uses response.headers['content-type'] = 'text/html', now your check in the getter can't know that it was changed and response.content_type_changed will remain false.

digitalresistor avatar Mar 14 '15 02:03 digitalresistor

This should get punted to WebOb instead.

digitalresistor avatar Apr 12 '15 20:04 digitalresistor

Agree with @bertjwregeer (although once we change WebOb we'll probably need to change Pyramid to depend on that new version and might need to change Pyramid too).

mcdonc avatar Apr 13 '15 00:04 mcdonc