New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request.authorization raises ValueError for unusual or malformed header values #231

Closed
lrowe opened this Issue Feb 25, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@lrowe
Copy link
Contributor

lrowe commented Feb 25, 2016

I'm seeing errors such as the following being logged in production:

  File "xxx.py", line 91, in xxx
    if request.authorization is not None:
  File "eggs/WebOb-1.4.1-py3.4.egg/webob/descriptors.py", line 164, in fget
    return parse(hget(r))
  File "eggs/WebOb-1.4.1-py3.4.egg/webob/descriptors.py", line 321, in parse_auth
    authtype, params = val.split(' ', 1)
ValueError: need more than 1 value to unpack

I believe this is because the Authorization header is malformed (does not contain a space or is blank.) Raising a ValueError here means that application code must protect each access to Request.authorization with a try/except block. WebOb should probably handle such cases more elegantly.

lrowe added a commit to lrowe/webob that referenced this issue Feb 25, 2016

Changelog entry
Request.authorization would raise ValueError for unusual or malformed header
values. See Pylons#231
@mmerickel

This comment has been minimized.

Copy link
Member

mmerickel commented Feb 25, 2016

In my own code I do have this try/except block. I actually didn't mind handling it myself to respond with a different error for a malformed header versus invalid credentials but I agree it can be non-obvious.

@bertjwregeer

This comment has been minimized.

Copy link
Member

bertjwregeer commented Feb 25, 2016

I don't think WebOb should silently return something that may or may not look like a valid authorization header.

Instead of raising a ValueError it should probably raise an exception that is not as generic and can be traced back to being a malformed header.

If user code specifically requests a header, especially one that is related to security, WebOb shouldn't silently attempt to acquiesce the malformed header.

@lrowe

This comment has been minimized.

Copy link
Contributor

lrowe commented Feb 25, 2016

The request that triggered my logged error had a User-Agent of "Microsoft Office Word 2014". Googling shows that Microsoft Office DAV may send Authorization: Bearer:

I think there's a good argument for saying set_credentials(('AuthType', '')) should return 'AuthType ' (with a space) instead of 'AuthType' as my PR currently does.

But when accessing Request.authorization, I don't think it makes much sense distinguish between 'AuthType' without a space and 'AuthType ' with a space by raising in the first case. Auth code will need to check the credential params anyway, and will presumably reject it if blank.

@lrowe

This comment has been minimized.

Copy link
Contributor

lrowe commented Feb 25, 2016

I've changed my PR so that set_credentials(('AuthType', '')) now returns 'AuthType ' which is spec compliant.

@bertjwregeer

This comment has been minimized.

Copy link
Member

bertjwregeer commented Jun 27, 2016

Where is set_credentials located? I can't find it in the WebOb source code, so I am not sure what those comments are related to.

@jalan

This comment has been minimized.

Copy link

jalan commented Apr 11, 2017

This issue affects pyramid_debugtoolbar. The "Request Vars" panel accesses request.authorization and does not handle ValueError:

2017-04-11 15:03:57,646 ERROR [waitress:341][waitress] Exception when serving /
Traceback (most recent call last):
  File "[...]/waitress-1.0.2-py3.5.egg/waitress/channel.py", line 338, in service
    task.service()
  File "[...]/waitress-1.0.2-py3.5.egg/waitress/task.py", line 169, in service
    self.execute()
  File "[...]/waitress-1.0.2-py3.5.egg/waitress/task.py", line 399, in execute
    app_iter = self.channel.server.application(env, start_response)
  File "[...]/pyramid/router.py", line 233, in __call__
    response = self.invoke_subrequest(request, use_tweens=True)
  File "[...]/pyramid/router.py", line 208, in invoke_subrequest
    response = handle_request(request)
  File "[...]/pyramid_debugtoolbar-3.0.5-py3.5.egg/pyramid_debugtoolbar/toolbar.py", line 272, in toolbar_tween
    toolbar.process_response(request, response)
  File "[...]/pyramid_debugtoolbar-3.0.5-py3.5.egg/pyramid_debugtoolbar/toolbar.py", line 87, in process_response
    panel.process_response(response)
  File "[...]/pyramid_debugtoolbar-3.0.5-py3.5.egg/pyramid_debugtoolbar/panels/request_vars.py", line 134, in process_response
    extracted_attributes = extract_request_attributes(self.request)
  File "[...]/pyramid_debugtoolbar-3.0.5-py3.5.egg/pyramid_debugtoolbar/panels/request_vars.py", line 60, in extract_request_attributes
    if not hasattr(request, attr_):
  File "[...]/webob/descriptors.py", line 164, in fget
    return parse(hget(r))
  File "[...]/webob/descriptors.py", line 321, in parse_auth
    authtype, params = val.split(' ', 1)
