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

gh-87497: Explicit that headers sent in camel-case #24661

Merged
merged 3 commits into from
Apr 14, 2022
Merged

gh-87497: Explicit that headers sent in camel-case #24661

merged 3 commits into from
Apr 14, 2022

Conversation

axel3rd
Copy link
Contributor

@axel3rd axel3rd commented Feb 26, 2021

Refers: #87497


Currently, the Request sends header keys in Camel case.

This is a choice strategy (no problem with that) and even if headers should be considered insensitively RFC 7230 - 3.2. Header Fields, this is not always the case.

The fact that header keys are "rewritten" (in Camel case) could occurs complexity in troubleshooting (sample).

Having a note/mention about that in documentation would be nice.


Proof

Consider a server which write headers as it:

#!/usr/bin/env python3

import http.server as SimpleHTTPServer
import socketserver as SocketServer
import logging

PORT = 8000

class GetHandler(
        SimpleHTTPServer.SimpleHTTPRequestHandler
        ):

    def do_GET(self):
        logging.error(self.headers)
        SimpleHTTPServer.SimpleHTTPRequestHandler.do_GET(self)


Handler = GetHandler
httpd = SocketServer.TCPServer(("", PORT), Handler)

httpd.serve_forever()

Check that headers are written as it: curl --header "x-soMe-hEad: test" http://localhost:8000

Consider a simple usage request call with a custom header X-SoMe-hEader:

#!/usr/bin/env python3

import urllib.request

url = "http://localhost:8000"
hdr = { 'X-SoMe-hEader' : 'foobar' }

req = urllib.request.Request(url, headers=hdr)
response = urllib.request.urlopen(req)
response.read()

Result is: X-Some-Header: foobar

https://bugs.python.org/issue43331

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@axel3rd

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@axel3rd
Copy link
Contributor Author

axel3rd commented Feb 26, 2021

Our records indicate the following people have not signed the CLA:

I have signed it 26 févr. à 19:47 😢 :

image

@github-actions
Copy link

github-actions bot commented Apr 1, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Apr 1, 2021
@axel3rd
Copy link
Contributor Author

axel3rd commented Apr 1, 2021

If any modification could be required on this PR, feel free to say it to me.

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Apr 2, 2021
@github-actions
Copy link

github-actions bot commented May 3, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label May 3, 2021
@axel3rd
Copy link
Contributor Author

axel3rd commented Jul 28, 2021

If any modification could be required on this PR, feel free to say it to me.

@MaxwellDupre
Copy link
Contributor

Thats not what I get after running
response.read()

b'\n\n\n<meta http-equiv="C...
Then a directory listing of where I am running the server script. Same running curl also.
Can you show exactly from star to finish, please?

@axel3rd
Copy link
Contributor Author

axel3rd commented Apr 5, 2022

@MaxwellDupre The header result is to check in "server side", written by logging.error(self.headers)

Hope this screen can help:

image

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Are there unit tests for this behavior? If not, we should add them if we choose to document it.

Doc/library/urllib.request.rst Outdated Show resolved Hide resolved
@axel3rd axel3rd marked this pull request as draft April 6, 2022 09:00
@axel3rd
Copy link
Contributor Author

axel3rd commented Apr 6, 2022

Doc updated, onboarding unit test process in progress.

@axel3rd axel3rd marked this pull request as ready for review April 6, 2022 10:42
@axel3rd
Copy link
Contributor Author

axel3rd commented Apr 6, 2022

@JelleZijlstra Doc fixed & UT added, let me know if it is OK for you.

.gitignore Outdated Show resolved Hide resolved
@JelleZijlstra JelleZijlstra changed the title bpo-43331: Explicit that headers sent in camel-case gh-87497: Explicit that headers sent in camel-case Apr 14, 2022
@JelleZijlstra JelleZijlstra merged commit 325d6f5 into python:main Apr 14, 2022
@miss-islington
Copy link
Contributor

Thanks @axel3rd for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 14, 2022
…ase (pythonGH-24661)

Co-authored-by: Jelle Zijlstra <[email protected]>
(cherry picked from commit 325d6f5)

Co-authored-by: Alix Lourme <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Apr 14, 2022
@bedevere-bot
Copy link

GH-91517 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Apr 14, 2022
@bedevere-bot
Copy link

GH-91518 is a backport of this pull request to the 3.9 branch.

miss-islington added a commit that referenced this pull request Apr 14, 2022
…H-24661)

Co-authored-by: Jelle Zijlstra <[email protected]>
(cherry picked from commit 325d6f5)

Co-authored-by: Alix Lourme <[email protected]>
@axel3rd axel3rd deleted the patch-1 branch April 16, 2022 21:33
JelleZijlstra pushed a commit that referenced this pull request Apr 20, 2022
…H-24661) (#91517)

Co-authored-by: Jelle Zijlstra <[email protected]>
(cherry picked from commit 325d6f5)

Co-authored-by: Alix Lourme <[email protected]>
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
…ase (pythonGH-24661)

Co-authored-by: Jelle Zijlstra <[email protected]>
(cherry picked from commit 325d6f5)

Co-authored-by: Alix Lourme <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants