Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Multiple request.headers handling #84

Closed
BigBlueHat opened this issue Jul 14, 2016 · 11 comments
Closed

Multiple request.headers handling #84

BigBlueHat opened this issue Jul 14, 2016 · 11 comments

Comments

@BigBlueHat
Copy link
Member

BigBlueHat commented Jul 14, 2016

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. 😄

@halindrome
Copy link

halindrome commented Jul 15, 2016

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?

@BigBlueHat
Copy link
Member Author

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

@halindrome
Copy link

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?

@BigBlueHat
Copy link
Member Author

@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.

@BigBlueHat
Copy link
Member Author

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

Thoughts?

@BigBlueHat
Copy link
Member Author

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. 😁

@gsnedders
Copy link
Member

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.

@halindrome
Copy link

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

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 23, 2016

See also web-platform-tests/wpt#2612

@BigBlueHat
Copy link
Member Author

Here's the fix: #87

@wpt-issue-mover
Copy link
Collaborator

This issue has been moved to web-platform-tests/wpt#8367; please continue all discussion there.

@w3c w3c locked and limited conversation to collaborators Nov 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants