New issue
Advanced search Search tips

Issue 869117 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Aug 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Form re-submission makes GET request instead of POST when initial navigation hit a network error

Project Member Reported by lukasza@chromium.org, Jul 30

Issue description

REPRO:

1. Navigate to http://ec2-54-71-23-13.us-west-2.compute.amazonaws.com/
   This contains the following HTML page:
    <html>
	<head>
		<title>Test Form</title>
	</head>
	<body>
		<form method="POST" action="submit.php">
			<input type="text" name="test" value="Test Value">
			<input type="submit" value="Do POST">
		</form>
	</body>
    </html>

2. In DevTools, in Network tab, check "Offline" checkbox to simulate going offline

3. Click the "Submit" button

4. As expected an error page will be shown (net::ERR_INTERNET_DISCONNECTED / offline dino)

5. Click "Reload"
   (http://ec2-54-71-23-13.us-west-2.compute.amazonaws.com/submit.php would show whether
    the request is GET or POST + it would show the request data)

EXPECTED BEHAVIOR: POST request is resubmitted

ACTUAL BEHAVIOR: GET request is made (AFAICT because step 4 called NCI::RendererDidNavigateToNewPage with |params.method| and |params.page_state| coming from a fake/made-up error page which incorrectly says to use GET rather than POST).

NOTES:
- This issue is similar to  issue 860807  (where FNE's method was incorrectly set in NCI::RendererDidNavigateToExistingPage rather than in NCI::RendererDidNavigateToNewPage)
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b

commit 98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Thu Aug 23 18:14:31 2018

Preserve request properties of failed navigations (e.g. POST data).

A failed navigation commits a fake data: page (see
RenderFrameImpl::LoadNavigationErrorPageInternal).  This commit should
preserve most aspects of the original request which failed.  This
includes the http method and POST data.

This CL makes sure that the |failed_request| in
RenderFrameImpl::CommitFailedNavigation includes POST data (this is
fixed by making sure that CreateURLRequestForNavigation preserves the
POST data.

This CL also passes |failed_request| from
RenderFrameImpl::CommitFailedNavigation into
WebLocalFrameImpl::CommitDataNavigation - this ensures that the
|failed_request|'s properties are properly preserved.

This CL also makes sure that |replace| value calculated in
RenderFrameImpl::CommitFailedNavigation is correct not only for reloads,
but also for back/forward navigations.

Bug:  860807 ,  869117 
Change-Id: Id4e781379defddb26f4e684632d173c46a6f660f
Reviewed-on: https://chromium-review.googlesource.com/1153564
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Commit-Queue: Ɓukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585547}
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/content/public/test/url_loader_interceptor.cc
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/content/public/test/url_loader_interceptor.h
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/content/renderer/dom_serializer_browsertest.cc
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/content/renderer/render_frame_impl.h
[add] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/content/test/data/form_that_posts_to_echoall_nocache.html
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/net/test/embedded_test_server/default_handlers.cc
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/third_party/blink/public/web/web_local_frame.h
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
[modify] https://crrev.com/98ccf1c6b1a024e078b6c96cebb8ed52e63bff1b/third_party/blink/renderer/core/frame/web_local_frame_impl.h

Status: Fixed (was: Untriaged)
I've verified on Mac Canary (70.0.3535.4) that this is fixed now.

Sign in to add a comment