{"action":"created","issue":{"url":"https://api.github.com/repos/Pylons/webob/issues/367","repository_url":"https://api.github.com/repos/Pylons/webob","labels_url":"https://api.github.com/repos/Pylons/webob/issues/367/labels{/name}","comments_url":"https://api.github.com/repos/Pylons/webob/issues/367/comments","events_url":"https://api.github.com/repos/Pylons/webob/issues/367/events","html_url":"https://github.com/Pylons/webob/issues/367","id":354566158,"node_id":"MDU6SXNzdWUzNTQ1NjYxNTg=","number":367,"title":"raise if a media range is passed to best_match","user":{"login":"mmerickel","id":487237,"node_id":"MDQ6VXNlcjQ4NzIzNw==","avatar_url":"https://avatars3.githubusercontent.com/u/487237?v=4","gravatar_id":"","url":"https://api.github.com/users/mmerickel","html_url":"https://github.com/mmerickel","followers_url":"https://api.github.com/users/mmerickel/followers","following_url":"https://api.github.com/users/mmerickel/following{/other_user}","gists_url":"https://api.github.com/users/mmerickel/gists{/gist_id}","starred_url":"https://api.github.com/users/mmerickel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mmerickel/subscriptions","organizations_url":"https://api.github.com/users/mmerickel/orgs","repos_url":"https://api.github.com/users/mmerickel/repos","events_url":"https://api.github.com/users/mmerickel/events{/privacy}","received_events_url":"https://api.github.com/users/mmerickel/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-08-28T04:23:54Z","updated_at":"2018-08-29T21:39:02Z","closed_at":null,"author_association":"MEMBER","body":"Versions of webob prior to 1.6 would always raise, but in 1.6, and 1.7 they are treated as ranges and in 1.8 they are treated as media types.\r\n\r\nVersion 1.5.1:\r\n\r\n```\r\n>>> MIMEAccept('text/html').best_match(['text/*'])\r\nTraceback (most recent call last):\r\n  File \"<stdin>\", line 1, in <module>\r\n  File \"/Users/michael/work/oss/pyramid/env27/lib/python2.7/site-packages/webob/acceptparse.py\", line 176, in best_match\r\n    if self._match(mask, offer):\r\n  File \"/Users/michael/work/oss/pyramid/env27/lib/python2.7/site-packages/webob/acceptparse.py\", line 303, in _match\r\n    _check_offer(offer)\r\n  File \"/Users/michael/work/oss/pyramid/env27/lib/python2.7/site-packages/webob/acceptparse.py\", line 320, in _check_offer\r\n    raise ValueError(\"The application should offer specific types, got %r\" % offer)\r\nValueError: The application should offer specific types, got 'text/*'\r\n>>> MIMENilAccept().best_match(['text/*'])\r\nTraceback (most recent call last):\r\n  File \"<stdin>\", line 1, in <module>\r\n  File \"/Users/michael/work/oss/pyramid/env27/lib/python2.7/site-packages/webob/acceptparse.py\", line 230, in best_match\r\n    _check_offer(offer)\r\n  File \"/Users/michael/work/oss/pyramid/env27/lib/python2.7/site-packages/webob/acceptparse.py\", line 320, in _check_offer\r\n    raise ValueError(\"The application should offer specific types, got %r\" % offer)\r\nValueError: The application should offer specific types, got 'text/*'\r\n```\r\n\r\nVersion 1.7.4:\r\n\r\n```\r\n>>> MIMEAccept('text/html').best_match(['text/*'])\r\n'text/*'\r\n>>> MIMENilAccept().best_match(['text/*'])\r\nTraceback (most recent call last):\r\n  File \"<stdin>\", line 1, in <module>\r\n  File \"/Users/michael/work/oss/pyramid/env27/lib/python2.7/site-packages/webob/acceptparse.py\", line 209, in best_match\r\n    _check_offer(offer)\r\n  File \"/Users/michael/work/oss/pyramid/env27/lib/python2.7/site-packages/webob/acceptparse.py\", line 340, in _check_offer\r\n    raise ValueError(\"The application should offer specific types, got %r\" % offer)\r\nValueError: The application should offer specific types, got 'text/*'\r\n```\r\n\r\nVersion 1.8.2:\r\n\r\n```\r\n>>> create_accept_header('text/html').best_match(['text/*'])\r\n'text/*'\r\n>>> create_accept_header(None).best_match(['text/*'])\r\n'text/*'\r\n>>> create_accept_header('invalid/x/y').best_match(['text/*'])\r\n'text/*'\r\n```\r\n\r\nI think the old behavior should be restored and invalid offers should raise."},"comment":{"url":"https://api.github.com/repos/Pylons/webob/issues/comments/417114863","html_url":"https://github.com/Pylons/webob/issues/367#issuecomment-417114863","issue_url":"https://api.github.com/repos/Pylons/webob/issues/367","id":417114863,"node_id":"MDEyOklzc3VlQ29tbWVudDQxNzExNDg2Mw==","user":{"login":"whiteroses","id":291075,"node_id":"MDQ6VXNlcjI5MTA3NQ==","avatar_url":"https://avatars3.githubusercontent.com/u/291075?v=4","gravatar_id":"","url":"https://api.github.com/users/whiteroses","html_url":"https://github.com/whiteroses","followers_url":"https://api.github.com/users/whiteroses/followers","following_url":"https://api.github.com/users/whiteroses/following{/other_user}","gists_url":"https://api.github.com/users/whiteroses/gists{/gist_id}","starred_url":"https://api.github.com/users/whiteroses/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/whiteroses/subscriptions","organizations_url":"https://api.github.com/users/whiteroses/orgs","repos_url":"https://api.github.com/users/whiteroses/repos","events_url":"https://api.github.com/users/whiteroses/events{/privacy}","received_events_url":"https://api.github.com/users/whiteroses/received_events","type":"User","site_admin":false},"created_at":"2018-08-29T21:39:02Z","updated_at":"2018-08-29T21:39:02Z","author_association":"CONTRIBUTOR","body":"The only change in behaviour between 1.7 and 1.8 was that calls to `_check_offer` in `NilAccept.__contains__` and `NilAccept.best_match` (what are now `_AcceptInvalidOrNoHeader.__contains__` and `_AcceptInvalidOrNoHeader.best_match`) were removed to match the behaviour of `MIMEAccept.__contains__` and `MIMEAccept.best_match` and [the docstring of `MIMEAccept._match`](https://github.com/Pylons/webob/blob/1.7-branch/webob/acceptparse.py#L282), which allowed media types/ranges with `*`s in them. No other changes were made, as the plan was to deprecate `__contains__` and `best_match`, so the only difference in behaviour between 1.7 and 1.8 is that `ValueError` is no longer raised when an offer with a `*` in it meets a header that would previously have been a `MIMENilAccept`.\r\n\r\nI see now that the docstrings for `__contains__` and `best_match` were never even updated after the change to accept ranges in 1.6 ([issue](https://github.com/Pylons/webob/issues/155) and [PR](https://github.com/ltvolks/webob/commit/8f49817f64833fc9171e506ad648314dc7d9a246)), with both docstrings referring only to types &mdash; so essentially it was an under-the-hood change in WebOb for Pyramid, and WebOb users would not have even known about the change to accept ranges without seeing mention of it in the changelog or the issue and PR. I don't know why I didn't see that the docstrings for `__contains__` and `best_match` hadn't been updated, or if I did why it didn't give me pause. Given all this, I would support restoring `_check_offer` calls to `best_match` and `__contains__`, especially as we can see from your example from 1.7.4 that `MIMEAccept.best_match` wasn't even working properly with ranges:\r\n\r\n    >>> MIMEAccept('text/html').best_match(['text/*'])\r\n    'text/*'\r\n\r\nThis is returning the offer &mdash; a range &mdash; as the best match.\r\n\r\nAdding `_check_offer` calls should be relatively simple, but should we remove the code that was added to handle ranges too?\r\n"}}
{"actor":{"display_login":"whiteroses"}}