Preserve order of views using "accept"

[dobesv]

Our code assumes that the first configured accept predicate will be picked if the client sends Accept: */* or no Accept header at all. However, when multiple views are added with different accept values, they may be shuffled around by the current call to set() and the one selected by Accept: */* is not the first one configured. This patch ensures the media types are offered in the same order they were configured in.

[mmerickel]

Due to the usage of the @view_config decorator, Pyramid views are very explicitly unordered, so I'm not sure this is the best approach to solving the problem. To make a default view that accepts everything, a good approach would be to have a view decorated with @view_config(...) that does not contain an accept predicate. Thus if the views with accept predicates fail, it would fall through to this one. Does this solve your problem?

[dobesv]

@mmerickel,

If the client sends Accept: */* or no accept header at all, the ones with the accept predicate are taken in higher priority than the ones without, so the view config with no accept predicate will never be selected in this case.

We are using imperative configuration so we do know the order in which views are added.

Basically I'm looking for some way to control the order in which views are selected based on the Accept header, if the client has no preference.

In WebOb.their Accept class lets you use a tuple (mime_type, server_priority) which allows the options to be explicitly ordered. However, it doesn't look like pyramid supports this and I'm not using WebOb's Accept class anyway; I had to replace it because it doesn't take the media type parameters into account when ranking matches.

If this patch seems undesirable (although it's a nice simple one and probably can run faster than the old code) then I'll have to devise another workaround. I may need to do that anyway. For example, I could put quality markers into the accept predicate itself and then fork the paste.util.mimeparse library I'm using to apply that in addition to the client's quality preferences.

[dobesv]

I believe I have figured out a workaround for my case - if the Accept header is */* or missing, I just put in my own accept header value that prioritizes the content types that need to be prioritized. Since it's just mobile clients (not web browsers) that are making these kinds of requests, we kind of "know" what types should take priority. I think this will work for me.

I still think this patch is a minor improvement over what was there before but I have an acceptable alternative solution without waiting for this to be merged in.

[mmerickel]

Well I'm -1 on this patch for the reason that it instills an ordering on the view definitions at import time. However, I fully agree that the current accept-header support is totally whack and we should do something about it. In my example below I would've expected some of those default views to be called a little more often than they are, or even conflict with each other.

~❯ curl -D - http://localhost:8080/
HTTP/1.1 200 OK
Content-Length: 11
Content-Type: application/json; charset=UTF-8
Date: Thu, 06 Mar 2014 20:38:55 GMT
Server: waitress

"json view"%                                                                                        
~❯ curl -D - http://localhost:8080/ -H 'Accept: application/json'
HTTP/1.1 200 OK
Content-Length: 11
Content-Type: application/json; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:07 GMT
Server: waitress

"json view"%                                                                                        
~❯ curl -D - http://localhost:8080/ -H 'Accept: text/html'
HTTP/1.1 200 OK
Content-Length: 9
Content-Type: text/html; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:13 GMT
Server: waitress

html view%                                                                                          
~❯ curl -D - http://localhost:8080/ -H 'Accept: */*'
HTTP/1.1 200 OK
Content-Length: 11
Content-Type: application/json; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:34 GMT
Server: waitress

"json view"%                                                                                        
~❯ curl -D - http://localhost:8080/ -H 'Accept: *'
HTTP/1.1 200 OK
Content-Length: 12
Content-Type: text/plain; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:39 GMT
Server: waitress

default view%                                                                                       
~❯ curl -D - http://localhost:8080/ -H 'Accept:'
HTTP/1.1 200 OK
Content-Length: 11
Content-Type: application/json; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:47 GMT
Server: waitress

"json view"%                                                                                        
~❯ curl -D - http://localhost:8080/ -H 'Accept: foo/bar'
HTTP/1.1 500 Internal Server Error
Content-Length: 110
Content-Type: text/plain
Date: Thu, 06 Mar 2014 20:43:21 GMT
Server: waitress

Internal Server Error

The server encountered an unexpected internal server error

(generated by waitress)%

from pyramid.config import Configurator
from pyramid.events import subscriber
from pyramid.view import view_config

@subscriber('pyramid.interfaces.INewRequest')
def new_request(event):
    request = event.request
    print('request: {}'.format(request.url))
    print('headers:\n\t', end='')
    print('\n\t'.join(
        '{}: {}'.format(k, v) for k, v in request.headers.items()))

