[WIN] Implement WebMutableURLRequest::setHTTPBody()
Created attachment 153661 [details] Patch
Comment on attachment 153661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153661&action=review > Source/WebKit/win/WebMutableURLRequest.cpp:298 > + Vector<char, 1024> buffer(statstg.cbSize.QuadPart); statstg.cbSize.QuadPart is a 64-bit unsigned integer. The Vector constructor takes a size_t, which is a 32-bit unsigned integer on 32-bit systems. So this will potentially overflow. We should probably return a failure result if overflow will occur. > Source/WebKit/win/WebMutableURLRequest.cpp:303 > + if (FAILED(data->Read(buffer.data(), buffer.size(), &bytesRead))) > + return E_FAIL; > + > + m_request.setHTTPBody(FormData::create(buffer.data(), bytesRead)); It's too bad we can't read the data into the FormData directly. The extra copy is unfortunate.
Created attachment 153830 [details] Patch
Attachment 153830 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/network/FormData.cpp:156: Declaration has space between type name and * in char *target [whitespace/declaration] [3] Source/WebKit/win/WebMutableURLRequest.cpp:301: Declaration has space between type name and * in char *formData [whitespace/declaration] [3] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 153832 [details] Patch
Comment on attachment 153832 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153832&action=review Looks good, but I think it could be clearer with some slight changes! > Source/WebCore/platform/network/FormData.cpp:161 > +void FormData::appendUninitializedData(size_t size, char*& data) I was initially confused about what this was supposed to do. I think it would be clearer if this method was called something like "expandDataStore". Also, couldn't it return the pointer to the start of the new allocation? This would allow us to write something like: void* target = expandDataStore(size); memcpy(target, data, size); > Source/WebKit/win/WebMutableURLRequest.cpp:303 > + httpBody->appendUninitializedData(stat.cbSize.LowPart, formData); ... Then this would be: void* formData = httpBody->expandDataStore(stat.cbSize.LowPart);
Created attachment 212355 [details] Patch
Attachment 212355 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/network/FormData.cpp', u'Source/WebCore/platform/network/FormData.h', u'Source/WebKit/win/ChangeLog', u'Source/WebKit/win/WebMutableURLRequest.cpp']" exit_code: 1 Source/WebCore/platform/network/FormData.h:133: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 212357 [details] Patch
Comment on attachment 212357 [details] Patch Very nice! r=me.
Comment on attachment 212357 [details] Patch Clearing flags on attachment: 212357 Committed r156303: <http://trac.webkit.org/changeset/156303>
All reviewed patches have been landed. Closing bug.