ValueError: not enough values to unpack (expected 2, got 1)

I'd say that's a good indication that raising ValueError here is unexpected. I can confirm that the change in #232 fixes the issue for the debug toolbar.

@bertjwregeer

This comment has been minimized.

Copy link
Member

bertjwregeer commented May 22, 2017

Merged the change into WebOb in #318

clrpackages pushed a commit to clearlinux-pkgs/WebOb that referenced this issue Jul 23, 2018

[update to 1.8.2] Prevent leaking if file exists outside of static path
Abhishek Kumar Singh (1):
      Fix Typo.

Anthony Sottile (1):
      Allow unnamed fields in form content

Bert JW Regeer (119):
      Grab whatsnew from 1.7-branch
      Back to dev for WebOb
      Add appveyor testing
      Change appveyor build numbers
      Python 3.5 on Windows doesn't have x-python mimetype
      Make sure to read the entire body
      Test on Python x86_64 on Windows too
      Update documentation
      Update HISTORY.txt
      Update whatsnew-1.7.txt to match 1.7-branch
      We test on both Py3 and Py2 and combine coverage
      Add PY2 to compat
      Collapse encget into single func for Py2/Py3
      Special case PY2: request.py
      Special case PY2: descriptors.py
      Special case PY2: response.py
      Special case PY2: cookies.py
      Special case PY2: client.py
      Special case PY2: multidict.py
      cmp() was removed in Py3
      Removed untrue statements, it's not recommended...
      Remove all Content-Type params when setting Content-Type
      charset is reset, update test
      Remove unused warning
      Update documentation for Content-Type
      Update docs
      Merge pull request #300 from julienmeyer/bug/issue-299
      Merge pull request #301 from Pylons/feature/remove_ctype_params
      Merge pull request #303 from stevepiercy/master
      Let's not rest on hopes, cgi.FieldStorage needs patching on Py3
      Add tests that showcase #305
      If app_iter is provided set it in Response
      Add CHANGES.txt
      Always enable Python 3.6 on Travis
      Officially support 3.6
      Merge pull request #306 from Pylons/bugfix/Response_app_iter
      Merge pull request #309 from asottile/allow_none_field_storage
      Update CHANGES.txt
      Forward port whatsnew 1.7
      Add RELEASING.rst for copy and paste ease of use when releasing
      Merge pull request #313 from inytar/prevent_leak
      Merge remote-tracking branch 'origin/master' into pr/255
      Add documentation about experimental features
      Document make_cookie better
      Add CHANGES.txt
      Update grammar
      Merge pull request #316 from Pylons/pr/255
      Merge remote-tracking branch 'origin/master' into pr/203
      Add CHANGES note
      Merge pull request #317 from Pylons/pr/203
      Merge remote-tracking branch 'origin/master' into pr/232
      Return a namedtuple
      Merge pull request #318 from Pylons/pr/232
      Don't remove self.content_md5 in Response when setting app_iter
      Add changes for #86
      Remove unnecessary files
      Move fixtures into a proper fixture
      Add some settings to setup.cfg
      Add coveragerc file
      Move files around, make testing better
      Remove PY2/PY3 and use pytest marking
      Remove multidict tests from misc
      Remove PY3 stuff and mark certain tests as py2only
      Use pytest.parametrize instead of looping
      Remove nose based test file, and move tests to requests.py
      Allow running tests in parallel
      Merge pull request #319 from Pylons/cleanup/testing
      Merge pull request #321 from JelleZijlstra/patch-1
      Merge pull request #324 from whiteroses/pin-pytest
      Merge pull request #326 from whiteroses/change-accept-encoding-acceptclass
      Merge pull request #329 from stevepiercy/master
      Merge pull request #328 from whiteroses/expose-parsed
      Merge pull request #330 from whiteroses/fix-acceptparse-docs
      Add some IPv6 HTTP_HOST tests
      Use rsplit instead of split
      Add CHANGES for #332
      Merge pull request #332 from Pylons/bugfix/ipv6_host_support
      Merge pull request #335 from whiteroses/accept-language
      Merge branch 'master' into stevepiercy-patch-1
      Merge pull request #333 from Pylons/stevepiercy-patch-1
      Merge pull request #338 from whiteroses/update-fix-accept-headers
      Update CHANGES.txt to warn about Accept handling differences
      Code in references.txt is now Python 3
      Add information about MultiDict to the multidict API docs
      Cleanup index.txt documentation
      Allow linking to reference
      Add Py3 only notice
      Explicitly import all the things, instead of doing import *
      Fix bad indentation
      Link some more attributes
      Support wsgi.input_terminated
      Remove deprecations that were raising
      PEP8
      Remove tests for deprecated functions
      Add MIMEAccept shim that warns about deprecation
      Verify warning is raised when calling MIMEAccept's init
      Change Accept header documentation slightly
      Remove Python 3.3 support
      Update copyright on documentation
      Add reference to section
      Add Whats New in WebOb 1.8 document
      Prep for 1.8.0rc1
      I can't spell
      Merge pull request #349 from stevepiercy/fix-342-add-trailing-whitespaces
      Merge pull request #350 from stevepiercy/remove-html-use-smartypants
      Merge pull request #353 from hongyuan1306/master
      Merge pull request #352 from Cykooz/master
      Update CHANGES.txt text for #352
      Update What's New for 1.8
      Ignore pytest cache
      Use string.Template.safe_substitute for HTTP exc body templates
      Merge pull request #354 from Pylons/switch-to-safe-substitute
      Prep for 1.8.0
      MIMEAccept mimics old behaviour better
      Add changes for backport of #356
      Prep for version 1.8.1
      Remove 'key' keyword argument from set_cookie
      Merge branch 'bugfix/nonbytes-samesite'
      Add CHANGES.txt and bump to 1.8.2

