| Summary: | [Soup] Should not ignore body for redirection responses | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Gustavo Noronha (kov) <gustavo> |
| Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | commit-queue, eric, jmalonzo, mrobinson, svillar |
| Priority: | P2 | Keywords: | Gtk, Soup |
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Bug Depends on: | 44261 | ||
| Bug Blocks: | |||
| Attachments: | |||
|
Description
Gustavo Noronha (kov)
2009-09-16 06:34:37 PDT
I created https://bugzilla.gnome.org/show_bug.cgi?id=629088 in gnome bugzilla as this could be considered as a pure libsoup bug. Created attachment 67015 [details]
Do not ignore body on 302 for redirect purpouses
danw suggested to fix that in WebKitGtk+ as it's not libsoup's business according to his words.
I added also a LayoutTest to check this behaviour
Created attachment 67041 [details]
Do not ignore body on 302 for redirect purpouses (SoupURILoader version)
Just in case #44261 and all the SoupURILoader stuff lands before this one
Comment on attachment 67015 [details]
Do not ignore body on 302 for redirect purpouses
Looks right to me. I think it would be cleaner to tell layoutTestController to waitUntilDone in the first page that is loaded, though. Not opposed to keeping it as is if there's a reason I missed, though.
Comment on attachment 67041 [details] Do not ignore body on 302 for redirect purpouses (SoupURILoader version) View in context: https://bugs.webkit.org/attachment.cgi?id=67041&action=review This looks very reasonable to me, but because of the funky ChangeLogs and Gustavo's comment, I'll r-. > LayoutTests/ChangeLog:13 > + Tests that 302 redirections without Location do not ignore the > + body for redirection purpouses (the body could have a meta > + http-equiv=refresh) > + > + [Soup] Should not ignore body for redirection responses > + https://bugs.webkit.org/show_bug.cgi?id=29299 > + > + * http/tests/navigation/redirect302-metaredirect-expected.txt: Added. > + * http/tests/navigation/redirect302-metaredirect.html: Added. I think the order of these is typically reversed with the bug link coming first. > WebCore/ChangeLog:11 > + Body is now provided to WebKitGtk+ in some redirections (like 302) > + because it could be used by servers to perform clunky redirections > + for example using http-equiv=REFRESH > + > + [Soup] Should not ignore body for redirection responses > + https://bugs.webkit.org/show_bug.cgi?id=29299 > + Same thing here. Created attachment 69917 [details]
Do not ignore body on 302 for redirect purpouses (SoupURILoader version)
New version
* fixed ChangeLogs
* Updated the patch as now m_msg was replaced by m_soupMessage
Regarding the waitUntilDone the main reason to design the test like that is that all the http/test/redirect* tests follow the same structure. I think in general it's a good idea to harmonize the way tests works
Comment on attachment 69917 [details]
Do not ignore body on 302 for redirect purpouses (SoupURILoader version)
Oki!
(In reply to comment #7) > (From update of attachment 69917 [details]) > Oki! Great! We need #44261 first before landing this one. Seems bug 44261 landed. What should be done here? Can we land this now? (In reply to comment #10) > Can we land this now? Yeah we can land the SoupURILoader version (ignore the other one) Comment on attachment 69917 [details] Do not ignore body on 302 for redirect purpouses (SoupURILoader version) Rejecting patch 69917 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 69917]" exit_code: 2 Cleaning working directory Updating working directory Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=69917&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=29299&ctype=xml Processing 1 patch from 1 bug. Processing patch 69917 from bug 29299. Failed to run "[u'/Users/abarth/git/webkit-queue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Gustavo Noronha Silva', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/4427032 Created attachment 70851 [details]
Do not ignore body on 302 for redirect purpouses (SoupURILoader version)
New version of the fix properly rebased against current master
Comment on attachment 67015 [details] Do not ignore body on 302 for redirect purpouses Cleared Gustavo Noronha Silva's review+ from obsolete attachment 67015 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 69917 [details] Do not ignore body on 302 for redirect purpouses (SoupURILoader version) Cleared Gustavo Noronha Silva's review+ from obsolete attachment 69917 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 70851 [details] Do not ignore body on 302 for redirect purpouses (SoupURILoader version) Clearing flags on attachment: 70851 Committed r69861: <http://trac.webkit.org/changeset/69861> All reviewed patches have been landed. Closing bug. |