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

Multiple request.headers handling #8367

Closed
wpt-issue-mover opened this issue Nov 20, 2017 · 12 comments · Fixed by #10562
Closed

Multiple request.headers handling #8367

wpt-issue-mover opened this issue Nov 20, 2017 · 12 comments · Fixed by #10562

Comments

@wpt-issue-mover
Copy link

Originally posted as w3c/wptserve#84 by @BigBlueHat on 14 Jul 2016, 21:03 UTC:

There seems to be code to handle multiple request headers, but when I attempt sending them into a wptserve.handlers.handler I only get the last one sent in...despite request.headers being a list.

I'm still digging through the code, but if someone has a hunch what might be up, I'm eager for pointers. 😄

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @halindrome on 15 Jul 2016, 15:22 UTC:

Presumably you are doing something with a local handler? Is there a code example you can point us at? And are you using the API as defined in https://wptserve.readthedocs.io/en/latest/handlers.html#python-handlers ?

Oh, and are you doing this by defining an additional "route" for wptserve?

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @BigBlueHat on 15 Jul 2016, 15:27 UTC:

This line is returning a single element list--even if two Prefer headers come in (I've also tested it with Link, etc...with the same result...just one item in the list):
https://github.com/Spec-Ops/web-platform-tests/pull/3/files#diff-4de40885b179e09a548b89d4134ab47eR91

I am using this within a Python Handler.

And the route is defined here--and it's super duper basic:
https://github.com/Spec-Ops/web-platform-tests/pull/3/files#diff-4de40885b179e09a548b89d4134ab47eR277

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @halindrome on 15 Jul 2016, 16:38 UTC:

Huh. Not sure I understand that route handler completely... are routes resolved relative to the base or the python file that gets executed when they are created? Also are you setting up your own endpoint on the wptserver, or just adding some routes and filehandlers to the existing server that is started when ./serve is executed in the top level directory?

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @BigBlueHat on 15 Jul 2016, 17:48 UTC:

@halindrome so...I missed the ./serve step/thing. This is currently setup to just be run via python index.py in that directory. 😛 Likely, it needs an upgrade.

I'll look into serve also, but that's not related to this issue (afaict). I'll keep digging.

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @BigBlueHat on 15 Jul 2016, 18:16 UTC:

Here's a Gist with a simple wptserve-based server that echo's the headers its given:
https://gist.github.com/BigBlueHat/0161b4cdad76a439b43d6c82c594191f

Thoughts?

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @BigBlueHat on 20 Jul 2016, 21:40 UTC:

So...it looks as if Python's BaseHTTPRequestHandler uses ye olde mimetools.Message (which in turn extends rfc822.Message) and that has it's own system for getting at multiple headers:
https://docs.python.org/2/library/rfc822.html#rfc822.Message.getallmatchingheaders

When that object is passed to wptserve.request.RequestHeaders it's loosing the inbound multiple headers and only keeping the last one...
https://github.com/w3c/wptserve/blob/master/wptserve/request.py#L348-L356

It is possible to work around this by accessing the _raw_headers and using getallmatchingheaders from the underlying object...but that seems...ungood.

It looks like someone meant this to work, however, judging by the get_list() method on RequestHeaders:
https://github.com/w3c/wptserve/blob/master/wptserve/request.py#L383-L392

I'll keep digging into this...I feel like I must be missing something super obvious...

help wanted. 😁

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @gsnedders on 21 Jul 2016, 01:03 UTC:

I don't think you are missing anything obvious. :)

It may well be the case that forking BaseHTTPRequestHandler is simply the best thing to do (we do have somewhat unusual requirements, after all, and are likely to find more things that we can't do with the stdlib class in future!), along with the related machinery it relies on. I agree _raw_headers is probably a bad move.

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @halindrome on 21 Jul 2016, 14:00 UTC:

Yeah.... I will work with @BigBlueHat on this. It is blocking testing of Annotation Protocol, which just entered CR phase.

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @Ms2ger on 23 Jul 2016, 08:19 UTC:

See also #2612

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptserve#84 (comment) by @BigBlueHat on 29 Jul 2016, 15:45 UTC:

Here's the fix: w3c/wptserve#87

@foolip
Copy link
Member

foolip commented Nov 21, 2017

What's the status of this, @BigBlueHat? w3c/wptserve#87 was never finished, is it still relevant?

@BigBlueHat
Copy link
Member

@foolip I've reapplied the commit to this new consolidated repo. Thanks!

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

Successfully merging a pull request may close this issue.

4 participants