@view_config(renderer='string')
def default_view(request):
    return 'default view'

@view_config(accept='*', renderer='string')
def default2_view(request):
    return 'string view'

@view_config(accept='*/*', renderer='string')
def default3_view(request):
    return 'another string view'

@view_config(accept='text/html')
def html_view(request):
    response = request.response
    response.text = 'html view'
    response.content_type = 'text/html'
    return response

@view_config(accept='application/json', renderer='json')
def json_view(request):
    return 'json view'

def main(global_config, **app_settings):
    settings = global_config
    settings.update(app_settings)

    config = Configurator(settings=settings)
    config.scan(__name__)
    return config.make_wsgi_app()

if __name__ == '__main__':
    from waitress import serve

    app = main({})
    serve(app, host='0.0.0.0', port=8080)

Note that the final request for foo/bar resulted in the below exception:

ERROR:waitress:Exception when serving /
Traceback (most recent call last):
  File "/Users/michael/work/oss/pyramid/env/lib/python3.3/site-packages/waitress-0.8.8-py3.3.egg/waitress/channel.py", line 337, in service
    task.service()
  File "/Users/michael/work/oss/pyramid/env/lib/python3.3/site-packages/waitress-0.8.8-py3.3.egg/waitress/task.py", line 173, in service
    self.execute()
  File "/Users/michael/work/oss/pyramid/env/lib/python3.3/site-packages/waitress-0.8.8-py3.3.egg/waitress/task.py", line 392, in execute
    app_iter = self.channel.server.application(env, start_response)
  File "/Users/michael/work/oss/pyramid/pyramid/router.py", line 242, in __call__
    response = self.invoke_subrequest(request, use_tweens=True)
  File "/Users/michael/work/oss/pyramid/pyramid/router.py", line 217, in invoke_subrequest
    response = handle_request(request)
  File "/Users/michael/work/oss/pyramid/pyramid/tweens.py", line 21, in excview_tween
    response = handler(request)
  File "/Users/michael/work/oss/pyramid/pyramid/router.py", line 163, in handle_request
    response = view_callable(context, request)
  File "/Users/michael/work/oss/pyramid/pyramid/config/views.py", line 596, in __call__
    return view(context, request)
  File "/Users/michael/work/oss/pyramid/pyramid/config/views.py", line 329, in attr_view
    return view(context, request)
  File "/Users/michael/work/oss/pyramid/pyramid/config/views.py", line 300, in predicate_wrapper
    if not predicate(context, request):
  File "/Users/michael/work/oss/pyramid/pyramid/config/predicates.py", line 136, in __call__
    return self.val in request.accept
  File "/Users/michael/work/oss/pyramid/env/lib/python3.3/site-packages/WebOb-1.3.1-py3.3.egg/webob/acceptparse.py", line 120, in __contains__
    if self._match(mask, offer):
  File "/Users/michael/work/oss/pyramid/env/lib/python3.3/site-packages/WebOb-1.3.1-py3.3.egg/webob/acceptparse.py", line 303, in _match
    _check_offer(offer)
  File "/Users/michael/work/oss/pyramid/env/lib/python3.3/site-packages/WebOb-1.3.1-py3.3.egg/webob/acceptparse.py", line 320, in _check_offer
    raise ValueError("The application should offer specific types, got %r" % offer)
ValueError: The application should offer specific types, got '*'

[dobesv]

Yes, I agree.

I created my own Accept class that uses the paste.util.mimeparse code to
match the media types, the code from WebOb's MIMEAccept class is not very
smart.

I think probably the optimal solution might be to copy and adapt the code
from paste.util.mimeparse into webob.acceptparse.MIMEAccept and submit that
to WebOb, ideally including support for quality values on the server end so
that I could use that to specify a preference on the server side.

It seemed a bit tricky to do this, though - in my case I didn't even
subclass Accept from WebOb, I just implemented whatever I needed in order
to make it work in pyramid. Actually fixing WebOb would require me to make
something suitable for use by any user of WebOb, which seemed a daunting
task.

On Thu, Mar 6, 2014 at 12:42 PM, Michael Merickel
[email protected]:

Well I'm -1 on this patch for the reason that it instills an ordering on
the view definitions at import time. However, I fully agree that the
current accept-header support is totally whack and we should do something
about it. In my example below I would've expected some of those default
views to be called a little more often than they are, or even conflict with
each other.

