Skip to content

Commit 8adb20e

Browse files
jonahgrahamjonahgraham
committed
Fixup warnings due to deprecated URL constructor (in WebKit)
Includes adding a handful of new tests for the code that this modifies to cover all the special cases. The new tests are Linux only, but they could be adapted to test these error conditions on other platforms. Needed to resolve new warnings due to update to Java21 #2824
1 parent 3e6e8d9 commit 8adb20e

File tree

2 files changed

+172
-20
lines changed

2 files changed

+172
-20
lines changed

‎bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,20 +2063,22 @@ public boolean setUrl (String url, String postData, String[] headers) {
20632063
* invalid URL string and try to fix it by prepending an appropriate protocol.
20642064
*/
20652065
try {
2066-
new URL(url);
2067-
} catch (MalformedURLException e) {
2068-
String testUrl = null;
2069-
if (url.charAt (0) == SEPARATOR_FILE) {
2070-
/* appears to be a local file */
2071-
testUrl = PROTOCOL_FILE + url;
2072-
} else {
2073-
testUrl = PROTOCOL_HTTP + url;
2074-
}
2075-
try {
2076-
new URL (testUrl);
2077-
url = testUrl; /* adding the protocol made the url valid */
2078-
} catch (MalformedURLException e2) {
2079-
/* adding the protocol did not make the url valid, so do nothing */
2066+
new URI(url).toURL();
2067+
} catch (URISyntaxException | IllegalArgumentException | MalformedURLException e) {
2068+
if (!url.isEmpty()) {
2069+
String testUrl = null;
2070+
if (url.charAt (0) == SEPARATOR_FILE) {
2071+
/* appears to be a local file */
2072+
testUrl = PROTOCOL_FILE + url;
2073+
} else {
2074+
testUrl = PROTOCOL_HTTP + url;
2075+
}
2076+
try {
2077+
new URI(testUrl).toURL();
2078+
url = testUrl; /* adding the protocol made the url valid */
2079+
} catch (URISyntaxException | IllegalArgumentException | MalformedURLException e2) {
2080+
/* adding the protocol did not make the url valid, so do nothing */
2081+
}
20802082
}
20812083
}
20822084

@@ -2137,7 +2139,7 @@ public boolean setUrl (String url, String postData, String[] headers) {
21372139
String mime_type = null;
21382140
String encoding_type = null;
21392141
try {
2140-
URL base = new URL(base_url);
2142+
URL base = new URI(base_url).toURL();
21412143
URLConnection url_conn = base.openConnection();
21422144
if (url_conn instanceof HttpURLConnection) {
21432145
HttpURLConnection conn = (HttpURLConnection) url_conn;
@@ -2201,8 +2203,12 @@ public boolean setUrl (String url, String postData, String[] headers) {
22012203
}
22022204
}
22032205
}
2206+
} else {
2207+
html = "Unsupported connection type: " + url_conn;
22042208
}
2205-
} catch (IOException e) { // MalformedURLException is an IOException also.
2209+
} catch (URISyntaxException | IllegalArgumentException | MalformedURLException e) {
2210+
html = "URL is invalid: " + e.getMessage();
2211+
} catch (IOException e) {
22062212
html = e.getMessage();
22072213
} finally {
22082214
if (html != null && lastRequest == w2_bug527738LastRequestCounter.get()) {

‎tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java‎

Lines changed: 150 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ public void test_LocationListener_LocationListener_ordered_changing () {
753753
}));
754754
shell.open();
755755
browser.setText("You should not see this message.");
756-
String url = getValidUrl();
756+
String url = getValidFileUrl();
757757
browser.setUrl(url);
758758
assertTrue(waitForPassCondition(() -> locations.size() == 2));
759759
assertTrue(locations.get(0).equals("about:blank") && locations.get(1).contains("testWebsiteWithTitle.html"));
@@ -762,8 +762,10 @@ public void test_LocationListener_LocationListener_ordered_changing () {
762762
@TempDir
763763
static Path tempFolder;
764764

765-
private String getValidUrl() {
766-
return SwtTestUtil.getPath("testWebsiteWithTitle.html", tempFolder).toUri().toString();
765+
private String getValidFileUrl() {
766+
String url = SwtTestUtil.getPath("testWebsiteWithTitle.html", tempFolder).toUri().toString();
767+
assertTrue(url.startsWith("file://"));
768+
return url;
767769
}
768770

769771
@Test
@@ -1183,7 +1185,27 @@ public void test_setUrl_local() {
11831185
assumeFalse(SwtTestUtil.isCocoa, "Test fails on Mac, see https://github.com/eclipse-platform/eclipse.platform.swt/issues/722");
11841186
String expectedTitle = "Website Title";
11851187
Runnable browserSetFunc = () -> {
1186-
String url = getValidUrl();
1188+
String url = getValidFileUrl();
1189+
testLogAppend("URL: " + url);
1190+
boolean opSuccess = browser.setUrl(url);
1191+
assertTrue(opSuccess);
1192+
};
1193+
validateTitleChanged(expectedTitle, browserSetFunc);
1194+
}
1195+
1196+
/**
1197+
* Verifies that an invalid URL (missing file://) will be converted to a valid
1198+
* URL with the file:// added.
1199+
*/
1200+
@Test
1201+
public void test_setUrl_invalid_url_local() {
1202+
assumeTrue(SwtTestUtil.isLinux, "Handling of invalid URLs is platform dependent");
1203+
String expectedTitle = "Website Title";
1204+
Runnable browserSetFunc = () -> {
1205+
String url = getValidFileUrl();
1206+
assertTrue(url.startsWith("file://"));
1207+
url = url.replaceFirst("^file://", "");
1208+
assertTrue(url.startsWith("/"));
11871209
testLogAppend("URL: " + url);
11881210
boolean opSuccess = browser.setUrl(url);
11891211
assertTrue(opSuccess);
@@ -1209,6 +1231,30 @@ public void test_setUrl_remote() throws IOException {
12091231
}
12101232
}
12111233

1234+
/**
1235+
* Verifies that an invalid URL (missing http://) will be converted to a valid
1236+
* URL with the http:// added.
1237+
*/
1238+
@Test
1239+
public void test_setUrl_invalid_url_remote() throws IOException {
1240+
assumeTrue(SwtTestUtil.isLinux, "Handling of invalid URLs is platform dependent");
1241+
1242+
try (var server = new EchoHttpServer()) {
1243+
1244+
String validUrl = server.getEchoUrl("test_setUrl_remote");
1245+
assertTrue(validUrl.startsWith("http://"));
1246+
String url = validUrl.replaceFirst("^http://", "");
1247+
1248+
String expectedTitle = "test_setUrl_remote";
1249+
Runnable browserSetFunc = () -> {
1250+
testLog.append("Setting Browser url to:" + url);
1251+
boolean opSuccess = browser.setUrl(url);
1252+
assertTrue(opSuccess);
1253+
};
1254+
validateTitleChanged(expectedTitle, browserSetFunc);
1255+
}
1256+
}
1257+
12121258
@Test
12131259
public void test_setUrl_remote_with_post() throws IOException {
12141260
try (var server = new EchoHttpServer()) {
@@ -1238,6 +1284,106 @@ public void test_setUrl_remote_with_post() throws IOException {
12381284
}
12391285
}
12401286

1287+
@Test
1288+
public void test_setUrl_post_invalid_url() {
1289+
assumeTrue(SwtTestUtil.isLinux, "Handling of invalid URLs is platform dependent");
1290+
// Purposefully invalid URL (Note that URLs that become valid with
1291+
// http:// or file:// prefixed will get converted to their valid URLs
1292+
String url = "";
1293+
String postData = "test_setUrl_post_invalid_url";
1294+
1295+
Runnable browserSetFunc = () -> {
1296+
testLog.append("Setting Browser url to:" + url);
1297+
boolean opSuccess = browser.setUrl(url, postData, null);
1298+
assertTrue(opSuccess);
1299+
};
1300+
1301+
final AtomicReference<Boolean> completed = new AtomicReference<>(false);
1302+
browser.addProgressListener(completedAdapter(event -> {
1303+
testLog.append("ProgressListener fired");
1304+
completed.set(true);
1305+
}));
1306+
browserSetFunc.run();
1307+
shell.open();
1308+
1309+
boolean hasFinished = waitForPassCondition(() -> completed.get().booleanValue());
1310+
assertTrue(hasFinished);
1311+
1312+
String lowerCase = browser.getText().toLowerCase();
1313+
assertTrue(lowerCase.contains("URL is invalid".toLowerCase()), "Browser getText was: " + browser.getText());
1314+
}
1315+
1316+
@Test
1317+
public void test_setUrl_post_connection_closes_prematurely() throws IOException {
1318+
assumeTrue(SwtTestUtil.isLinux, """
1319+
Handling POST in Linux is handled by SWT, so we need extra testing for the SWT code.
1320+
This test can be adapted to win32/cocoa, but that would be testing the third-party
1321+
browser component. Therefore (for now) this test only runs on Linux.
1322+
""");
1323+
try (var server = new EchoHttpServer() {
1324+
@Override
1325+
protected void handlePostEcho(HttpExchange exchange) {
1326+
// Immediately close the connection - this should generate a user
1327+
// visible error in the UI.
1328+
1329+
// For Webkit case where we handle post
1330+
exchange.close();
1331+
}
1332+
}) {
1333+
String url = server.postEchoUrl();
1334+
String postData = "test_setUrl_remote_with_post";
1335+
1336+
Runnable browserSetFunc = () -> {
1337+
testLog.append("Setting Browser url to:" + url);
1338+
boolean opSuccess = browser.setUrl(url, postData, null);
1339+
assertTrue(opSuccess);
1340+
};
1341+
1342+
final AtomicReference<Boolean> completed = new AtomicReference<>(false);
1343+
browser.addProgressListener(completedAdapter(event -> {
1344+
testLog.append("ProgressListener fired");
1345+
completed.set(true);
1346+
}));
1347+
browserSetFunc.run();
1348+
shell.open();
1349+
1350+
boolean hasFinished = waitForPassCondition(() -> completed.get().booleanValue());
1351+
assertTrue(hasFinished);
1352+
1353+
String lowerCase = browser.getText().toLowerCase();
1354+
assertTrue(lowerCase.contains("Unexpected end of file from server".toLowerCase()), "Browser getText was: " + browser.getText());
1355+
}
1356+
}
1357+
1358+
@Test
1359+
public void test_setUrl_post_file_url() {
1360+
assumeTrue(SwtTestUtil.isLinux, "Handling of invalid URLs is platform dependent");
1361+
// Purposefully invalid URL (Note that URLs that become valid with
1362+
// http:// or file:// prefixed will get converted to their valid URLs
1363+
String url = getValidFileUrl();
1364+
String postData = "test_setUrl_post_file_url";
1365+
1366+
Runnable browserSetFunc = () -> {
1367+
testLog.append("Setting Browser url to:" + url);
1368+
boolean opSuccess = browser.setUrl(url, postData, null);
1369+
assertTrue(opSuccess);
1370+
};
1371+
1372+
final AtomicReference<Boolean> completed = new AtomicReference<>(false);
1373+
browser.addProgressListener(completedAdapter(event -> {
1374+
testLog.append("ProgressListener fired");
1375+
completed.set(true);
1376+
}));
1377+
browserSetFunc.run();
1378+
shell.open();
1379+
1380+
boolean hasFinished = waitForPassCondition(() -> completed.get().booleanValue());
1381+
assertTrue(hasFinished);
1382+
1383+
String lowerCase = browser.getText().toLowerCase();
1384+
assertTrue(lowerCase.contains("Unsupported connection type".toLowerCase()), "Browser getText was: " + browser.getText());
1385+
}
1386+
12411387
private void validateTitleChanged(String expectedTitle, Runnable browserSetFunc) {
12421388
final AtomicReference<String> actualTitle = new AtomicReference<>("");
12431389
browser.addTitleListener(event -> {

0 commit comments

Comments
 (0)