-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Header cleanup #3404
Header cleanup #3404
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3404 +/- ##
==========================================
- Coverage 98.01% 97.94% -0.08%
==========================================
Files 44 44
Lines 8500 8500
Branches 1377 1377
==========================================
- Hits 8331 8325 -6
- Misses 70 74 +4
- Partials 99 101 +2
Continue to review full report at Codecov.
|
tools/gen.py
Outdated
@@ -16,11 +15,17 @@ def factory(): | |||
|
|||
|
|||
TERMINAL = object() | |||
HIDDEN_FROM_FIND_HEADER = ( | |||
'Proxy-Authorization', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please elaborate why do you want to skip the header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You dropped it manually in #3095. I just updated the build script to skip it there as well.
Here is the diff after running ./tools/gen.py
on the master
branch:
diff --git a/aiohttp/_find_header.c b/aiohttp/_find_header.c
index 3b656f9..71b8de4 100644
--- a/aiohttp/_find_header.c
+++ b/aiohttp/_find_header.c
@@ -6629,11 +6629,6 @@ PROX:
PROXY:
NEXT_CHAR();
switch (ch) {
- case '_':
- if (last) {
- return -1;
- }
- goto PROXY_;
case '-':
if (last) {
return -1;
@@ -6724,6 +6719,16 @@ PROXY_AUTH:
return -1;
}
goto PROXY_AUTHE;
+ case 'O':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHO;
+ case 'o':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHO;
default:
return -1;
}
@@ -6847,6 +6852,142 @@ PROXY_AUTHENTICAT:
return -1;
}
+PROXY_AUTHO:
+ NEXT_CHAR();
+ switch (ch) {
+ case 'R':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHOR;
+ case 'r':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHOR;
+ default:
+ return -1;
+ }
+
+PROXY_AUTHOR:
+ NEXT_CHAR();
+ switch (ch) {
+ case 'I':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORI;
+ case 'i':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORI;
+ default:
+ return -1;
+ }
+
+PROXY_AUTHORI:
+ NEXT_CHAR();
+ switch (ch) {
+ case 'Z':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZ;
+ case 'z':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZ;
+ default:
+ return -1;
+ }
+
+PROXY_AUTHORIZ:
+ NEXT_CHAR();
+ switch (ch) {
+ case 'A':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZA;
+ case 'a':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZA;
+ default:
+ return -1;
+ }
+
+PROXY_AUTHORIZA:
+ NEXT_CHAR();
+ switch (ch) {
+ case 'T':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZAT;
+ case 't':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZAT;
+ default:
+ return -1;
+ }
+
+PROXY_AUTHORIZAT:
+ NEXT_CHAR();
+ switch (ch) {
+ case 'I':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZATI;
+ case 'i':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZATI;
+ default:
+ return -1;
+ }
+
+PROXY_AUTHORIZATI:
+ NEXT_CHAR();
+ switch (ch) {
+ case 'O':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZATIO;
+ case 'o':
+ if (last) {
+ return -1;
+ }
+ goto PROXY_AUTHORIZATIO;
+ default:
+ return -1;
+ }
+
+PROXY_AUTHORIZATIO:
+ NEXT_CHAR();
+ switch (ch) {
+ case 'N':
+ if (last) {
+ return 51;
+ }
+ goto PROXY_AUTHORIZATION;
+ case 'n':
+ if (last) {
+ return 51;
+ }
+ goto PROXY_AUTHORIZATION;
+ default:
+ return -1;
+ }
+
R:
NEXT_CHAR();
switch (ch) {
@@ -9827,6 +9968,7 @@ MAX_FORWARDS:
ORIGIN:
PRAGMA:
PROXY_AUTHENTICATE:
+PROXY_AUTHORIZATION:
RANGE:
REFERER:
RETRY_AFTER:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks.
I don't recall details but IMHO there is no reason for Proxy-Authorization
header skipping.
Please revert skipping logic in gen.py
and regenerate _find_headers.c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Thanks! |
What do these changes do?
Fix import of cpython headers.
Drop invalid header start
PROXY_
.Are there changes in behavior for the user?
No, this is a low level optimization.
Related issue number
The PR that introduced the header optimization: #3095
Checklist
CONTRIBUTORS.txt
CHANGES
folder<issue_id>.<type>
for example (588.bugfix)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.