Bug 31410

Summary: REGRESSION (Safari 4.0.4): HTTP 307 after a 303 after a POST re-sends POST data from the original request (31410)
Product: WebKit Reporter: masterchiff
Component: Page LoadingAssignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, beidson, emacemac7, eric, mjs, simon.fraser, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Mac/Win fix + layouttest
beidson: commit-queue-
Fix the tab, no other changes ap: review+, beidson: commit-queue-

Description masterchiff 2009-11-12 08:04:32 PST
Hey,

I'm finding that WebKit (starting with Safari 4.0.4 (WebKit 5531.21.10), has always been working before) is not behaving accordingly to http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

Steps to reproduce:

- POST some data to some page, e.g. /comments.html

- comments.html responds with "303 See Other" and "Location: http://.../goto.html?comment=1234"
To quote from the RFC:
>This method exists primarily to allow the output of a POST-activated script to redirect the
>user agent to a selected resource.
>The new URI is not a substitute reference for the originally requested resource. (!!)

- goto.html?comment=1234 is being requested via GET (so far, so good!)

- goto.html responds with "307 Temporary Redirect" and a Location header (because the URL it redirects to may change)
>The requested resource resides temporarily under a different URI. Since the redirection
>MAY be altered on occasion, the client SHOULD continue to use the Request-URI for future requests.

- Instead of GETing the new Location (/thread123.html), Safari POSTs to /thread123.html again, with the data from the original POST request. It did not do that before 4.0.4. Both Windows and Mac 4.0.4s are affected.

---

It seems that the 307 response code kinda overwrites the previous 303 and WebKit ignores that on the 303, it should have forgotten about the POST data already.

Any chance we can see this fixed? I'll have to use 301 instead of 307 for now, as a work-around, but I don't like that. Thanks!
Comment 1 Alexey Proskuryakov 2009-11-12 14:25:27 PST
<rdar://problem/7390251>
Comment 2 Simon Fraser (smfr) 2009-11-20 13:24:04 PST
Does this affect one or more public website?
Comment 3 Brady Eidson 2010-02-22 17:58:10 PST
I have a fix upcoming...
Comment 4 Brady Eidson 2010-02-22 17:59:05 PST
Note due to the design of our networking layer abstraction, I've only been able to fix this on Mac and Windows.  Other platforms are possibly/probably affected.
Comment 5 Brady Eidson 2010-02-22 18:21:00 PST
Created attachment 49252 [details]
Mac/Win fix + layouttest
Comment 6 WebKit Review Bot 2010-02-22 18:24:08 PST
Attachment 49252 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/network/cf/ResourceHandleCFNet.cpp:435:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 1 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Brady Eidson 2010-02-22 18:36:25 PST
Created attachment 49253 [details]
Fix the tab, no other changes
Comment 8 Alexey Proskuryakov 2010-02-23 09:28:15 PST
Comment on attachment 49253 [details]
Fix the tab, no other changes

r=me

I'm not sure if quietly disabling the test for other problem is best - maybe you should leave it fail for platform maintainers to decide what to do, or just go ahead and file bugs for each.
Comment 9 Brady Eidson 2010-02-23 09:30:51 PST
(In reply to comment #8)
> (From update of attachment 49253 [details])
> r=me
> 
> I'm not sure if quietly disabling the test for other problem is best - maybe
> you should leave it fail for platform maintainers to decide what to do, or just
> go ahead and file bugs for each.

It's generally our policy to not knowingly introduce layouttest failures to another platform, isn't it?

I'll file the bugs.
Comment 10 Brady Eidson 2010-02-23 09:37:13 PST
Filed https://bugs.webkit.org/show_bug.cgi?id=35300 and https://bugs.webkit.org/show_bug.cgi?id=35301 for gtk and qt to follow up on this.
Comment 11 Brady Eidson 2010-02-23 09:38:07 PST
Note Chromium should probably follow up on this too, but they don't even seem to have a Skipped list for me to follow, so...
Comment 12 Eric Seidel (no email) 2010-03-05 16:37:22 PST
Looks like this was landed:
http://trac.webkit.org/changeset/55157