Skip to content

Parsing issues with Set-Cookie header #229

@nnposter

Description

@nnposter

Library http.lua does not correctly parse certain cases of Set-Cookie header.

Case 1: Cookie Merge

Consider the following HTTP response:

...
Set-Cookie: c1=aaa
Set-Cookie: c2=bbb; expires=Sun, 25-Oct-2015 04:35:13 GMT
...

The current code parses the headers as follows:

  • Cookie:
    • name="c1"
    • value="aaa,c2=bbb"
    • expires="Sun, 25-Oct-2015 04:35:13 GMT"

Patch http-parse-set-cookie.patch corrects the behavior, producing:

  • Cookie:
    • name="c1"
    • value="aaa"
  • Cookie:
    • name="c2"
    • value="bbb"
    • expires="Sun, 25-Oct-2015 04:35:13 GMT"

The patch does the following:

  • Function parse_set_cookie() is no longer fed response.header["set-cookie"], which is a concatenation of all the cookie definitions. Instead the function is invoked on each Set-Cookie header separately, before they are joined, inside parse_header().
  • This allows removal of unnecessary outer looping construct while true do ... end from the cookie parsing code.
  • Swaps the position of functions parse_set_cookie() and parse_header() inside http.lua. This is necessary due to the moved invocation point for parse_set_cookie().

The last two changes visually inflate the patch quite a lot but a smart diff can see through it.

Case 2: Comma Splitting

Consider the following HTTP response:

...
Set-Cookie: c1=aaa; path=/bbb/ccc,ddd/eee
...

Note that this is a legitimate path. (See W3C and RFC 6265 for details.)
The current code parses the header as follows:

  • Cookie:
    • name="c1"
    • value="aaa"
    • path="=/bbb/ccc"

Patch http-parse-set-cookie-comma.patch corrects the behavior by removing special parsing of the comma from parse_set_cookie(). The parsing is not needed any more due to the function now processing only one header at a time.

Clean-up

Finally I am proposing a clean-up of parse_set_cookie() code, removing unnecessary checks like if pos <= #s and converting string functional calls to the object notation. See patch http-parse-set-cookie-cleanup.patch.

The patches are meant to be applied in this order:

  1. http-parse-set-cookie.patch
  2. http-parse-set-cookie-comma.patch
  3. http-parse-set-cookie-cleanup.patch

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions