Skip to content

Commit

Permalink
[webview_flutter_android] Fixes iframe navigation with `onNavigationR…
Browse files Browse the repository at this point in the history
…equest` (flutter#6335)

This PR reintroduces the check on `WebResourceRequest.isForMainFrame` in the `shouldOverrideUrlLoading` callback.
This check already existed in the implementation before the change to Pigeon, but was not taken over.  See https://github.com/flutter/packages/blob/418bef0d6f3100e7a4bafb86537285e6d2095159/packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/FlutterWebViewClient.java#L99

The reasoning was already already in comment there:
```
      // Since we cannot call loadUrl for a subframe, we currently only allow the delegate to stop
      // navigations that target the main frame, if the request is not for the main frame
      // we just return false to allow the navigation.
      //
      // For more details see: flutter/flutter#25329 (comment)
```
 
Currently when a `NavigationDeletage.onNavigationRequest` is set and a navigation request for an iframe happens it will automatically be blocked and handed to the Flutter `onNavigationRequest`.  
If the request gets a `NavigationDecision.navigate` it will be loaded into the  mainframe through a `loadUrl`.  
If the request gets a `NavigationDecision.prevent` it will never be executed.

This causes an issue when for example an iframe loads local data.
Example
```data:text/html,<script>onresize=function(){parent.postMessage(0,'*')}<\/script>```

An iframe is allowed to do this, but with the current implementation this data will be loaded in the main frame, leading to a white page. 

Issue: flutter/flutter#145208
  • Loading branch information
petermnt authored and TecHaxter committed May 22, 2024
1 parent 01dab88 commit bde660e
Show file tree
Hide file tree
Showing 8 changed files with 325 additions and 10 deletions.
4 changes: 4 additions & 0 deletions packages/webview_flutter/webview_flutter_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 3.16.1

* Fixes iframe navigation being handled in the main frame when `NavigationDelegate.onNavigationRequest` is present.

## 3.16.0

* Adds onReceivedHttpError WebViewClient callback to support
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ public void onReceivedError(
public boolean shouldOverrideUrlLoading(
@NonNull WebView view, @NonNull WebResourceRequest request) {
flutterApi.requestLoading(this, view, request, reply -> {});
return returnValueForShouldOverrideUrlLoading;

// The client is only allowed to stop navigations that target the main frame because
// overridden URLs are passed to `loadUrl` and `loadUrl` cannot load a subframe.
return request.isForMainFrame() && returnValueForShouldOverrideUrlLoading;
}

// Legacy codepath for < 24; newer versions use the variant above.
Expand Down Expand Up @@ -187,7 +190,10 @@ public void onReceivedError(
public boolean shouldOverrideUrlLoading(
@NonNull WebView view, @NonNull WebResourceRequest request) {
flutterApi.requestLoading(this, view, request, reply -> {});
return returnValueForShouldOverrideUrlLoading;

// The client is only allowed to stop navigations that target the main frame because
// overridden URLs are passed to `loadUrl` and `loadUrl` cannot load a subframe.
return request.isForMainFrame() && returnValueForShouldOverrideUrlLoading;
}

// Legacy codepath for < Lollipop; newer versions use the variant above.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package io.flutter.plugins.webviewflutter;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
Expand All @@ -28,15 +30,13 @@
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;

public class WebViewClientTest {
public class WebViewClientCompatImplTest {
@Rule public MockitoRule mockitoRule = MockitoJUnit.rule();

@Mock public WebViewClientFlutterApiImpl mockFlutterApi;

@Mock public WebView mockWebView;

@Mock public WebViewClientCompatImpl mockWebViewClient;

InstanceManager instanceManager;
WebViewClientHostApiImpl hostApiImpl;
WebViewClientCompatImpl webViewClient;
Expand All @@ -51,7 +51,7 @@ public void setUp() {
@NonNull
public WebViewClient createWebViewClient(
@NonNull WebViewClientFlutterApiImpl flutterApi) {
webViewClient = (WebViewClientCompatImpl) super.createWebViewClient(flutterApi);
webViewClient = new WebViewClientCompatImpl(flutterApi);
return webViewClient;
}
};
Expand Down Expand Up @@ -93,6 +93,54 @@ public void urlLoading() {
.urlLoading(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
}

@Test
public void urlLoadingForMainFrame() {
webViewClient.setReturnValueForShouldOverrideUrlLoading(false);

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.isForMainFrame()).thenReturn(true);

assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
verify(mockFlutterApi)
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
}

@Test
public void urlLoadingForMainFrameWithOverride() {
webViewClient.setReturnValueForShouldOverrideUrlLoading(true);

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.isForMainFrame()).thenReturn(true);

assertTrue(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
verify(mockFlutterApi)
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
}

@Test
public void urlLoadingNotForMainFrame() {
webViewClient.setReturnValueForShouldOverrideUrlLoading(false);

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.isForMainFrame()).thenReturn(false);

assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
verify(mockFlutterApi)
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
}

@Test
public void urlLoadingNotForMainFrameWithOverride() {
webViewClient.setReturnValueForShouldOverrideUrlLoading(true);

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.isForMainFrame()).thenReturn(false);

assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
verify(mockFlutterApi)
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
}

@Test
public void convertWebResourceRequestWithNullHeaders() {
final Uri mockUri = mock(Uri.class);
Expand All @@ -111,6 +159,7 @@ public void convertWebResourceRequestWithNullHeaders() {

@Test
public void setReturnValueForShouldOverrideUrlLoading() {
WebViewClientHostApiImpl.WebViewClientCompatImpl mockWebViewClient = mock();
final WebViewClientHostApiImpl webViewClientHostApi =
new WebViewClientHostApiImpl(
instanceManager,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package io.flutter.plugins.webviewflutter;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import android.net.Uri;
import android.webkit.WebResourceRequest;
import android.webkit.WebResourceResponse;
import android.webkit.WebView;
import android.webkit.WebViewClient;
import androidx.annotation.NonNull;
import io.flutter.plugins.webviewflutter.WebViewClientHostApiImpl.WebViewClientCreator;
import java.util.HashMap;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;

public class WebViewClientImplTest {
@Rule public MockitoRule mockitoRule = MockitoJUnit.rule();

@Mock public WebViewClientFlutterApiImpl mockFlutterApi;

@Mock public WebView mockWebView;

InstanceManager instanceManager;
WebViewClientHostApiImpl hostApiImpl;
WebViewClientHostApiImpl.WebViewClientImpl webViewClient;

@Before
public void setUp() {
instanceManager = InstanceManager.create(identifier -> {});

final WebViewClientCreator webViewClientCreator =
new WebViewClientCreator() {
@Override
@NonNull
public WebViewClient createWebViewClient(
@NonNull WebViewClientFlutterApiImpl flutterApi) {
webViewClient = new WebViewClientHostApiImpl.WebViewClientImpl(flutterApi);
return webViewClient;
}
};

hostApiImpl =
new WebViewClientHostApiImpl(instanceManager, webViewClientCreator, mockFlutterApi);
hostApiImpl.create(1L);
}

@After
public void tearDown() {
instanceManager.stopFinalizationListener();
}

@Test
public void onPageStarted() {
webViewClient.onPageStarted(mockWebView, "https://www.google.com", null);
verify(mockFlutterApi)
.onPageStarted(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
}

@Test
public void onReceivedError() {
webViewClient.onReceivedError(mockWebView, 32, "description", "https://www.google.com");
verify(mockFlutterApi)
.onReceivedError(
eq(webViewClient),
eq(mockWebView),
eq(32L),
eq("description"),
eq("https://www.google.com"),
any());
}

@Test
public void urlLoading() {
webViewClient.shouldOverrideUrlLoading(mockWebView, "https://www.google.com");
verify(mockFlutterApi)
.urlLoading(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
}

@Test
public void urlLoadingForMainFrame() {
webViewClient.setReturnValueForShouldOverrideUrlLoading(false);

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.isForMainFrame()).thenReturn(true);

assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
verify(mockFlutterApi)
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
}

@Test
public void urlLoadingForMainFrameWithOverride() {
webViewClient.setReturnValueForShouldOverrideUrlLoading(true);

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.isForMainFrame()).thenReturn(true);

assertTrue(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
verify(mockFlutterApi)
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
}

@Test
public void urlLoadingNotForMainFrame() {
webViewClient.setReturnValueForShouldOverrideUrlLoading(false);

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.isForMainFrame()).thenReturn(false);

assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
verify(mockFlutterApi)
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
}

@Test
public void urlLoadingNotForMainFrameWithOverride() {
webViewClient.setReturnValueForShouldOverrideUrlLoading(true);

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.isForMainFrame()).thenReturn(false);

assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
verify(mockFlutterApi)
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
}

@Test
public void convertWebResourceRequestWithNullHeaders() {
final Uri mockUri = mock(Uri.class);
when(mockUri.toString()).thenReturn("");

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.getMethod()).thenReturn("method");
when(mockRequest.getUrl()).thenReturn(mockUri);
when(mockRequest.isForMainFrame()).thenReturn(true);
when(mockRequest.getRequestHeaders()).thenReturn(null);

final GeneratedAndroidWebView.WebResourceRequestData data =
WebViewClientFlutterApiImpl.createWebResourceRequestData(mockRequest);
assertEquals(data.getRequestHeaders(), new HashMap<String, String>());
}

@Test
public void doUpdateVisitedHistory() {
webViewClient.doUpdateVisitedHistory(mockWebView, "https://www.google.com", true);
verify(mockFlutterApi)
.doUpdateVisitedHistory(
eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), eq(true), any());
}

@Test
public void onReceivedHttpError() {
final Uri mockUri = mock(Uri.class);
when(mockUri.toString()).thenReturn("");

final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
when(mockRequest.getMethod()).thenReturn("method");
when(mockRequest.getUrl()).thenReturn(mockUri);
when(mockRequest.isForMainFrame()).thenReturn(true);
when(mockRequest.getRequestHeaders()).thenReturn(null);

final WebResourceResponse mockResponse = mock(WebResourceResponse.class);
when(mockResponse.getStatusCode()).thenReturn(404);

webViewClient.onReceivedHttpError(mockWebView, mockRequest, mockResponse);
verify(mockFlutterApi)
.onReceivedHttpError(
eq(webViewClient),
eq(mockWebView),
any(WebResourceRequest.class),
any(WebResourceResponse.class),
any());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ const String kNavigationExamplePage = '''
<head><title>Navigation Delegate Example</title></head>
<body>
<p>
The navigation delegate is set to block navigation to the youtube website.
The navigation delegate is set to block navigation to the pub.dev website.
</p>
<ul>
<ul><a href="https://www.youtube.com/">https://www.youtube.com/</a></ul>
<ul><a href="https://pub.dev/">https://pub.dev/</a></ul>
<ul><a href="https://www.google.com/">https://www.google.com/</a></ul>
</ul>
</body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1464,7 +1464,11 @@ class AndroidNavigationDelegate extends PlatformNavigationDelegate {
final LoadRequestCallback? onLoadRequest = _onLoadRequest;
final NavigationRequestCallback? onNavigationRequest = _onNavigationRequest;

if (onNavigationRequest == null || onLoadRequest == null) {
// The client is only allowed to stop navigations that target the main frame because
// overridden URLs are passed to `loadUrl` and `loadUrl` cannot load a subframe.
if (!isForMainFrame ||
onNavigationRequest == null ||
onLoadRequest == null) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: webview_flutter_android
description: A Flutter plugin that provides a WebView widget on Android.
repository: https://github.com/flutter/packages/tree/main/packages/webview_flutter/webview_flutter_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+webview%22
version: 3.16.0
version: 3.16.1

environment:
sdk: ^3.1.0
Expand Down
Loading

0 comments on commit bde660e

Please sign in to comment.