Donald Stufft (1):
      Support the SameSite field in Cookies

Hong Yuan (1):
      Fix suggested secret length in doc

Ira Lun (159):
      Pin pytest to >= 3.1.0.
      Change AcceptClass of request.accept_encoding to AcceptEncoding.
      Add some basic docstrings for Sphinx autodoc.
      Include all relevant classes from acceptparse.py in docs.
      Add docstrings for ``.parse()`` in the classes where it is overridden.
      Document ``.parse()`` with ``automethod``.
      Remove comment.
      Expose ``Accept.parsed``.
      Document acceptparse.py using :members: and :inherited-members:.
      Add AcceptLanguageValidHeader class with .__init__ and .parse().
      Document instance attributes of AcceptLanguageValidHeader.
      Add AcceptLanguageValidHeader.basic_filtering().
      Fix comment.
      Improve docstring.
      Make comment more precise.
      Replace loop with any() and a generator.
      Add AcceptLanguageValidHeader.lookup().
      Add AcceptLanguage._old_match.
      Add AcceptLanguageValidHeader.__iter__.
      Add AcceptLanguageValidHeader.__contains__.
      Improve comment.
      Fix typo.
      Add AcceptLanguageValidHeader.best_match.
      Minor rewording in docstring.
      Add AcceptLanguageValidHeader.quality().
      Add colon.
      Make heading bold.
      Improve docstring.
      Add deprecation warnings.
      Add _AcceptLanguageInvalidOrNoHeader class.
      Add _AcceptLanguageInvalidOrNoHeader.__contains__ and test.
      Fix docstring.
      Add _AcceptLanguageInvalidOrNoHeader.__iter__.
      Add _AcceptLanguageInvalidOrNoHeader.__nonzero__.
      Add AcceptLanguageValidHeader.__nonzero__/.__bool__ and test.
      Add _AcceptLanguageInvalidOrNoHeader.basic_filtering.
      Add missing space.
      Add _AcceptLanguageInvalidOrNoHeader.best_match.
      Remove call to _check_offer().
      Add _AcceptLanguageInvalidOrNoHeader.quality.
      Add _AcceptLanguageInvalidOrNoHeader.lookup and tests.
      Add AcceptLanguageNoHeader class, __init__, test and documentation.
      Add AcceptLanguageInvalidHeader class, __init__, test and doc.
      Add __repr__ and tests for the three AcceptLanguage classes.
      Add __str__ and tests for the three AcceptLanguage classes.
      Add create_accept_language_header().
      Turn re-usable code into function.
      Add __add__ and __radd__ for AcceptLanguageValidHeader, with tests.
      Add __add__ and __radd__ for AcceptLanguageNoHeader, with tests.
      Add __add__ and __radd__ for AcceptLanguageInvalidHeader, with tests.
      Add blank line.
      Add docstrings for NoHeader's and InvalidHeader's __str__s.
      Update documentation.
      Test NoHeader and InvalidHeader rather than InvalidOrNoHeader.
      Add accept_language_property and tests.
      Use accept_language_property in request.
      Update AcceptLanguage class, and stop inheriting from Accept.
      Remove HeaderClass attribute, which we no longer need.
      Make test methods' names and order consistent.
      Assert that all three subclasses are instances of AcceptLanguage.
      Add a blank line after docstrings documenting classes.
      Remove :inherited-members: option.
      Make docstring a raw string.
      Add deprecation warnings for __iter__.
      Fix wording.
      Add tests for AcceptLanguageValidHeader.best_match.
      Add tests for ...NoHeader.best_match and ...InvalidHeader.best_match.
      Add tests for .quality().
      Remove check for 'q'.
      Add tests for Accept.__iter__ and NilAccept.__iter__.
      Remove :members: option.
      Make .header_value and .parsed read-only properties.
      Set autodoc_member_order.
      Change variable name from ``accept_language`` to ``instance``.
      Make AcceptLanguageInvalidHeader.__repr__ safer.
      Make AcceptLanguageValidHeader.__repr__ safer.
      Make __repr__ consistent between the AcceptLanguage classes.
      Stop displaying header_value in AcceptLanguageValidHeader.__repr__.
      Improve comment.
      Change double quotes to single quotes for consistency.
      Remove comment.
      Remove unnecessary blank lines.
      Improve __repr__ of AcceptLanguage classes.
      Add functions to help with forming the regexes for parsing the headers.
      Remove note on parsing.
      Remove unnecessary ``else:``.
      Change 'generator' to 'generator iterator' to avoid ambiguity.
      Use more helpful variable names.
      Raise ValueError with error message.
      Change 'generator iterator' to 'iterator' in docstring.
      Change test imports to module scope.
      Add spaces.
      Treat invalid header values like ``NoHeader``s when __add__ing.
      Remove unused `default_quality` parameter.
      Fix missing commas and spacing in single-element tuples.
      Correct and improve wording of docstrings.
      Improve wording in docstring.
      Simplify regexes.
      Add underscore to classmethod to make it clear that it's not public API.
      Change PendingDeprecationWarnings to DeprecationWarnings.
      Check and raise instead of using asserts.
      Remove undocumented `modifier` parameter.
      Remove unnecessary blank line.
      Move .parse from AcceptLanguageValidHeader to AcceptLanguage.
      Update to say AcceptLanguageValidHeader._old_match will be deprecated.
      Fix typo.
      Add missing word.
      Fix incorrect test.
      Rewrite AcceptEncoding class and add docs and tests.
      Add AcceptEncodingValidHeader class, docs and tests.
      Add _AcceptEncodingInvalidOrNoHeader class.
      Add AcceptEncodingNoHeader class, docs and tests.
      Add AcceptEncodingInvalidHeader class, docs and tests.
      Add create_accept_encoding_header, docs and tests.
      Add accept_encoding_property and tests.
      Add tchar and token regexes.
      Add _list_0_or_more__compiled_re.
      Fix incorrect class names.
      Use accept_encoding_property for request.accept_encoding.
      Add Accept-Encoding-related imports.
      Remove now-unused NoAccept class.
      Rewrite AcceptCharset class, and add docs and tests.
      Remove obsolete tests.
      Add AcceptCharsetValidHeader class, docs and tests.
      Add _AcceptCharsetInvalidOrNoHeader class.
      Add AcceptCharsetNoHeader class, docs and tests.
      Add AcceptCharsetInvalidHeader class, docs and tests.
      Add create_accept_charset_header, docs and tests.
      Add accept_charset_property, docs and tests.
      Add tests for request.accept_encoding.
      Add request.accept_charset tests.
      Rewrite Accept class, and add docs and tests.
      Add AcceptValidHeader class, docs and tests.
      Add _AcceptInvalidOrNoHeader class.
      Add AcceptNoHeader class, docs and tests.
      Add AcceptInvalidHeader class, docs and tests.
      Add create_accept_header, docs and tests.
      Rewrite accept_property and add tests.
      Add tests for request.accept.
      Remove obsolete tests.
      Update processing of Accept header.
      Remove MIMEAccept, MIMENilAccept and NilAccept.
      Remove the unused _check_offer.
      Update import and isinstance test.
      Fix assert.
      Remove unused imports.
      Remove obsolete regex.
      Update module docstring.
      Fix markup.
      Only retrieve qvalue, as we do not need language_range.
      Correct method calls in examples.
      Improve AcceptLanguageValidHeader.basic_filtering.
      Fix typo.
      List possible types for all header subclasses.
      Place imports in alphabetical order.
      Place imports in alphabetical order.
      Remove hyphen, as the Accept header itself has no hyphen.
      Place methods in alphabetical order.
      Add MIMEAccept for backward compatibility.

