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

Fails with wrong headers #1370

Closed
bb-migration opened this Issue Jun 23, 2015 · 8 comments

Comments

Projects
None yet
5 participants
@bb-migration

bb-migration commented Jun 23, 2015

Originally reported by: Daniel Frischhut (Bitbucket: daniel_frischhut, GitHub: Unknown)


We got a request with the following header-fields:

  'CONNECTION': 'close'
  'CACHE-CONTROL': 'no-cache'
  'ACCEPT': 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8'
  'ACCEPT-CHARSET': 'ISO-8859-1,utf-8;q=0.7,*;q=0.7)'
  'USER-AGENT': 'Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.95 Safari/537.36'
  'ACCEPT-LANGUAGE': 'en-us'
  'ACCEPT-ENCODING': 'gzip,deflate'

and cherrypy faild with follwing Traceback:

Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/cherrypy/_cprequest.py", line 670, in respond
    response.body = self.handler()
  File "/usr/local/lib/python2.7/dist-packages/cherrypy/lib/encoding.py", line 260, in __call__
    ct.params[\'charset\'] = self.find_acceptable_charset()
  File "/usr/local/lib/python2.7/dist-packages/cherrypy/lib/encoding.py", line 146, in find_acceptable_charset
    encs = request.headers.elements(\'Accept-Charset\')
  File "/usr/local/lib/python2.7/dist-packages/cherrypy/lib/httputil.py", line 456, in elements
    return header_elements(key, value)
  File "/usr/local/lib/python2.7/dist-packages/cherrypy/lib/httputil.py", line 237, in header_elements
    return list(reversed(sorted(result)))
  File "/usr/local/lib/python2.7/dist-packages/cherrypy/lib/httputil.py", line 217, in __lt__
    if self.qvalue == other.qvalue:
  File "/usr/local/lib/python2.7/dist-packages/cherrypy/lib/httputil.py", line 207, in qvalue
    return float(val)
ValueError: invalid literal for float(): 0.7)

i edited the httputil.py in line 207 and surrounded the return float(val) with a try - except and returned the default value.


@bb-migration

This comment has been minimized.

bb-migration commented Jun 23, 2015

Original comment by duong nt (Bitbucket: bugmenot4, GitHub: bugmenot4):


"q=0.7)" is invalid, you should fix the application

@bb-migration

This comment has been minimized.

bb-migration commented Jun 23, 2015

Original comment by Daniel Frischhut (Bitbucket: daniel_frischhut, GitHub: Unknown):


I know that this is invalid, I got this header in a productive environment, so I have no influence on the browsers out there. The application returns a 500 error right now, this may is wanted in some cases - in other cases the application should continue and just ignore the wrong header element.

@bb-migration

This comment has been minimized.

bb-migration commented Jul 19, 2015

Original comment by Sylvain Hellegouarch (Bitbucket: Lawouach, GitHub: Lawouach):


Hi Daniel,

I don't think it's a bug per-se and the server reacts accordingly by sending out a 500 error. Yet, from a user point of view, I can appreciate you'd want the encoding header to be more forgiving.

This is more of a feature than a bug however.

@webknjaz

This comment has been minimized.

Member

webknjaz commented Sep 23, 2016

@jaraco I don't think there should be any forgiveness to malformed input. Do you?

@jaraco

This comment has been minimized.

Member

jaraco commented Oct 4, 2016

Malformed input should return a 40x error and not a 500 error. I'm less sure about whether the error should be silently suppressed or raise a 40x.

@skieffer

This comment has been minimized.

skieffer commented Jan 18, 2018

This morning my website experienced the same issue, and due to the exact same malformed header:

ACCEPT-CHARSET: ISO-8859-1,utf-8;q=0.7,*;q=0.7)

What is this client out there in the wild that adds this erroneous close-parenthesis?

For what it's worth, I came here to make the same request, i.e. that we tolerate the header. I think when the user sees a 500 error it makes me look bad, and I imagine they are probably totally unaware that they are sending this malformed header. They will never fix the problem, and my website will continue to appear broken to them.

@seils

This comment has been minimized.

Contributor

seils commented Apr 20, 2018

@webknjaz, @jaraco, I'll take this one.

I'm considering a couple of approaches:

  • Catch the ValueError exception and return float(0) instead, essentially making the charset with the malformed weight value unusable.
  • Catch the ValueError and remove the charset with the malformed weight value from the list.

Any preference?

@webknjaz

This comment has been minimized.

Member

webknjaz commented Apr 20, 2018

@seils @skieffer Malicious requests have to result in client errors http 400 (40x).
We should not tolerate this.
It this comes from client side, it means that they (1) don't use a normal browser, which normally implies that (2) it's a script/bot/exploit scanner.
So the only correct thing to do (RFC compliant) is to return 400.

@seils seils referenced this issue Apr 20, 2018

Closed

Fix 1370 #1707

webknjaz added a commit that referenced this issue May 28, 2018

Fixes #1370
Return  for requests with malformed Accept-Charset quality values.

@webknjaz webknjaz closed this in 38f199c May 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment