diff --git a/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java b/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java index 9619f0defce..4243495a64a 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java @@ -185,8 +185,8 @@ private static record MouseRelatedDomEvent(String eventType, boolean altKey, boo HttpCookie parser = HttpCookie.parse(CookieValue).get(0); URL origin; try { - origin = new URL(CookieUrl); - } catch (MalformedURLException e) { + origin = new URI(CookieUrl).toURL(); + } catch (URISyntaxException | IllegalArgumentException | MalformedURLException e) { return; } if (parser.getDomain() == null) { diff --git a/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/IE.java b/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/IE.java index a733a0600db..0eca21a3287 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/IE.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/IE.java @@ -717,8 +717,8 @@ else if (IEVersion >= 8) { boolean isPDF = false; String path = null; try { - path = new URL(url3).getPath(); - } catch (MalformedURLException e) { + path = new URI(url3).toURL().getPath(); + } catch (URISyntaxException | IllegalArgumentException | MalformedURLException e) { } if (path != null) { int extensionIndex = path.lastIndexOf('.'); diff --git a/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java b/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java index 7846cdbfba2..d3ad33a5708 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java +++ b/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java @@ -2063,20 +2063,22 @@ public boolean setUrl (String url, String postData, String[] headers) { * invalid URL string and try to fix it by prepending an appropriate protocol. */ try { - new URL(url); - } catch (MalformedURLException e) { - String testUrl = null; - if (url.charAt (0) == SEPARATOR_FILE) { - /* appears to be a local file */ - testUrl = PROTOCOL_FILE + url; - } else { - testUrl = PROTOCOL_HTTP + url; - } - try { - new URL (testUrl); - url = testUrl; /* adding the protocol made the url valid */ - } catch (MalformedURLException e2) { - /* adding the protocol did not make the url valid, so do nothing */ + new URI(url).toURL(); + } catch (URISyntaxException | IllegalArgumentException | MalformedURLException e) { + if (!url.isEmpty()) { + String testUrl = null; + if (url.charAt (0) == SEPARATOR_FILE) { + /* appears to be a local file */ + testUrl = PROTOCOL_FILE + url; + } else { + testUrl = PROTOCOL_HTTP + url; + } + try { + new URI(testUrl).toURL(); + url = testUrl; /* adding the protocol made the url valid */ + } catch (URISyntaxException | IllegalArgumentException | MalformedURLException e2) { + /* adding the protocol did not make the url valid, so do nothing */ + } } } @@ -2137,7 +2139,7 @@ public boolean setUrl (String url, String postData, String[] headers) { String mime_type = null; String encoding_type = null; try { - URL base = new URL(base_url); + URL base = new URI(base_url).toURL(); URLConnection url_conn = base.openConnection(); if (url_conn instanceof HttpURLConnection) { HttpURLConnection conn = (HttpURLConnection) url_conn; @@ -2201,8 +2203,12 @@ public boolean setUrl (String url, String postData, String[] headers) { } } } + } else { + html = "Unsupported connection type: " + url_conn; } - } catch (IOException e) { // MalformedURLException is an IOException also. + } catch (URISyntaxException | IllegalArgumentException | MalformedURLException e) { + html = "URL is invalid: " + e.getMessage(); + } catch (IOException e) { html = e.getMessage(); } finally { if (html != null && lastRequest == w2_bug527738LastRequestCounter.get()) { diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java index 0b8d191043a..c3bf2aaebcc 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java @@ -753,7 +753,7 @@ public void test_LocationListener_LocationListener_ordered_changing () { })); shell.open(); browser.setText("You should not see this message."); - String url = getValidUrl(); + String url = getValidFileUrl(); browser.setUrl(url); assertTrue(waitForPassCondition(() -> locations.size() == 2)); assertTrue(locations.get(0).equals("about:blank") && locations.get(1).contains("testWebsiteWithTitle.html")); @@ -762,8 +762,10 @@ public void test_LocationListener_LocationListener_ordered_changing () { @TempDir static Path tempFolder; -private String getValidUrl() { - return SwtTestUtil.getPath("testWebsiteWithTitle.html", tempFolder).toUri().toString(); +private String getValidFileUrl() { + String url = SwtTestUtil.getPath("testWebsiteWithTitle.html", tempFolder).toUri().toString(); + assertTrue(url.startsWith("file://")); + return url; } @Test @@ -1183,7 +1185,27 @@ public void test_setUrl_local() { assumeFalse(SwtTestUtil.isCocoa, "Test fails on Mac, see https://github.com/eclipse-platform/eclipse.platform.swt/issues/722"); String expectedTitle = "Website Title"; Runnable browserSetFunc = () -> { - String url = getValidUrl(); + String url = getValidFileUrl(); + testLogAppend("URL: " + url); + boolean opSuccess = browser.setUrl(url); + assertTrue(opSuccess); + }; + validateTitleChanged(expectedTitle, browserSetFunc); +} + +/** + * Verifies that an invalid URL (missing file://) will be converted to a valid + * URL with the file:// added. + */ +@Test +public void test_setUrl_invalid_url_local() { + assumeTrue(SwtTestUtil.isLinux, "Handling of invalid URLs is platform dependent"); + String expectedTitle = "Website Title"; + Runnable browserSetFunc = () -> { + String url = getValidFileUrl(); + assertTrue(url.startsWith("file://")); + url = url.replaceFirst("^file://", ""); + assertTrue(url.startsWith("/")); testLogAppend("URL: " + url); boolean opSuccess = browser.setUrl(url); assertTrue(opSuccess); @@ -1209,6 +1231,30 @@ public void test_setUrl_remote() throws IOException { } } +/** + * Verifies that an invalid URL (missing http://) will be converted to a valid + * URL with the http:// added. + */ +@Test +public void test_setUrl_invalid_url_remote() throws IOException { + assumeTrue(SwtTestUtil.isLinux, "Handling of invalid URLs is platform dependent"); + + try (var server = new EchoHttpServer()) { + + String validUrl = server.getEchoUrl("test_setUrl_remote"); + assertTrue(validUrl.startsWith("http://")); + String url = validUrl.replaceFirst("^http://", ""); + + String expectedTitle = "test_setUrl_remote"; + Runnable browserSetFunc = () -> { + testLog.append("Setting Browser url to:" + url); + boolean opSuccess = browser.setUrl(url); + assertTrue(opSuccess); + }; + validateTitleChanged(expectedTitle, browserSetFunc); + } +} + @Test public void test_setUrl_remote_with_post() throws IOException { try (var server = new EchoHttpServer()) { @@ -1238,6 +1284,106 @@ public void test_setUrl_remote_with_post() throws IOException { } } +@Test +public void test_setUrl_post_invalid_url() { + assumeTrue(SwtTestUtil.isLinux, "Handling of invalid URLs is platform dependent"); + // Purposefully invalid URL (Note that URLs that become valid with + // http:// or file:// prefixed will get converted to their valid URLs + String url = ""; + String postData = "test_setUrl_post_invalid_url"; + + Runnable browserSetFunc = () -> { + testLog.append("Setting Browser url to:" + url); + boolean opSuccess = browser.setUrl(url, postData, null); + assertTrue(opSuccess); + }; + + final AtomicReference completed = new AtomicReference<>(false); + browser.addProgressListener(completedAdapter(event -> { + testLog.append("ProgressListener fired"); + completed.set(true); + })); + browserSetFunc.run(); + shell.open(); + + boolean hasFinished = waitForPassCondition(() -> completed.get().booleanValue()); + assertTrue(hasFinished); + + String lowerCase = browser.getText().toLowerCase(); + assertTrue(lowerCase.contains("URL is invalid".toLowerCase()), "Browser getText was: " + browser.getText()); +} + +@Test +public void test_setUrl_post_connection_closes_prematurely() throws IOException { + assumeTrue(SwtTestUtil.isLinux, """ + Handling POST in Linux is handled by SWT, so we need extra testing for the SWT code. + This test can be adapted to win32/cocoa, but that would be testing the third-party + browser component. Therefore (for now) this test only runs on Linux. + """); + try (var server = new EchoHttpServer() { + @Override + protected void handlePostEcho(HttpExchange exchange) { + // Immediately close the connection - this should generate a user + // visible error in the UI. + + // For Webkit case where we handle post + exchange.close(); + } + }) { + String url = server.postEchoUrl(); + String postData = "test_setUrl_remote_with_post"; + + Runnable browserSetFunc = () -> { + testLog.append("Setting Browser url to:" + url); + boolean opSuccess = browser.setUrl(url, postData, null); + assertTrue(opSuccess); + }; + + final AtomicReference completed = new AtomicReference<>(false); + browser.addProgressListener(completedAdapter(event -> { + testLog.append("ProgressListener fired"); + completed.set(true); + })); + browserSetFunc.run(); + shell.open(); + + boolean hasFinished = waitForPassCondition(() -> completed.get().booleanValue()); + assertTrue(hasFinished); + + String lowerCase = browser.getText().toLowerCase(); + assertTrue(lowerCase.contains("Unexpected end of file from server".toLowerCase()), "Browser getText was: " + browser.getText()); + } +} + +@Test +public void test_setUrl_post_file_url() { + assumeTrue(SwtTestUtil.isLinux, "Handling of invalid URLs is platform dependent"); + // Purposefully invalid URL (Note that URLs that become valid with + // http:// or file:// prefixed will get converted to their valid URLs + String url = getValidFileUrl(); + String postData = "test_setUrl_post_file_url"; + + Runnable browserSetFunc = () -> { + testLog.append("Setting Browser url to:" + url); + boolean opSuccess = browser.setUrl(url, postData, null); + assertTrue(opSuccess); + }; + + final AtomicReference completed = new AtomicReference<>(false); + browser.addProgressListener(completedAdapter(event -> { + testLog.append("ProgressListener fired"); + completed.set(true); + })); + browserSetFunc.run(); + shell.open(); + + boolean hasFinished = waitForPassCondition(() -> completed.get().booleanValue()); + assertTrue(hasFinished); + + String lowerCase = browser.getText().toLowerCase(); + assertTrue(lowerCase.contains("Unsupported connection type".toLowerCase()), "Browser getText was: " + browser.getText()); +} + private void validateTitleChanged(String expectedTitle, Runnable browserSetFunc) { final AtomicReference actualTitle = new AtomicReference<>(""); browser.addTitleListener(event -> {