Skip to content
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

Add validation for header name #6154

Merged
merged 1 commit into from Jun 8, 2022

Conversation

nateprewitt
Copy link
Member

@nateprewitt nateprewitt commented Jun 8, 2022

Following up on #6083, this refactors our header validation function from 2.11.0 to consider the header name. We'd originally avoided adding header name validation because we wanted to limit the change scope to header splitting with new lines. Since then the standard library has made similar changes to ours and now raises a ValueError in http.client. This gives us inconsistent errors depending on which portion of the header you provide a bad value.

>>> requests.get("https://httpbin.org/get", headers={":bad": "header"})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/nateprewitt/Work/OpenSource/requests/requests/api.py", line 73, in get
    return request("get", url, params=params, **kwargs)
  File "/Users/nateprewitt/Work/OpenSource/requests/requests/api.py", line 59, in request
    return session.request(method=method, url=url, **kwargs)
  File "/Users/nateprewitt/Work/OpenSource/requests/requests/sessions.py", line 587, in request
    resp = self.send(prep, **send_kwargs)
  File "/Users/nateprewitt/Work/OpenSource/requests/requests/sessions.py", line 701, in send
    r = adapter.send(request, **kwargs)
  File "/Users/nateprewitt/Work/OpenSource/requests/requests/adapters.py", line 489, in send
    resp = conn.urlopen(
  File "/Users/nateprewitt/Work/OpenSource/urllib3/src/urllib3/connectionpool.py", line 727, in urlopen
    httplib_response = self._make_request(
  File "/Users/nateprewitt/Work/OpenSource/urllib3/src/urllib3/connectionpool.py", line 433, in _make_request
    conn.request(method, url, **httplib_request_kw)
  File "/Users/nateprewitt/Work/OpenSource/urllib3/src/urllib3/connection.py", line 309, in request
    super().request(method, url, body=body, headers=headers)
  File "/Users/nateprewitt/.pyenv/versions/3.10.4/lib/python3.10/http/client.py", line 1282, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/Users/nateprewitt/.pyenv/versions/3.10.4/lib/python3.10/http/client.py", line 1323, in _send_request
    self.putheader(hdr, value)
  File "/Users/nateprewitt/Work/OpenSource/urllib3/src/urllib3/connection.py", line 274, in putheader
    super().putheader(header, *values)
  File "/Users/nateprewitt/.pyenv/versions/3.10.4/lib/python3.10/http/client.py", line 1250, in putheader
    raise ValueError('Invalid header name %r' % (header,))
ValueError: Invalid header name b':bad'

This PR ports the validation scheme from http.client for header names and ensures we raise an InvalidHeader error consistently in all supported versions.

This was referenced Jun 8, 2022
@nateprewitt nateprewitt changed the title Add valdation for header name Add validation for header name Jun 8, 2022
Copy link
Member

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nateprewitt nateprewitt merged commit e36f345 into psf:main Jun 8, 2022
@nateprewitt nateprewitt deleted the improved_header_validation branch June 8, 2022 18:04
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants