http://bugs.winehq.org/show_bug.cgi?id=37023
Bug ID: 37023 Summary: winhttp: HEAD-requests are not handled correctly Product: Wine Version: 1.7.23 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: major Priority: P2 Component: winhttp Assignee: wine-bugs@winehq.org Reporter: anduchs@gmail.com
Created attachment 49194 --> http://bugs.winehq.org/attachment.cgi?id=49194 Test case for this issue
When opening a HEAD-request, winhttp tries to read response-contents even though there is none coming. #29715 mentioned the same problem as fixed, but it does still happen / happen again. Regression or new case ?
Attached are a testcase and a patch.
http://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #1 from Andreas anduchs@gmail.com --- Created attachment 49195 --> http://bugs.winehq.org/attachment.cgi?id=49195 Patch to fix HEAD-requests in winhttp
http://bugs.winehq.org/show_bug.cgi?id=37023
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, patch, testcase
https://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #2 from Andreas anduchs@gmail.com --- Created attachment 49400 --> https://bugs.winehq.org/attachment.cgi?id=49400 Wine-TestSuit: Add test for winhttp HEAD requests
Unfortunately based on timing, but that seems to be the only way to detect this error.
Or does anyone have a better idea on how to detect a timeout-case inside winhttp ?
https://bugs.winehq.org/show_bug.cgi?id=37023
Mike Ellery mellery@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mellery@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=37023
Sebastian Lackner sebastian@fds-team.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sebastian@fds-team.de
https://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #3 from Sebastian Lackner sebastian@fds-team.de --- I don't see a big problem with timing based tests (especially when you send first the patch to fix it), but you should probably use something a bit more easy like GetTickCount(). Moreover, the timeout shouldn't be too high since this is only a localhost connection, and never expected to take several seconds.
I am not sure about the fix, a better method would be to set request->content_length to zero when no answer is expected, like:
--- snip --- diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index d497d7a..c07e663 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -1806,2 +1806,3 @@ static DWORD set_content_length( request_t *request ) { + static const WCHAR headW[] = {'H','E','A','D',0}; WCHAR encoding[20]; @@ -1823,2 +1824,10 @@ static DWORD set_content_length( request_t *request ) } + + /* For a HEAD request we never expect any content */ + if (request->verb && !strcmpiW(request->verb, headW)) + { + request->content_length = 0; + request->read_chunked = FALSE; + } + request->content_read = 0; --- snip ---
The main issue I see is that the tests do not even work on Windows: http://newtestbot.winehq.org/JobDetails.pl?Key=10229
Does this make the bug invalid, or does your testcase just not properly reproduce the issue?
https://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #4 from Andreas Fuchs anduchs@gmail.com ---
Does this make the bug invalid, or does your testcase just not properly reproduce the issue?
I guess my testcase does not really work. I went through some iterations on the similar issue in https://bugs.winehq.org/show_bug.cgi?id=36937 . So I'll have to get this one up to par...
Attachment 1 "Test case for this issue" should however still yield different behaviours right ?
I hope to go through the stuff again soon. Since there is still a blocker on Marvel Heros that I cannot trace down even after these two patches, I'm lacking interest a bit... ;-)
https://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #5 from Andreas Fuchs anduchs@gmail.com ---
I am not sure about the fix, a better method would be to set request->content_length to zero when no answer is expected, like:
--- snip --- diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index d497d7a..c07e663 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -1806,2 +1806,3 @@ static DWORD set_content_length( request_t *request ) {
- static const WCHAR headW[] = {'H','E','A','D',0}; WCHAR encoding[20];
@@ -1823,2 +1824,10 @@ static DWORD set_content_length( request_t *request ) }
- /* For a HEAD request we never expect any content */
- if (request->verb && !strcmpiW(request->verb, headW))
- {
request->content_length = 0;request->read_chunked = FALSE;- }
- request->content_read = 0;
--- snip ---
Looking at this patch I'm sceptical. One of the points about a head-request is, that the server's answer will contain a content_length field. So I'm not sure, what wine's winhttp returns when being asked for the content_length. I _guess_ WinHttpQueryHeaders() is not a problem. Are there other ways that the content_length-field will be accessed by the application ?
https://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #6 from Sebastian Lackner sebastian@fds-team.de --- (In reply to Andreas Fuchs from comment #4)
Attachment 1 [details] "Test case for this issue" should however still yield different behaviours right ?
Yes, the behaviour is different. Nevertheless, as it seems to fail completely on Windows, I normally wouldn't expect that any app tries to use it - except its an issue somewhere in your testcase.
(In reply to Andreas Fuchs from comment #5)
Looking at this patch I'm sceptical. One of the points about a head-request is, that the server's answer will contain a content_length field. So I'm not sure, what wine's winhttp returns when being asked for the content_length. I _guess_ WinHttpQueryHeaders() is not a problem. Are there other ways that the content_length-field will be accessed by the application ?
As far as I know the content_length field is only used for receiving the data, when the app tries to check the header fields the corresponding entry from an internal list is used instead.
https://bugs.winehq.org/show_bug.cgi?id=37023
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW URL| |http://cdn.marvelheroes.com | |/marvelheroes/binaries/marv | |elheroesinstaller.exe CC| |focht@gmx.net Summary|winhttp: HEAD-requests are |winhttp: HEAD-requests are |not handled correctly |not handled correctly | |(Marvel Heroes 2015 | |launcher) Ever confirmed|0 |1 Severity|major |normal
--- Comment #7 from Anastasius Focht focht@gmx.net --- Hello folks,
confirming, still present.
Lowering severity since there is only one application in the wild reported, suffering from this yet.
Retested with Marvel Heroes 2015 and Wine 1.7.34 ('winetricks -q winhttp' obviously works around).
$ sha1sum marvelheroesinstaller.exe 029aac847631e768a3a99e55b5202e4ad9748a42 marvelheroesinstaller.exe
$ du -sh marvelheroesinstaller.exe 64M marvelheroesinstaller.exe
$ wine --version wine-1.7.34
Regards
https://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #8 from Hans Leidekker hans@meelstraat.net --- Created attachment 50442 --> https://bugs.winehq.org/attachment.cgi?id=50442 patch
Can you try this patch?
https://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #9 from Anastasius Focht focht@gmx.net --- Hello Hans,
the patch works on top of wine-1.7.34, thanks :)
The bootstrapper/launcher can now download metadata, full client and all incremental patches (13+ GiB) from a clean 32-bit WINEPREFIX without any overrides.
Regards
https://bugs.winehq.org/show_bug.cgi?id=37023
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |michael.m.hensley@gmail.com
--- Comment #10 from Anastasius Focht focht@gmx.net --- *** Bug 37682 has been marked as a duplicate of this bug. ***
https://bugs.winehq.org/show_bug.cgi?id=37023
--- Comment #11 from Hans Leidekker hans@meelstraat.net --- Should be fixed by 27ba8c834326bb526fc57148feb6d83883c7a7c6.
https://bugs.winehq.org/show_bug.cgi?id=37023
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |27ba8c834326bb526fc57148feb | |6d83883c7a7c6 Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #12 from Anastasius Focht focht@gmx.net --- Hello folks,
this works now.
Fixed by commit http://source.winehq.org/git/wine.git/commitdiff/27ba8c834326bb526fc57148feb...
Thanks Hans
Regards
https://bugs.winehq.org/show_bug.cgi?id=37023
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #13 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 1.7.35.
-
wine-bugs＠winehq.org