Return error, not null, on bad URL. Fixes #6137. (#6141)

OpenRefine · Nov 6, 2023 · 8dcf89d · 8dcf89d
1 parent a64546c
commit 8dcf89d
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 14 deletions.
diff --git 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;
@@ -244,20 +241,13 @@ public String handleResponse(final ClassicHttpResponse response) throws IOExcept
     }
 
     public String getResponse(String urlString, Header[] headers, HttpClientResponseHandler<String> 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/...r/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperationTests.java b/...r/src/com/google/refine/operations/column/ColumnAdditionByFetchingURLsOperationTests.java
@@ -51,6 +51,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;
@@ -193,7 +194,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
@@ -217,8 +218,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)));
         }
     }