http://bugs.winehq.org/show_bug.cgi?id=17914
Summary: Using \n rather then \r\n breaks HTTP RFC with HTTP_AddRequestHeadersW Product: Wine Version: 1.1.18 Platform: Other OS/Version: other Status: UNCONFIRMED Severity: minor Priority: P2 Component: wininet AssignedTo: wine-bugs@winehq.org ReportedBy: dn@devicenull.org
Half-Life 2 (and possibly other apps) uses \n characters to indicate a new line when dealing with the InternetOpenUrl function. Windows accepts this fine, and ends up with the correct output. Wine on the other hand blindly copies this into the value as it parses the line, creating an invalid HTTP request.
The call looks like this: trace:wininet:INTERNET_InternetOpenUrlW (0x1279fda0, L"http://192.168.3.89/127.0.0.1/maps/de_dust3.bsp", L"Referer: hl2://127.0.0.1:27015\n", 00000024, 84400000, 0c360e00)
the problem appears a bit later: trace:wininet:HTTP_HttpAddRequestHeadersW copying header: L"Referer: hl2://127.0.0.1:27015\n\0000" trace:wininet:HTTP_HttpAddRequestHeadersW interpreting header L"Referer: hl2://127.0.0.1:27015\n" trace:wininet:HTTP_InterpretHttpHeader field(L"Referer") Value(L"hl2://127.0.0.1:27015\n")
note the \n on the end of the value. This ends up being copied into the final HTTP request, and breaking the HTTP RFC.
The fix seems to be to just accept \n as well as \r\n sequences in HTTP_InterpretHttpHeader
http://bugs.winehq.org/show_bug.cgi?id=17914
CShadowRun signup+winehq@cshadowrun.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW Ever Confirmed|0 |1
--- Comment #1 from CShadowRun signup+winehq@cshadowrun.com 2009-03-31 13:09:05 --- *** This bug has been confirmed by popular vote. ***
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #2 from Brian Rak dn@devicenull.org 2009-03-31 13:47:23 --- Created an attachment (id=20242) --> (http://bugs.winehq.org/attachment.cgi?id=20242) Patch fixing this issue
http://bugs.winehq.org/show_bug.cgi?id=17914
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch
--- Comment #3 from Austin English austinenglish@gmail.com 2009-03-31 14:29:23 --- A testcase would help.
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #4 from Brian Rak dn@devicenull.org 2009-03-31 16:59:48 --- I'm afraid I don't understand WINE's testing code well enough to be able to write that. I can do a basic one showing the results of HttpAddRequestHeadersW to ensure that it handles \n characters if that's what you are looking for.
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #5 from Austin English austinenglish@gmail.com 2009-03-31 20:21:53 --- (In reply to comment #4)
I'm afraid I don't understand WINE's testing code well enough to be able to write that. I can do a basic one showing the results of HttpAddRequestHeadersW to ensure that it handles \n characters if that's what you are looking for.
A simple testcase testing its behavior that doesn't work without your patch, but does with your patch is what I mean.
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #6 from Dmitry Timoshkov dmitry@codeweavers.com 2009-04-01 00:14:32 --- Also it would be useful to test whether single '\r' works same way as '\n' and '\r\n'.
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #7 from Yann Droneaud yann@droneaud.fr 2009-05-26 02:53:29 --- Created an attachment (id=21321) --> (http://bugs.winehq.org/attachment.cgi?id=21321) Call to HttpSendRequest() before patch
I have a similar problem with another application where it pass "application/x-www-form-urlencoded\n\nContent-Length: 328\n" as additionnal headers in HttpSendRequest(). Requests built with those headers are misinterpreted by HTTP server, eg. the \n\n sequence is seen as end of header and all following headers are parts of the content.
Attached the WINEDEBUG log for the application. To come: same log after the patch, and two test case.
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #8 from Yann Droneaud yann@droneaud.fr 2009-05-26 02:55:35 --- Created an attachment (id=21322) --> (http://bugs.winehq.org/attachment.cgi?id=21322) Call to HttpSendRequest() after patch
Filtered WINEDEBUG log for same application after patching HttpAddRequestHeaders() function.
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #9 from Yann Droneaud yann@droneaud.fr 2009-05-26 03:00:14 --- Created an attachment (id=21323) --> (http://bugs.winehq.org/attachment.cgi?id=21323) Added test for HttpAddRequestHeaders(): check for \n sperated headers
New test case for HttpAddRequestsHeaders() (git format-patch). Not tested under MS Windows.
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #10 from Yann Droneaud yann@droneaud.fr 2009-05-26 03:02:09 --- Created an attachment (id=21324) --> (http://bugs.winehq.org/attachment.cgi?id=21324) Added test for HttpSendRequest(): check for headers separated with '\n'
New test for HttpSendRequest() (require access to codeweavers http server) (git format-patch). Not tested under MS Windows.
http://bugs.winehq.org/show_bug.cgi?id=17914
Hans Leidekker hans@meelstraat.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hans@meelstraat.net
--- Comment #11 from Hans Leidekker hans@meelstraat.net 2009-05-26 03:44:51 --- Tests in the first patch succeed on native wininet from ie8. The second patch produces failures:
http.c:933: Test failed: HttpSendRequest Failed with error 12150 http.c:939: Test failed: Read 0 bytes instead of 13
http://bugs.winehq.org/show_bug.cgi?id=17914
Yann Droneaud yann@droneaud.fr changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |yann@droneaud.fr
--- Comment #12 from Yann Droneaud yann@droneaud.fr 2009-05-26 04:10:33 --- (In reply to comment #11)
Tests in the first patch succeed on native wininet from ie8. The second patch produces failures:
http.c:933: Test failed: HttpSendRequest Failed with error 12150
12150 ERROR_HTTP_HEADER_NOT_FOUND
Indeed, my test case is broken regarding to mandatory Content-Length header... I wrote Content-Length=9 instead of Content-Length: 9 sorry.
http://bugs.winehq.org/show_bug.cgi?id=17914
Yann Droneaud yann@droneaud.fr changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #21324|0 |1 is obsolete| |
--- Comment #13 from Yann Droneaud yann@droneaud.fr 2009-05-26 04:22:10 --- Created an attachment (id=21326) --> (http://bugs.winehq.org/attachment.cgi?id=21326) Added test for HttpSendRequest(): check for headers separated with '\n' (Fixed Content-Length)
Test case fixed regarding Content-Length. (Still not tested under MS-Windows).
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #14 from Hans Leidekker hans@meelstraat.net 2009-05-26 04:50:00 --- Now the second test succeeds as well on native wininet.
http://bugs.winehq.org/show_bug.cgi?id=17914
--- Comment #15 from Yann Droneaud yann@droneaud.fr 2009-05-26 05:20:14 --- Created an attachment (id=21328) --> (http://bugs.winehq.org/attachment.cgi?id=21328) PATCH: wininet: Additionnal header can be separated by \n + strip_spaces() improvement
Here is my patch (git-format) against HTTP_HttpAddRequestHeadersW(). The new code will chop every leading carriage return and linefeed between headers.
Additionnal, there's also a patch on strip_spaces() to remove leading and trailing space (space, tab, carriage, line feed) characters. Seems not needed.
http://bugs.winehq.org/show_bug.cgi?id=17914
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |testcase
http://bugs.winehq.org/show_bug.cgi?id=17914
Mike Kaplinskiy mike.kaplinskiy@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mike.kaplinskiy@gmail.com
--- Comment #16 from Mike Kaplinskiy mike.kaplinskiy@gmail.com 2009-06-17 19:32:26 --- Fix (apparently) checked in. Commit 6c767c4e2ca1da1f1f7f678a9034ce99b0caf1f6 .
http://bugs.winehq.org/show_bug.cgi?id=17914
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED
--- Comment #17 from Austin English austinenglish@gmail.com 2009-06-17 19:34:06 --- Fixed.
http://bugs.winehq.org/show_bug.cgi?id=17914
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #18 from Alexandre Julliard julliard@winehq.org 2009-06-19 11:07:11 --- Closing bugs fixed in 1.1.24.