Bug 40476 - [Qt] Support custom open() verbs/methods in XMLHttpRequest
Summary: [Qt] Support custom open() verbs/methods in XMLHttpRequest
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: QtWebKit Unassigned
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-06-11 08:06 PDT by Andreas Kling
Modified: 2010-10-18 01:16 PDT (History)
2 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 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.
Comment 1 Andreas Kling 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.
Comment 2 Andreas Kling 2010-06-11 08:12:18 PDT 
Created attachment 58475 [details]
Proposed patch
Comment 3 Kenneth Rohde Christiansen 2010-06-11 09:13:24 PDT 
Comment on attachment 58475 [details]
Proposed patch

Are there any security implications of this?
Comment 4 Andreas Kling 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.)
Comment 5 Eric Seidel (no email) 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.
Comment 6 Dinu Jacob 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
Comment 7 Dinu Jacob 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
Comment 8 Andreas Kling 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.
Comment 9 Kenneth Rohde Christiansen 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.
Comment 10 Andreas Kling 2010-10-17 11:20:26 PDT 
Created attachment 70978 [details]
Proposed patch v2

Updated patch with more elaborate ChangeLog.
Comment 11 Andreas Kling 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.
Comment 12 Early Warning System Bot 2010-10-17 11:28:24 PDT 
Attachment 70978 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4456062
Comment 13 Andreas Kling 2010-10-18 01:16:00 PDT 
Committed r69946: <http://trac.webkit.org/changeset/69946>