Bug 58727 - [Qt] QWebView can not show eBay pages correctly
Summary: [Qt] QWebView can not show eBay pages correctly
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Windows XP
: P1 Critical
Assignee: Luiz Agostini
URL:
Keywords: Qt, QtTriaged
: 50746 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-04-16 02:06 PDT by zhiyiliu
Modified: 2011-07-06 10:11 PDT (History)
14 users (show)

See Also:


Attachments
patch (2.54 KB, patch)
2011-05-05 15:59 PDT, Luiz Agostini
hausmann: review-
Details | Formatted Diff | Diff
patch (3.09 KB, patch)
2011-05-26 14:50 PDT, Luiz Agostini
vestbo: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zhiyiliu 2011-04-16 02:06:38 PDT
For some eBay pages, QWebView always shows blank page. I've checked the LoadFinished signal, and the OK is false. The same pages can be rendered correctly on popular browsers, such as IE, Safari or Chrome. I can consistently reproduce the problem both on Windows and Ubuntu Linux.

Here's how to reproduce it (a 2 minute effort),

1. Create a QT Gui project
2. Drag a QWebView widget to the form
3. In the MainWindow constructor, add ui->webView->setUrl(QUrl("http://www.ebay.com"));
4. Now run your project
5. Login to eBay in your app, click on the top-right "My eBay" link, and you get a blank page.
Comment 1 Benjamin Poulain 2011-04-17 10:22:43 PDT
Moving to P1, this should be investigated before QtWebKit 2.2.

