Bug 40476

Summary:

[Qt] Support custom open() verbs/methods in XMLHttpRequest

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

Page Loading

Assignee:

QtWebKit Unassigned <webkit-qt-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dinu.jacob, webkit-ews

Priority:

Keywords:

Qt, QtTriaged

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch	kenneth: review-
Proposed patch v2	kenneth: review+

Andreas Kling

Reported 2010-06-11 08:06:06 PDT

For Qt 4.7 we can implement this by falling back to QNetworkAccessManager::CustomOperation for any verb that's not specifically supported by QNAM.

Attachments
Proposed patch (1.57 KB, patch) 2010-06-11 08:12 PDT, Andreas Kling	kenneth: review-	Details Formatted Diff Diff
Proposed patch v2 (3.73 KB, patch) 2010-10-17 11:20 PDT, Andreas Kling	kenneth: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Andreas Kling

Comment 1 2010-06-11 08:11:31 PDT

It should be noted that current QtWebKit hangs completely when attempting to use an unknown verb with XHR.

Andreas Kling

Comment 2 2010-06-11 08:12:18 PDT

Created attachment 58475 [details] Proposed patch

Kenneth Rohde Christiansen

Comment 3 2010-06-11 09:13:24 PDT

Comment on attachment 58475 [details] Proposed patch Are there any security implications of this?

Andreas Kling

Comment 4 2010-06-12 17:54:25 PDT

(In reply to comment #3) > (From update of attachment 58475 [details]) > Are there any security implications of this? The verb is sanitized in XMLHttpRequest::open() (WebCore/xml/XMLHttpRequest.cpp) AFAICT the Mac port does the same thing as this patch (passes the method argument verbatim.)

Eric Seidel (no email)

Comment 5 2010-06-12 18:47:11 PDT

Comment on attachment 58475 [details] Proposed patch This needs a layout test. It should be possible to test this with an http test.

Dinu Jacob

Comment 6 2010-10-15 08:26:15 PDT

(In reply to comment #5) > (From update of attachment 58475 [details]) > This needs a layout test. It should be possible to test this with an http test. Layout test with custom verb already exists under xmlhttprequest. For example: methods.html uses the verbs "WKFOOBAR", "HEAD", "SEARCH" etc

Dinu Jacob

Comment 7 2010-10-15 08:28:52 PDT

(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 58475 [details] [details]) > > This needs a layout test. It should be possible to test this with an http test. > > Layout test with custom verb already exists under xmlhttprequest. For example: methods.html uses the verbs "WKFOOBAR", "HEAD", "SEARCH" etc Test File: LayoutTests/http/tests/xmlhttprequest/methods.html

Andreas Kling

Comment 8 2010-10-15 08:30:16 PDT

Comment on attachment 58475 [details] Proposed patch Resetting r? since this is covered by existing (skipped) tests. They will be unskipped once the Qt buildbot is updated to Qt 4.7.

Kenneth Rohde Christiansen

Comment 9 2010-10-15 10:18:26 PDT

Comment on attachment 58475 [details] Proposed patch the ChangeLog doesn't explain why this is a good thing.

Andreas Kling

Comment 10 2010-10-17 11:20:26 PDT

Created attachment 70978 [details] Proposed patch v2 Updated patch with more elaborate ChangeLog.

Andreas Kling

Comment 11 2010-10-17 11:25:52 PDT

Thanks for the review Kenneth :) There's a missing 4.7 guard in httpMethod(), I'll land with that tomorrow, gotta run.

Early Warning System Bot

Comment 12 2010-10-17 11:28:24 PDT

Attachment 70978 [details] did not build on qt: Build output: http://queues.webkit.org/results/4456062

Andreas Kling

Comment 13 2010-10-18 01:16:00 PDT

Committed r69946: <http://trac.webkit.org/changeset/69946>

Note You need to log in before you can comment on or make changes to this bug.