Skip to content

Commit

Permalink
fix: crash when calling non-reentrant function in loadURL (#40163)
Browse files Browse the repository at this point in the history
fix: crash when calling non-reentrant function in loadURL

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
  • Loading branch information
trop[bot] and codebytere committed Oct 10, 2023
1 parent 7985f82 commit 0da1d6d
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
17 changes: 15 additions & 2 deletions shell/browser/api/electron_api_web_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "components/security_state/content/content_utils.h"
#include "components/security_state/core/security_state.h"
#include "content/browser/renderer_host/frame_tree_node.h" // nogncheck
#include "content/browser/renderer_host/navigation_controller_impl.h" // nogncheck
#include "content/browser/renderer_host/render_frame_host_manager.h" // nogncheck
#include "content/browser/renderer_host/render_widget_host_impl.h" // nogncheck
#include "content/browser/renderer_host/render_widget_host_view_base.h" // nogncheck
Expand Down Expand Up @@ -2423,8 +2424,20 @@ void WebContents::LoadURL(const GURL& url,
params.transition_type = ui::PageTransitionFromInt(
ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR);
params.override_user_agent = content::NavigationController::UA_OVERRIDE_TRUE;
// Discard non-committed entries to ensure that we don't re-use a pending
// entry

// It's not safe to start a new navigation or otherwise discard the current
// one while the call that started it is still on the stack. See
// http://crbug.com/347742.
auto& ctrl_impl = static_cast<content::NavigationControllerImpl&>(
web_contents()->GetController());
if (ctrl_impl.in_navigate_to_pending_entry()) {
Emit("did-fail-load", static_cast<int>(net::ERR_FAILED),
net::ErrorToShortString(net::ERR_FAILED), url.possibly_invalid_spec(),
true);
return;
}

// Discard non-committed entries to ensure we don't re-use a pending entry.
web_contents()->GetController().DiscardNonCommittedEntries();
web_contents()->GetController().LoadURLWithParams(params);

Expand Down
13 changes: 13 additions & 0 deletions spec/api-web-contents-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,19 @@ describe('webContents module', () => {
}
});

it('fails if loadURL is called inside a non-reentrant critical section', (done) => {
w.webContents.once('did-fail-load', (_event, _errorCode, _errorDescription, validatedURL) => {
expect(validatedURL).to.contain('blank.html');
done();
});

w.webContents.once('did-start-loading', () => {
w.loadURL(`file://${fixturesPath}/pages/blank.html`);
});

w.loadURL('data:text/html,<h1>HELLO</h1>');
});

it('sets appropriate error information on rejection', async () => {
let err: any;
try {
Expand Down

0 comments on commit 0da1d6d

Please sign in to comment.