As the summary states, you won't be able to upload images to your Google+ account using QtTestBrowser from QtWebKit 2.2 branch. The reason seems to be the QIODevice parameter that points to the POST data in QNetworkAccesssManager::createRequest, is simply invalid and accessing it causes a crash if you do not first check for the "QNetworkRequest::Content-Length" attribute.
The cause of this bug turns out to be the lack of support for BlobData in QtNetworkReplyHandler. Actually, the fact that QtNetworkReplyHandler does not even check for BlobData when form upload data type is actually a bug since it would be attempting to access a non-existent iodevice. In any case, I am not sure how to implement support for this functionality, but the lack of such support means, no image upload support in Google+ and worst yet a crash if you attempt to read from the POST iodevice in QNetworkAccessManager::createRequest.
*** Bug 68581 has been marked as a duplicate of this bug. ***
We have enabled BLOBs but are not handling them at all in network request. What is worse is that we treat the formdata types as if they were a binary choice of either Data or File.
Created attachment 167541 [details]
Created attachment 167544 [details]
Handle expected modification time.
Comment on attachment 167544 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=167544&action=review
> + qint64 fileEnd = fi.size();
> +#if ENABLE(BLOB)
> + if (element.m_fileLength != BlobDataItem::toEndOfFile)
> + fileEnd = qMin<qint64>(fi.size(), element.m_fileStart + element.m_fileLength);
> + m_fileSize += qMax<qint64>(0, fileEnd - element.m_fileStart);
> + m_fileSize += fileEnd;
I think this would be slightly easier to follow if you keep the fileEnd declaration inside the #if ENABLE(BLOB) and keep using fi.size() directly outside.
> + default:
> + ASSERT_NOT_REACHED();
This could be worth a comment stating that you expect encodedBlob types to have been converted already.
> + else if (element.m_fileLength != BlobDataItem::toEndOfFile && m_currentDelta == element.m_fileLength)
> + moveToNextElement();
If I'm following all this, m_fileLength is provided by JS and might be larger than the actual file, in that case is there any protection to prevent looping forever or cutting the end of the last elements?
> +#if ENABLE(BLOB)
> + // Check if there is a blob in the form data.
> + bool hasBlob = false;
It would be nice to put all the blob logic added to getIODevice in a function named something like convertBlobFormData instead.
Are any of those other skipped blob tests easy to cover?
(In reply to comment #6)
> > Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:169
> > + else if (element.m_fileLength != BlobDataItem::toEndOfFile && m_currentDelta == element.m_fileLength)
> > + moveToNextElement();
> If I'm following all this, m_fileLength is provided by JS and might be larger than the actual file, in that case is there any protection to prevent looping forever or cutting the end of the last elements?
The condition just prior to this one checks for endOfFile, this is only to terminate if we are before end-of-file but at the end of the file-length specifier.
> > Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:647
> > +#if ENABLE(BLOB)
> > + // Check if there is a blob in the form data.
> > + bool hasBlob = false;
> > +
> It would be nice to put all the blob logic added to getIODevice in a function named something like convertBlobFormData instead.
> > LayoutTests/platform/qt/TestExpectations:587
> > http/tests/local/blob/send-hybrid-blob.html
> Are any of those other skipped blob tests easy to cover?
The rest of the blobdata tests all depend on either DragAndDrop logic which we don't support in layout tests, or the FILE_SYSTEM api, which don't have enabled (and which would require additional support in QNetworkRequestHandler).
Created attachment 167719 [details]
Landed in r130746, but webkit-path land crashed before closing bug.
(In reply to comment #9)
> Landed in r130746, but webkit-path land crashed before closing bug.
Reopen, because it made 45 tests crash - http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r130746%20%2843587%29/results.html
Created attachment 167742 [details]
Committed r130753: <http://trac.webkit.org/changeset/130753>