cookie parsing behave differently in windows

[notch1p]

(let ((cookie (cl-cookie:make-cookie-jar)))
    (dex:get "https://welearn.sflep.com/user/prelogin.aspx?loginret=http%3a%2f%2fwelearn.sflep.com%2fuser%2floginredirect.aspx"
        :max-redirects 0
        :cookie-jar cookie))

The above code would run just fine on *unix. But on Windows (uses WINHTTP) I tried to run the same code but get an error saying

junk in string "1800, ASP.NET_SessionId=...."

And after tracing some function I think it's some thing related with parse-integer in cl-cookie, but that doesn't explain why this doesn't happen on unix. So I manually curl'd the above site:

< Set-Cookie: acw_tc=2760778917141419707405694e0ccde7c2be2e6181bf13ddd19aecf671ecdf;path=/;HttpOnly;Max-Age=1800
< Set-Cookie: ASP.NET_SessionId=rv0iyxkcae3g2irkyml3pypv; path=/; HttpOnly; SameSite=Lax

we have 2 set-cookie headers here and somehow "1800, ASP.NET_SessionId=...." are not splitted by #\, causing parse-integer to apply on the whole string, which throws the exception above.
The usocket one runs fine so i think it's a bug with winhttp (or some code in winhttp.lisp that fails to handle multiple set-cookie headers).

Meanwhile, is there a way to not use winhttp on windows? dexador.usocket.request kinda works but it's a internal function and I do not want to mess with that.

[ajberkley]

You can remove :WINDOWS from *features* before loading dexador to have it use the usocket backend by default.

I looked through the cookie header generating code for winhttp and didn't see anything obviously wrong, but I don't have a windows machine to play with right now...

[notch1p]

yes. I figured :windows only appear if you load cl+ssl and although it's a dependency of dexador you have to load it first. Also you have to make sure it knows the platform (that being you can't remove features like :os-windows and :win32)

Although it's ugly my temporary way to solve this is to load cl+ssl -> pop :windows -> then load dexador.

It's hella mess and also sbcl would throws error while ccl doesn't. this is not a good solution by any means.

[fukamachi]

Hmm, maybe it's better to have dedicated systems for each backend for manual choice, such as dexador-usocket.asd.

[notch1p]

personally I think having something like *dexador-backend* which serves as a global setting for backend and defaults to WINHTTP on windows or USOCKET on *Unix would be nice.

then again the issue above is pretty strange. I'm curious whether someone else that has a windows machine can reproduce the same error.

[fukamachi]

I added "dexador-usocket" system for loading the usocket backend explicitly.
Andrew already said this, but I don't see any differences in the parsing code for the WinHTTP backend.

[notch1p]

• the dexador-usocket works as expected on windows. I wouldn't bother to use winhttp.

But here's what I've found after taking some time to debug winhttp:
So first the backtrace:

• with usocket (the correct one)

(CL-COOKIE:PARSE-SET-COOKIE-HEADER
      "acw_tc=2760824617166543460375131ed5fcdba3997b40c5ad050ab4fa9b810ba6ac;path=/;HttpOnly;Max-Age=1800"
      "welearn.sflep.com" "/user/prelogin.aspx")
(CL-COOKIE:PARSE-SET-COOKIE-HEADER
      "ASP.NET_SessionId=v05ejyfixyhhoqnmqm1xktjb; path=/; HttpOnly; SameSite=Lax"
      "welearn.sflep.com" "/user/prelogin.aspx")

• with winhttp (the wrong one)

(CL-COOKIE:PARSE-SET-COOKIE-HEADER "acw_tc=2760778217166537661284226ea9877b7f9f0fc2ca81a27f34d67184102dfc;path=/;HttpOnly;Max-Age=1800, ASP.NET_SessionId=lsxoabet3qvdlsazd134szkl; path=/; HttpOnly; SameSite=Lax" "welearn.sflep.com" "/user/prelogin.aspx")

In the second one, We can see that the set-cookie string didn't correctly break (because of the ,) into 2 parts therefore leading to the exception, and the function call that throws the error below:

dexador/src/backend/winhttp.lisp

Line 204 in 76d5f56

(parse-set-cookie-header cookie

Following this, I checked the value of response-headers (on line 199 or the same thing on line#694 in usocket.lisp):

dexador/src/backend/winhttp.lisp

Lines 198 to 208 in 76d5f56

	(when-let (set-cookies (append (ensure-list (gethash "set-cookie" response-headers))
	(ensure-list (gethash "set-cookie2" response-headers))))
	(merge-cookies cookie-jar
	(remove nil (mapcar (lambda (cookie)
	(declare (type string cookie))
	(unless (= (length cookie) 0)
	(parse-set-cookie-header cookie
	(quri:uri-host uri)
	(quri:uri-path uri))))
	set-cookies)))))

And surprisingly, the value of (gethash "set-cookie" response-headers) aren't the same:
(As I have said, the request above contains 2 Set-cookie headers)

• On usocket, these 2 set-cookie are merged into a list which has 2 items and each contains everything from the 2 set-cookies respectively.
• On winhttp however, they are merged into a single string delimited by , hence the error

junk in string "1800, ASP.NET_SessionId=...."

so this is what it should look like (a list):

(acw_tc=2760776f17166522239694922e794ffe0c1c5cc101d25964e0a56a6c0eac35;path=/;HttpOnly;Max-Age=1800 ;; 1st set-cookie
                              ASP.NET_SessionId=v0nrawvxdex4spxdxdfcpv1t; path=/; HttpOnly;SameSite=Lax) ;; 2nd set-cookie

instead of a single string

"acw_tc=2760778217166537661284226ea9877b7f9f0fc2ca81a27f34d67184102dfc;path=/;HttpOnly;Max-Age=1800, ASP.NET_SessionId=lsxoabet3qvdlsazd134szkl; path=/; HttpOnly; SameSite=Lax"

But How? Since the code's the same... Well I think query-headers* is to be blamed:

dexador/src/backend/winhttp.lisp

Lines 55 to 64 in 76d5f56

	(defun query-headers* (req)
	(loop with hash = (make-hash-table :test 'equal)
	for (name-camelcased value) in (query-headers req)
	for name = (string-downcase name-camelcased)
	if (gethash name hash)
	do (setf (gethash name hash)
	(format nil "~A, ~A" (gethash name hash) value))
	else
	do (setf (gethash name hash) value)
	finally (return hash)))

line#61 would produce that ,. I guess it's a separator and changed it from , to #\newline, as the latter is not allowed in http header.

And to sum up we need to split the cookie on windows, I think.
So my temporary solution: 65aa605 (yikes, its really bad)
But I hope this would least provide some insight on the issue.