Skip to content

Commit 6e60826

Browse files
petermntpetermnt
authored
[webview_flutter_android] Fixes iframe navigation with onNavigationRequest (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#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#145208
1 parent d500725 commit 6e60826

File tree

8 files changed

+325
-10
lines changed

8 files changed

+325
-10
lines changed

‎packages/webview_flutter/webview_flutter_android/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 3.16.1
2+
3+
* Fixes iframe navigation being handled in the main frame when `NavigationDelegate.onNavigationRequest` is present.
4+
15
## 3.16.0
26

37
* Adds onReceivedHttpError WebViewClient callback to support

‎packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewClientHostApiImpl.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,10 @@ public void onReceivedError(
8888
public boolean shouldOverrideUrlLoading(
8989
@NonNull WebView view, @NonNull WebResourceRequest request) {
9090
flutterApi.requestLoading(this, view, request, reply -> {});
91-
return returnValueForShouldOverrideUrlLoading;
91+
92+
// The client is only allowed to stop navigations that target the main frame because
93+
// overridden URLs are passed to `loadUrl` and `loadUrl` cannot load a subframe.
94+
return request.isForMainFrame() && returnValueForShouldOverrideUrlLoading;
9295
}
9396

9497
// Legacy codepath for < 24; newer versions use the variant above.
@@ -187,7 +190,10 @@ public void onReceivedError(
187190
public boolean shouldOverrideUrlLoading(
188191
@NonNull WebView view, @NonNull WebResourceRequest request) {
189192
flutterApi.requestLoading(this, view, request, reply -> {});
190-
return returnValueForShouldOverrideUrlLoading;
193+
194+
// The client is only allowed to stop navigations that target the main frame because
195+
// overridden URLs are passed to `loadUrl` and `loadUrl` cannot load a subframe.
196+
return request.isForMainFrame() && returnValueForShouldOverrideUrlLoading;
191197
}
192198

193199
// Legacy codepath for < Lollipop; newer versions use the variant above.

‎packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientTest.java‎ renamed to ‎packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientCompatImplTest.java‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
package io.flutter.plugins.webviewflutter;
66

77
import static org.junit.Assert.assertEquals;
8+
import static org.junit.Assert.assertFalse;
9+
import static org.junit.Assert.assertTrue;
810
import static org.mockito.ArgumentMatchers.any;
911
import static org.mockito.ArgumentMatchers.eq;
1012
import static org.mockito.Mockito.mock;
@@ -28,15 +30,13 @@
2830
import org.mockito.junit.MockitoJUnit;
2931
import org.mockito.junit.MockitoRule;
3032

31-
public class WebViewClientTest {
33+
public class WebViewClientCompatImplTest {
3234
@Rule public MockitoRule mockitoRule = MockitoJUnit.rule();
3335

3436
@Mock public WebViewClientFlutterApiImpl mockFlutterApi;
3537

3638
@Mock public WebView mockWebView;
3739

38-
@Mock public WebViewClientCompatImpl mockWebViewClient;
39-
4040
InstanceManager instanceManager;
4141
WebViewClientHostApiImpl hostApiImpl;
4242
WebViewClientCompatImpl webViewClient;
@@ -51,7 +51,7 @@ public void setUp() {
5151
@NonNull
5252
public WebViewClient createWebViewClient(
5353
@NonNull WebViewClientFlutterApiImpl flutterApi) {
54-
webViewClient = (WebViewClientCompatImpl) super.createWebViewClient(flutterApi);
54+
webViewClient = new WebViewClientCompatImpl(flutterApi);
5555
return webViewClient;
5656
}
5757
};
@@ -93,6 +93,54 @@ public void urlLoading() {
9393
.urlLoading(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
9494
}
9595

96+
@Test
97+
public void urlLoadingForMainFrame() {
98+
webViewClient.setReturnValueForShouldOverrideUrlLoading(false);
99+
100+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
101+
when(mockRequest.isForMainFrame()).thenReturn(true);
102+
103+
assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
104+
verify(mockFlutterApi)
105+
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
106+
}
107+
108+
@Test
109+
public void urlLoadingForMainFrameWithOverride() {
110+
webViewClient.setReturnValueForShouldOverrideUrlLoading(true);
111+
112+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
113+
when(mockRequest.isForMainFrame()).thenReturn(true);
114+
115+
assertTrue(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
116+
verify(mockFlutterApi)
117+
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
118+
}
119+
120+
@Test
121+
public void urlLoadingNotForMainFrame() {
122+
webViewClient.setReturnValueForShouldOverrideUrlLoading(false);
123+
124+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
125+
when(mockRequest.isForMainFrame()).thenReturn(false);
126+
127+
assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
128+
verify(mockFlutterApi)
129+
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
130+
}
131+
132+
@Test
133+
public void urlLoadingNotForMainFrameWithOverride() {
134+
webViewClient.setReturnValueForShouldOverrideUrlLoading(true);
135+
136+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
137+
when(mockRequest.isForMainFrame()).thenReturn(false);
138+
139+
assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
140+
verify(mockFlutterApi)
141+
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
142+
}
143+
96144
@Test
97145
public void convertWebResourceRequestWithNullHeaders() {
98146
final Uri mockUri = mock(Uri.class);
@@ -111,6 +159,7 @@ public void convertWebResourceRequestWithNullHeaders() {
111159

112160
@Test
113161
public void setReturnValueForShouldOverrideUrlLoading() {
162+
WebViewClientHostApiImpl.WebViewClientCompatImpl mockWebViewClient = mock();
114163
final WebViewClientHostApiImpl webViewClientHostApi =
115164
new WebViewClientHostApiImpl(
116165
instanceManager,

‎packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientImplTest.java‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
package io.flutter.plugins.webviewflutter;
6+
7+
import static org.junit.Assert.assertEquals;
8+
import static org.junit.Assert.assertFalse;
9+
import static org.junit.Assert.assertTrue;
10+
import static org.mockito.ArgumentMatchers.any;
11+
import static org.mockito.ArgumentMatchers.eq;
12+
import static org.mockito.Mockito.mock;
13+
import static org.mockito.Mockito.verify;
14+
import static org.mockito.Mockito.when;
15+
16+
import android.net.Uri;
17+
import android.webkit.WebResourceRequest;
18+
import android.webkit.WebResourceResponse;
19+
import android.webkit.WebView;
20+
import android.webkit.WebViewClient;
21+
import androidx.annotation.NonNull;
22+
import io.flutter.plugins.webviewflutter.WebViewClientHostApiImpl.WebViewClientCreator;
23+
import java.util.HashMap;
24+
import org.junit.After;
25+
import org.junit.Before;
26+
import org.junit.Rule;
27+
import org.junit.Test;
28+
import org.mockito.Mock;
29+
import org.mockito.junit.MockitoJUnit;
30+
import org.mockito.junit.MockitoRule;
31+
32+
public class WebViewClientImplTest {
33+
@Rule public MockitoRule mockitoRule = MockitoJUnit.rule();
34+
35+
@Mock public WebViewClientFlutterApiImpl mockFlutterApi;
36+
37+
@Mock public WebView mockWebView;
38+
39+
InstanceManager instanceManager;
40+
WebViewClientHostApiImpl hostApiImpl;
41+
WebViewClientHostApiImpl.WebViewClientImpl webViewClient;
42+
43+
@Before
44+
public void setUp() {
45+
instanceManager = InstanceManager.create(identifier -> {});
46+
47+
final WebViewClientCreator webViewClientCreator =
48+
new WebViewClientCreator() {
49+
@Override
50+
@NonNull
51+
public WebViewClient createWebViewClient(
52+
@NonNull WebViewClientFlutterApiImpl flutterApi) {
53+
webViewClient = new WebViewClientHostApiImpl.WebViewClientImpl(flutterApi);
54+
return webViewClient;
55+
}
56+
};
57+
58+
hostApiImpl =
59+
new WebViewClientHostApiImpl(instanceManager, webViewClientCreator, mockFlutterApi);
60+
hostApiImpl.create(1L);
61+
}
62+
63+
@After
64+
public void tearDown() {
65+
instanceManager.stopFinalizationListener();
66+
}
67+
68+
@Test
69+
public void onPageStarted() {
70+
webViewClient.onPageStarted(mockWebView, "https://www.google.com", null);
71+
verify(mockFlutterApi)
72+
.onPageStarted(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
73+
}
74+
75+
@Test
76+
public void onReceivedError() {
77+
webViewClient.onReceivedError(mockWebView, 32, "description", "https://www.google.com");
78+
verify(mockFlutterApi)
79+
.onReceivedError(
80+
eq(webViewClient),
81+
eq(mockWebView),
82+
eq(32L),
83+
eq("description"),
84+
eq("https://www.google.com"),
85+
any());
86+
}
87+
88+
@Test
89+
public void urlLoading() {
90+
webViewClient.shouldOverrideUrlLoading(mockWebView, "https://www.google.com");
91+
verify(mockFlutterApi)
92+
.urlLoading(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
93+
}
94+
95+
@Test
96+
public void urlLoadingForMainFrame() {
97+
webViewClient.setReturnValueForShouldOverrideUrlLoading(false);
98+
99+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
100+
when(mockRequest.isForMainFrame()).thenReturn(true);
101+
102+
assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
103+
verify(mockFlutterApi)
104+
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
105+
}
106+
107+
@Test
108+
public void urlLoadingForMainFrameWithOverride() {
109+
webViewClient.setReturnValueForShouldOverrideUrlLoading(true);
110+
111+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
112+
when(mockRequest.isForMainFrame()).thenReturn(true);
113+
114+
assertTrue(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
115+
verify(mockFlutterApi)
116+
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
117+
}
118+
119+
@Test
120+
public void urlLoadingNotForMainFrame() {
121+
webViewClient.setReturnValueForShouldOverrideUrlLoading(false);
122+
123+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
124+
when(mockRequest.isForMainFrame()).thenReturn(false);
125+
126+
assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
127+
verify(mockFlutterApi)
128+
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
129+
}
130+
131+
@Test
132+
public void urlLoadingNotForMainFrameWithOverride() {
133+
webViewClient.setReturnValueForShouldOverrideUrlLoading(true);
134+
135+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
136+
when(mockRequest.isForMainFrame()).thenReturn(false);
137+
138+
assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
139+
verify(mockFlutterApi)
140+
.requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
141+
}
142+
143+
@Test
144+
public void convertWebResourceRequestWithNullHeaders() {
145+
final Uri mockUri = mock(Uri.class);
146+
when(mockUri.toString()).thenReturn("");
147+
148+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
149+
when(mockRequest.getMethod()).thenReturn("method");
150+
when(mockRequest.getUrl()).thenReturn(mockUri);
151+
when(mockRequest.isForMainFrame()).thenReturn(true);
152+
when(mockRequest.getRequestHeaders()).thenReturn(null);
153+
154+
final GeneratedAndroidWebView.WebResourceRequestData data =
155+
WebViewClientFlutterApiImpl.createWebResourceRequestData(mockRequest);
156+
assertEquals(data.getRequestHeaders(), new HashMap<String, String>());
157+
}
158+
159+
@Test
160+
public void doUpdateVisitedHistory() {
161+
webViewClient.doUpdateVisitedHistory(mockWebView, "https://www.google.com", true);
162+
verify(mockFlutterApi)
163+
.doUpdateVisitedHistory(
164+
eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), eq(true), any());
165+
}
166+
167+
@Test
168+
public void onReceivedHttpError() {
169+
final Uri mockUri = mock(Uri.class);
170+
when(mockUri.toString()).thenReturn("");
171+
172+
final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
173+
when(mockRequest.getMethod()).thenReturn("method");
174+
when(mockRequest.getUrl()).thenReturn(mockUri);
175+
when(mockRequest.isForMainFrame()).thenReturn(true);
176+
when(mockRequest.getRequestHeaders()).thenReturn(null);
177+
178+
final WebResourceResponse mockResponse = mock(WebResourceResponse.class);
179+
when(mockResponse.getStatusCode()).thenReturn(404);
180+
181+
webViewClient.onReceivedHttpError(mockWebView, mockRequest, mockResponse);
182+
verify(mockFlutterApi)
183+
.onReceivedHttpError(
184+
eq(webViewClient),
185+
eq(mockWebView),
186+
any(WebResourceRequest.class),
187+
any(WebResourceResponse.class),
188+
any());
189+
}
190+
}

‎packages/webview_flutter/webview_flutter_android/example/lib/main.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ const String kNavigationExamplePage = '''
2323
<head><title>Navigation Delegate Example</title></head>
2424
<body>
2525
<p>
26-
The navigation delegate is set to block navigation to the youtube website.
26+
The navigation delegate is set to block navigation to the pub.dev website.
2727
</p>
2828
<ul>
29-
<ul><a href="https://www.youtube.com/">https://www.youtube.com/</a></ul>
29+
<ul><a href="https://pub.dev/">https://pub.dev/</a></ul>
3030
<ul><a href="https://www.google.com/">https://www.google.com/</a></ul>
3131
</ul>
3232
</body>

‎packages/webview_flutter/webview_flutter_android/lib/src/android_webview_controller.dart‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1464,7 +1464,11 @@ class AndroidNavigationDelegate extends PlatformNavigationDelegate {
14641464
final LoadRequestCallback? onLoadRequest = _onLoadRequest;
14651465
final NavigationRequestCallback? onNavigationRequest = _onNavigationRequest;
14661466

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

‎packages/webview_flutter/webview_flutter_android/pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: webview_flutter_android
22
description: A Flutter plugin that provides a WebView widget on Android.
33
repository: https://github.com/flutter/packages/tree/main/packages/webview_flutter/webview_flutter_android
44
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+webview%22
5-
version: 3.16.0
5+
version: 3.16.1
66

77
environment:
88
sdk: ^3.1.0

0 commit comments

Comments
 (0)