From e9762a256f91b9da0b1e307f307ba59233feafed Mon Sep 17 00:00:00 2001 From: Tom Morris Date: Mon, 6 Nov 2023 05:48:33 -0500 Subject: [PATCH] Return error, not null, on bad URL. Fixes #6137. (#6141) --- main/src/com/google/refine/util/HttpClient.java | 12 +----------- .../ColumnAdditionByFetchingURLsOperationTests.java | 7 ++++--- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/main/src/com/google/refine/util/HttpClient.java b/main/src/com/google/refine/util/HttpClient.java index 7f33a71a1baa..0cf6e12b3fa7 100644 --- a/main/src/com/google/refine/util/HttpClient.java +++ b/main/src/com/google/refine/util/HttpClient.java @@ -28,9 +28,6 @@ package com.google.refine.util; import java.io.IOException; -import java.net.MalformedURLException; -import java.net.URISyntaxException; -import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; @@ -202,20 +199,13 @@ public String handleResponse(final ClassicHttpResponse response) throws IOExcept } public String getResponse(String urlString, Header[] headers, HttpClientResponseHandler responseHandler) throws IOException { - try { - // Use of URL constructor below is purely to get additional error checking to mimic - // previous behavior for the tests. - new URL(urlString).toURI(); - } catch (IllegalArgumentException | MalformedURLException | URISyntaxException e) { - return null; - } HttpGet httpGet = new HttpGet(urlString); if (headers != null && headers.length > 0) { httpGet.setHeaders(headers); } - httpGet.setConfig(defaultRequestConfig); // FIXME: Redundant? already includes in client builder + httpGet.setConfig(defaultRequestConfig); // FIXME: Redundant? already included in client builder return httpClient.execute(httpGet, responseHandler); } diff --git a/main/tests/server/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperationTests.java b/main/tests/server/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperationTests.java index 2a50a5a20148..08e6ecc90c08 100644 --- a/main/tests/server/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperationTests.java +++ b/main/tests/server/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperationTests.java @@ -50,6 +50,7 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT import com.google.refine.RefineTest; import com.google.refine.browsing.EngineConfig; +import com.google.refine.expr.EvalError; import com.google.refine.expr.ExpressionUtils; import com.google.refine.model.AbstractOperation; import com.google.refine.model.Cell; @@ -192,7 +193,7 @@ public void testInvalidUrl() throws Exception { server.enqueue(new MockResponse()); Row row0 = new Row(2); - row0.setCell(0, new Cell("auinrestrsc", null)); // malformed -> null + row0.setCell(0, new Cell("auinrestrsc", null)); // malformed -> error project.rows.add(row0); Row row1 = new Row(2); row1.setCell(0, new Cell(url.toString(), null)); // fine @@ -216,8 +217,8 @@ public void testInvalidUrl() throws Exception { int newCol = project.columnModel.getColumnByName("junk").getCellIndex(); // Inspect rows - Assert.assertEquals(project.rows.get(0).getCellValue(newCol), null); - Assert.assertTrue(project.rows.get(1).getCellValue(newCol) != null); + Assert.assertTrue(project.rows.get(0).getCellValue(newCol) instanceof EvalError); + Assert.assertNotNull(project.rows.get(1).getCellValue(newCol)); Assert.assertTrue(ExpressionUtils.isError(project.rows.get(2).getCellValue(newCol))); } }