Jelle Zijlstra (1):
      Fix BytesWarning in webob.cookies._value_quote

Julien Meyer (3):
      Merge pull request #1 from Pylons/master
      Remove headers length from content part if present and filename not present
      Remove unittest.TestCase  from Test_cgi_FieldStorage_Py3_tests class declaration

Kirill Kuzminykh (2):
      ``request.POST`` now supports DELETE requests with the appropriate Content-Type (#351).
      request.POST now supports any requests with the appropriate Content-Type (#351).

Laurence Rowe (3):
      Avoid raising ValueError in request.authorization for unusual or malformed header values.
      Changelog entry Request.authorization would raise ValueError for unusual or malformed header values. See Pylons/webob#231
      Always include space when serializing an authorization header tuple.

Michael Merickel (1):
      Merge pull request #327 from whiteroses/document-acceptparse-classes

Nick Stenning (2):
      Add some tests for wsgify/wsgify.middleware args
      Correctly preserve args when using wsgify callables

Steve Piercy (12):
      Polish up reST syntax (only in docstrings for code, since comments don't get rendered)
      mo bettah reST syntax
      Merge pull request #302 from stevepiercy/master
      update contributing.md versions - ping @bertjwregeer to verify versions, and to make note to roll in 1.7-branch on its release
      Merge pull request #312 from stevepiercy/master
      ignore docs/_build
      Update links to https, "stable", and canonical URL
      Use https
      update link to save redirect
      Update and fix links per `make linkcheck` (#334)
      add trailing whitespaces to deprecation warning for MIMEAccept
      html_use_smartypants is no longer needed because RTD now uses Sphinx 1.6.5

inytar (1):
      Prevent leaking if file exists outside of static path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment