Bug 72329

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] Patch

Created attachment 167544 [details] Patch Handle expected modification time.

Comment on attachment 167544 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167544&action=review > Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:81 > + 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); > +#else > + m_fileSize += fileEnd; > +#endif 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. > Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:110 > + default: > + ASSERT_NOT_REACHED(); This could be worth a comment stating that you expect encodedBlob types to have been converted already. > 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? > 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?

(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. > Right. > > 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] Patch

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] Patch

Committed r130753: <http://trac.webkit.org/changeset/130753>