I can reproduce with trunk on Linux. When clicking on "MyEbay" (or loading http://my.ebay.com/ ) after login, I only get "OK" on the page.

Changing the user-agent string to the one of Safari fixes the issue. We should have a look for the missing bits like for hotmail. If no sensible workaround can be found, we will close this bug.

We should also raise the issue to ebay.com.
Comment 2 zhiyiliu 2011-04-19 01:50:44 PDT
(In reply to comment #1)
> Moving to P1, this should be investigated before QtWebKit 2.2.
> 
> I can reproduce with trunk on Linux. When clicking on "MyEbay" (or loading http://my.ebay.com/ ) after login, I only get "OK" on the page.
> 
> Changing the user-agent string to the one of Safari fixes the issue. We should have a look for the missing bits like for hotmail. If no sensible workaround can be found, we will close this bug.
> 
> We should also raise the issue to ebay.com.

I just did some more tests. If I put "Safari/533.20.27" or "Safari/531.22.7" in the user-agent, it will break. But if I take the Safari part out, it will work. In fact, anything other than "Safari" (such as "my browser") for user-agent would work fine.
Comment 3 Luiz Agostini 2011-05-05 15:59:46 PDT
Created attachment 92487 [details]
patch
Comment 4 Kenneth Rohde Christiansen 2011-05-06 01:53:56 PDT
Comment on attachment 92487 [details]
patch

could this be a security issue?
Comment 5 Luiz Agostini 2011-05-06 10:13:07 PDT
(In reply to comment #4)
> (From update of attachment 92487 [details])
> could this be a security issue?

I am not sure. I think that it may be not because in this specific case QNAM is not emitting the sslErrors signal. But it would be nice to have Markus opinion on that.
Comment 6 Markus Goetz 2011-05-10 02:58:21 PDT
Can you backtrace to where QNAM is setting/emitting the ProtocolFailure?
Comment 7 Luiz Agostini 2011-05-16 10:18:52 PDT
(In reply to comment #6)
> Can you backtrace to where QNAM is setting/emitting the ProtocolFailure?

Sure, I will do it today.

But more then finding out what is going on in this specific case, I would like to ignore all the errors that are not related to security issues.

In this case for example the error is not SslHandshakeFailedError, no sslErrors signal is been emited (I will confirm it) and the response content seems to be good enough for WebKit to render the page and proceed the eBay navigation, meaning the the pair user/password has been accepted by eBay.

What I would like to know is how to identify the security issues with the information provided by QNetworkReply. With this information I will be able to ignore the other QNAM errors and proceed based on message headers and contents only (if they are present), on a best effort basis.
Comment 8 Simon Hausmann 2011-05-25 18:30:29 PDT
Comment on attachment 92487 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=92487&action=review

r- because neither the ChangeLog nor the hunk in QNetworkReplyHandler explain why this is done, i.e. why QNAM sets ProtocolFailure but we choose to ignore it.

> Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:430
> +    if (httpStatusCode == 200 && reply->error() == QNetworkReply::ProtocolFailure)
> +        return true;
> +

Please find out why QNAM is setting ProtocolFailure as error here. If the situation is justified, we should explain with a comment why we ignore the error.
Comment 9 Luiz Agostini 2011-05-26 14:50:06 PDT
Created attachment 95047 [details]
patch
Comment 10 Simon Hausmann 2011-05-26 20:18:26 PDT
Comment on attachment 95047 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95047&action=review

> Source/WebCore/ChangeLog:11
> +        QNAM is having problems when uncompressing some of the contents returned by eBay and
> +        notifing error QNetworkReply::ProtocolFailure. It happens that the HTTP status code
> +        for the same message is 200 OK and QNetworkReply::ProtocolFailure does not seem to be
> +        related to any security issue.

Thanks for the analysis :). So isn't this actually a Qt bug? I mean, the fact that we get _some_ data but an error is set makes me think that either

    a) the data was successfully decompressed but Qt thinks that there's an error even though there isn't (interpreting zlib results incorrectly?)

or

    b) there was a real error decompressing the data and we deliver corrupted content to webkit, but as it turns out webkit handles the corrupted content and we don't see any visual glitches in the resulting rendering.

Do you know which one it is?
Comment 11 Luiz Agostini 2011-05-27 11:06:23 PDT
(In reply to comment #10)
> (From update of attachment 95047 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95047&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        QNAM is having problems when uncompressing some of the contents returned by eBay and
> > +        notifing error QNetworkReply::ProtocolFailure. It happens that the HTTP status code
> > +        for the same message is 200 OK and QNetworkReply::ProtocolFailure does not seem to be
> > +        related to any security issue.
> 
> Thanks for the analysis :). So isn't this actually a Qt bug? I mean, the fact that we get _some_ data but an error is set makes me think that either
> 
>     a) the data was successfully decompressed but Qt thinks that there's an error even though there isn't (interpreting zlib results incorrectly?)
> 
> or
> 
>     b) there was a real error decompressing the data and we deliver corrupted content to webkit, but as it turns out webkit handles the corrupted content and we don't see any visual glitches in the resulting rendering.
> 
> Do you know which one it is?

I have always been working with hypothesis b.

After a quick look at the QNAM's code now I think that the server is closing the connection a bit early and some bits of the message are lost. So, there were indeed a protocol failure and the content is probably corrupted.
Comment 12 Luiz Agostini 2011-05-31 13:04:52 PDT
Simon, the idea was to ignore those QNAM errors that are not security issues, if there is some content to WebKit to deal with.
I think that it would not be very different from actually receiving not compressed corrupted content from the server.

It seems that QNAM aborts just after noticing the error and before uncompressing the last data chunk. It would be better, considering this best effort policy, to uncompress all the received content before aborting.
Comment 13 Luiz Agostini 2011-06-09 11:52:09 PDT
How should we proceed here?
Comment 14 Luiz Agostini 2011-06-14 10:40:58 PDT
Does anyone have any opinions regarding this issue?

Although I think that this change is OK, we may just close this bug and raise the issue to ebay.com.
Comment 15 Kenneth Rohde Christiansen 2011-06-14 15:31:19 PDT
(In reply to comment #14)
> Does anyone have any opinions regarding this issue?
> 
> Although I think that this change is OK, we may just close this bug and raise the issue to ebay.com.

Adding christian as he might know people who can actually raise that issue.
Comment 16 Robert Hogan 2011-06-20 12:15:13 PDT
*** Bug 50746 has been marked as a duplicate of this bug. ***
Comment 17 Joel Parks 2011-06-27 14:02:53 PDT
(In reply to comment #16)
> *** Bug 50746 has been marked as a duplicate of this bug. ***

as part of the discussion in 50746, the Qt bug was raised:
http://bugreports.qt.nokia.com/browse/QTBUG-16022

And the associated analysis shows that the Ebay has delivered gzip-compressed content without end-of-stream marker.  


(In reply to comment #12)
> Simon, the idea was to ignore those QNAM errors that are not security issues, if there is some content to WebKit to deal with.
> I think that it would not be very different from actually receiving not compressed corrupted content from the server.
> 
> It seems that QNAM aborts just after noticing the error and before uncompressing the last data chunk. It would be better, considering this best effort policy, to uncompress all the received content before aborting.


I am in favor of that "best effort policy" attempt to uncompress all the received content before aborting.
Comment 18 Thijs 2011-06-29 01:31:43 PDT
I encounter the same bug with http://sports.qq.com
At first I thought it was a generic gzip-handling issue, but now I assume this site is also missing the end-of-stream marker. 

I am also very much in favor of "best effort policy". Can I help out?
Comment 19 Thijs 2011-06-29 01:53:37 PDT
I saved the gzipped reply from http://sports.qq.com to a local file and then ran zcat to decompress it. It didn't report any problems at all, so I'm not yet sure if sports.qq.com also fails because of missing end-of-stream marker, or because some other problem.
Comment 20 Thomas Thrainer 2011-06-29 01:56:53 PDT
Comment #1 states that changing the user-agent string fixes the issue with eBay, maybe sports.qq.com does deliver different gzip-streams for different user-agents as well?
Comment 21 Luiz Agostini 2011-06-29 09:13:29 PDT
(In reply to comment #18)
> I encounter the same bug with http://sports.qq.com
> At first I thought it was a generic gzip-handling issue, but now I assume this site is also missing the end-of-stream marker. 
> 
> I am also very much in favor of "best effort policy". Can I help out?

I could not reproduce the problem with http://sports.qq.com. 
Could you provide a sequence of steps that I could use to reproduce it? I must say that I cannot understand much of that page. :)
Comment 22 Thijs 2011-06-30 01:59:45 PDT
@Thomas : I've created a separate test case and found which code causes the error (the returned data is not uncompressed, but shown inside the browser):

1) create a custom QNetworkAccessManager and override createRequest(...)
2) inside the function createRequest(...) create a copy of the request object (so I can modify it) as such :  QNetworkRequest reqCopy(req);
3) then do: reqCopy.setRawHeader("Accept-Encoding", "gzip"); //this causes the problems

Note that the value ("gzip" here) doesn't matter. Just setting the Accept-Encoding header confuses QtWebKit. 

when I disable this setRawHeader() call, it works 100% reliable (so gzipped content is handled correctly if i didn't set the "Accept-Encoding" header).


note: sports.qq.com sometimes returns uncompressed data, but usually it is compressed. You might need to try a few times and make sure caching is disabled.
I disabled cache with :
reqCopy.setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::AlwaysNetwork);
Comment 23 Thijs 2011-06-30 02:17:05 PDT
(In reply to comment #22)

from the source code of qhttpnetworkconnection.cpp :


====================
    value = request.headerField("accept-encoding");
    if (value.isEmpty()) {
#ifndef QT_NO_COMPRESS
        request.setHeaderField("Accept-Encoding", "gzip");
        request.d->autoDecompress = true;
#else
        // if zlib is not available set this to false always
        request.d->autoDecompress = false;
#endif
    }
====================


I believe this is a bug. If I manually set the "accept-encoding" header, autoDecompress becomes false (even if I set it to "gzip").
Comment 24 Thijs 2011-06-30 02:41:29 PDT
(In reply to comment #23)

> I believe this is a bug. If I manually set the "accept-encoding" header, autoDecompress becomes false (even if I set it to "gzip").

Created a new bug report this this issue with a proposed solution:
https://bugs.webkit.org/show_bug.cgi?id=63696
Comment 25 Tor Arne Vestbø 2011-07-05 06:17:05 PDT
Comment on attachment 95047 [details]
patch

I believe this is related to the Qt network stack not dealing with a missing EOF marker in the gzip data, and we should fix it in Qt, not work around it here.
Comment 26 Ademar Reis 2011-07-05 10:28:07 PDT
(In reply to comment #25)
> (From update of attachment 95047 [details])
> I believe this is related to the Qt network stack not dealing with a missing EOF marker in the gzip data, and we should fix it in Qt, not work around it here.

The bug in Qt is http://bugreports.qt.nokia.com/browse/QTBUG-16022, how can we make sure this bug is closed on qt-4.7 and qt-4.8 (worst case just qt-4.8)?
Comment 27 Luiz Agostini 2011-07-05 10:30:49 PDT
(In reply to comment #25)
> (From update of attachment 95047 [details])
> I believe this is related to the Qt network stack not dealing with a missing EOF marker in the gzip data, and we should fix it in Qt, not work around it here.

The idea was to have a workaround for 2.2. But I agree with you.
Then I think that we should close this bug.
Comment 28 Ademar Reis 2011-07-06 10:11:32 PDT
This was fixed in Qt (http://bugreports.qt.nokia.com/browse/QTBUG-16022). I've tested it (using qt-4.7) and it works fine (the fix will also be merged into 4.8 in the next days).