~❯ curl -D - http://localhost:8080/
HTTP/1.1 http://localhost:8080/HTTP/1.1 200 OK
Content-Length: 11
Content-Type: application/json; charset=UTF-8
Date: Thu, 06 Mar 2014 20:38:55 GMT
Server: waitress

"json view"%
~❯ curl -D - http://localhost:8080/ -H 'Accept: application/json'
HTTP/1.1 200 OK
Content-Length: 11
Content-Type: application/json; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:07 GMT
Server: waitress

"json view"%
~❯ curl -D - http://localhost:8080/ -H 'Accept: text/html'
HTTP/1.1 200 OK
Content-Length: 9
Content-Type: text/html; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:13 GMT
Server: waitress

html view%
~❯ curl -D - http://localhost:8080/ -H 'Accept: /'
HTTP/1.1 200 OK
Content-Length: 11
Content-Type: application/json; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:34 GMT
Server: waitress

"json view"%
~❯ curl -D - http://localhost:8080/ -H 'Accept: *'
HTTP/1.1 200 OK
Content-Length: 12
Content-Type: text/plain; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:39 GMT
Server: waitress

default view%
~❯ curl -D - http://localhost:8080/ -H 'Accept:'
HTTP/1.1 200 OK
Content-Length: 11
Content-Type: application/json; charset=UTF-8
Date: Thu, 06 Mar 2014 20:39:47 GMT
Server: waitress

"json view"%
~❯

from pyramid.config import Configuratorfrom pyramid.events import subscriberfrom pyramid.view import view_config
@subscriber('pyramid.interfaces.INewRequest')def new_request(event):
request = event.request
print('request: {}'.format(request.url))
print('headers:\n\t', end='')
print('\n\t'.join(
'{}: {}'.format(k, v) for k, v in request.headers.items()))
@view_config(renderer='string')def default_view(request):
return 'default view'
@view_config(accept='', renderer='string')def default2_view(request):
return 'string view'
@view_config(accept='/_', renderer='string')def default3_view(request):
return 'another string view'
@view_config(accept='text/html')def html_view(request):
response = request.response
response.text = 'html view'
response.content_type = 'text/html'
return response
@view_config(accept='application/json', renderer='json')def json_view(request):
return 'json view'
def main(global_config, *_app_settings):
settings = global_config
settings.update(app_settings)

config = Configurator(settings=settings)
config.scan(__name__)
return config.make_wsgi_app()

if name == 'main':
from waitress import serve

app = main({})
serve(app, host='0.0.0.0', port=8080)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1259#issuecomment-36934346
.

[mmerickel]

Well if a client says they accept */* and nothing else, they shouldn't be too surprised that they don't get what they were expecting. I assume you're proposing something like accept='application/json;q=0.8', accept='text/html;q=1.0' to resolve that ambiguity. That sounds scary and confusing. In general the only ambiguous situations are */* and no accept header at all. In both cases I'm tempted to say you could define your view with accept='*' and expect it to be invoked in those situations, but I'm kind of just spitballing atm mainly because it's been a little while since I've looked at the accept header spec.

[dobesv]

Treating Accept: */* as application/json, text/html, /;q=0.1` works
fine for our app, but maybe not others.

I'm not suggesting that as a patch to pyramid itself, that would be more
troublesome.

These empty Accept headers are coming in from a bunch of legacy mobile
client code that predates our server actually caring about Accept headers.
Now we added some new options that are available using a different Accept
header but the clients are not aware of this and pyramid was selecting the
wrong variant in this case. However, it's a safe bet (for us) that the old
clients are expecting whatever application/json would have returned. So,
this fixes our problem but isn't a general solution.

On Thu, Mar 6, 2014 at 2:47 PM, Michael Merickel
[email protected]:

Well if a client says they accept / and nothing else, they shouldn't be
too surprised that they don't get what they were expecting. I assume you're
proposing something like accept='application/json;q=0.8',
accept='text/html;q=1.0' to resolve that ambiguity. That sounds scary and
confusing. In general the only ambiguous situations are / and no accept
header at all. In both cases I'm tempted to say you could define your view
with accept='*' and expect it to be invoked in those situations, but I'm
kind of just spitballing atm.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1259#issuecomment-36947415
.

[dobesv]

Perhaps this isn't the time to solve this at the pyramid level, unless you get more complaints or questions about it.