WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
35300
(GTK) HTTP 307 after a 303 after a POST re-sends POST data from the original request
https://bugs.webkit.org/show_bug.cgi?id=35300
Summary
(GTK) HTTP 307 after a 303 after a POST re-sends POST data from the original ...
Brady Eidson
Reported
2010-02-23 09:33:02 PST
HTTP 307 after a 303 after a POST re-sends POST data from the original request. See
https://bugs.webkit.org/show_bug.cgi?id=31410
for more discussion and the fix that went in for Mac and Windows. GTK needs to do the same.
Attachments
Patch
(4.56 KB, patch)
2013-04-05 14:05 PDT
,
Alberto Garcia
gustavo
: review-
gustavo
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alberto Garcia
Comment 1
2012-12-12 11:04:29 PST
Looks like in GTK the second redirection doesn't even work (I get a timeout). I'm taking a look at this.
Dan Winship
Comment 2
2013-01-11 07:30:35 PST
So with the changes from
bug 88961
, we now make the correct series of requests. However, the test still fails because DRT doesn't print the right strings on redirect. Eg, -expected has:
http://127.0.0.1:8000/loading/resources/post-to-303-target.php
- willSendRequest <NSURLRequest URL
http://127.0.0.1:8000/loading/resources/303-to-307-target.php
, main document URL
http://127.0.0.1:8000/loading/resources/303-to-307-target.php
, http method GET> redirectResponse <NSURLResponse
http://127.0.0.1:8000/loading/resources/post-to-303-target.php
, http status code 303> but we output:
http://127.0.0.1:8000/loading/resources/303-to-307-target.php
- willSendRequest <NSURLRequest URL
http://127.0.0.1:8000/loading/resources/303-to-307-target.php
, main document URL
http://127.0.0.1:8000/loading/resources/303-to-307-target.php
, http method GET> redirectResponse <NSURLResponse
http://127.0.0.1:8000/loading/resources/post-to-303-target.php
, http status code 303> (The first URL is wrong.)
youenn fablet
Comment 3
2013-02-08 07:27:44 PST
EFL has a similar bug (
https://bugs.webkit.org/show_bug.cgi?id=93214
) for which I submitted a patch. As the code is shared with Gtk (libsoup backend), this patch may be of interest. I actually ran DRT GTK with and without the patch. Without the patch, the last request is a POST (possible regression?). With the patch, the last request is a GET, as expected. As of the DRT URL bug, the WebKitWebResource private uri field is changed in FrameLoaderClient::dispatchWillSendRequest. I was wondering whether it would be a more appropriate model to keep this field unchanged accross redirections? If so, with a small change to willSendRequestCallback, this bug would probably be fixed. I can provide a patch along those lines.
Alberto Garcia
Comment 4
2013-04-05 14:05:06 PDT
Created
attachment 196679
[details]
Patch (In reply to
comment #3
)
> As of the DRT URL bug, the WebKitWebResource private uri field is > changed in FrameLoaderClient::dispatchWillSendRequest.
> I was wondering whether it would be a more appropriate model to keep > this field unchanged accross redirections?
Yes, this actually seems to be the problem. Do we need to change that field? The identifier is supposed to be associated to the initial request. It works fine if we do it like that, here's the patch, I tried all http/tests and it doesn't seem to break anything.
WebKit Review Bot
Comment 5
2013-04-05 14:08:27 PDT
Attachment 196679
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/gtk-wk1/TestExpectations', u'Source/WebKit/gtk/ChangeLog', u'Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp', u'Tools/ChangeLog', u'Tools/DumpRenderTree/gtk/DumpRenderTree.cpp']" exit_code: 1 Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1284: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alberto Garcia
Comment 6
2013-04-09 09:25:38 PDT
One other possibility if we don't want to change the current behavior is to add a new 'original-uri' property to the WebKitWebResource class. I can easily prepare a patch. (In reply to
comment #4
)
> Do we need to change that field? The identifier is supposed to be > associated to the initial request.
One thing to note, though, is that we're currently updating the uri property (which is read-only) by directly accessing the private structure from a different class... Gustavo wrote the original code (
bug 29134
), adding him to Cc.
WebKit Commit Bot
Comment 7
2013-04-10 18:32:18 PDT
Attachment 196679
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/gtk-wk1/TestExpectations', u'Source/WebKit/gtk/ChangeLog', u'Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp', u'Tools/ChangeLog', u'Tools/DumpRenderTree/gtk/DumpRenderTree.cpp']" exit_code: 1 Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1284: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
youenn fablet
Comment 8
2013-04-11 00:27:42 PDT
(In reply to
comment #6
)
> One other possibility if we don't want to change the current behavior > is to add a new 'original-uri' property to the WebKitWebResource > class.
Does that new property be useful outside this test? If created for the purpose of the test, I would avoid doing that. A temporary workaround may be to create an expected file for that test. We would at least check that there is no regression in the POST-303-307 behavior.
> One thing to note, though, is that we're currently updating the uri > property (which is read-only) by directly accessing the private > structure from a different class...
Yes, when reading that code, I thought it might be better to encapsulate this as a function call, if this code is to remain.
Gustavo Noronha (kov)
Comment 9
2013-04-11 06:39:54 PDT
Comment on
attachment 196679
[details]
Patch Yeah, the URI is to remain read-only for API users, it should only be updated by internal code, that is why it's done the way it is. I'm OK with adding an internal call to update it instead of poking the private structure, though. I think adding a new property for original-uri is a good idea, since it maintains the API expectations while still providing complete information. All that said, wk1 is in maintenance mode and there was a decision not to add new API to it unless strictly necessary, so I wonder, do we really want to fix this test at this point? r- for the change in API expectation, adding a new property sounds like a better solution.
Alberto Garcia
Comment 10
2013-04-11 06:52:55 PDT
(In reply to
comment #9
)
> All that said, wk1 is in maintenance mode and there was a decision > not to add new API to it unless strictly necessary, so I wonder, do > we really want to fix this test at this point?
I think you're right, actually.
Michael Catanzaro
Comment 11
2017-10-21 19:22:23 PDT
Closing since this looks like an old WK1 bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug