search

orthecreedence / wookie

Asynchronous HTTP server in common lisp
http://wookie.lyonbros.com/
MIT License
189 stars 19 forks source link

WOOKIE:SEND-RESPONSE duplicates Content-Length header when headers is given as a hash-table with lowercase string keys #76

Closed martinflack closed 7 years ago

martinflack commented 8 years ago

In WOOKIE:SEND-RESPONSE (wookie-20160628-git) a local variable HEADERS is created using (RESPONSE-HEADERS RESPONSE) and passed-in HEADERS. They are converted from hash-tables to plists as necessary, and appended together. The code following that does not require the plist type for the various calls to GET-HEADER but it does for MAP-PLIST.

The issue is the if you pass in a hash-table from, e.g. CARRIER:REQUEST, where the hash-table keys are HTTP header names as lowercase strings, the check for (GET-HEADER HEADERS :CONTENT-LENGTH) fails because it checks the plist of string keys against a keyword symbol; thus a duplicate Content-Length header is then added.

Possible fixes:

Check both :CONTENT-LENGTH and "content-length". If there's a desire to support capitalized headers then also "Content-Length".

The plist creation could intern header names into the keyword package. (Since this accrues, it may have a security implication with potential denial-of-service via memory exhaustion.)

The key passed to GET-HEADER could be :CONTENT-LENGTH or "content-length" based on (KEYWORDP (CAR HEADERS)) which would be a heuristic test on how the header keys are given.

(It may be worth protecting against the two original sources of headers having disparate techniques for keys. After converting to plists, checking the first key on each could result in one of them being converted to match the other if they are different types.)

Another approach is to maintain hash-tables all the way through, perhaps even the two of them, and avoid conversion to plist, and use HASH-TABLE-TEST to determine how keys are given, documenting that EQ/EQL hash-tables will be assumed to carry keyword keys and EQUAL/EQUALP hash-tables will be assumed to carry strings keys (or, rather, anything that's not EQ/EQL as some implementations allow customized equality functions).

For now, I worked around the issue by converting to a plist ahead of time before calling the function. ;-)

PuercoPop commented 8 years ago

From skimming the code wouldn't replacing (getf ,headers key) in get-header with a getf version that compares with string-equal instead of eq fix the issue with the least effort?

martinflack commented 8 years ago

Yup, I think so.