Issue checkstyle#13012: Use toASCIIString for all URIs

reftel · Apr 21, 2023 · 91cadd9 · 91cadd9
1 parent fc2bdfe
commit 91cadd9
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 14 deletions.
diff --git a/config/signatures.txt b/config/signatures.txt
@@ -3,3 +3,4 @@ com.puppycrawl.tools.checkstyle.DefaultConfiguration#getAttribute(java.lang.Stri
 com.puppycrawl.tools.checkstyle.DefaultConfiguration#addAttribute(java.lang.String,java.lang.String) @ Usage of deprecated API is forbidden. Please use DefaultConfiguration.addProperty(String name, String value) instead.
 com.puppycrawl.tools.checkstyle.api.AbstractCheck#log(int,int,java.lang.String,java.lang.Object[]) @ Use of this log method is forbidden, please use AbstractCheck#log(DetailAST ast, String key, Object... args)
 com.puppycrawl.tools.checkstyle.api.AbstractCheck#log(int,java.lang.String,java.lang.Object[]) @ Use of this log method is forbidden, please use AbstractCheck#log(DetailAST ast, String key, Object... args)
+java.net.URI#toString() @ This method can return garbage for non-ascii data, please use URI#toASCIIString()
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java
@@ -262,7 +262,7 @@ public static Configuration loadConfiguration(String config,
             throws CheckstyleException {
         // figure out if this is a File or a URL
         final URI uri = CommonUtil.getUriByFilename(config);
-        final InputSource source = new InputSource(uri.toString());
+        final InputSource source = new InputSource(uri.toASCIIString());
         return loadConfiguration(source, overridePropsResolver,
                 ignoredModulesOptions, threadModeSettings);
     }

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/header/AbstractHeaderCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/header/AbstractHeaderCheck.java
@@ -116,7 +116,7 @@ private void loadHeaderFile() throws CheckstyleException {
         }
         catch (final IOException ex) {
             throw new CheckstyleException(
-                    "unable to load header file " + headerFile, ex);
+                    "unable to load header file " + headerFile.toASCIIString(), ex);
         }
     }
 
@@ -205,7 +205,7 @@ public Set<String> getExternalResourceLocations() {
             result = Collections.emptySet();
         }
         else {
-            result = Collections.singleton(headerFile.toString());
+            result = Collections.singleton(headerFile.toASCIIString());
         }
 
         return result;

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheck.java
@@ -559,7 +559,7 @@ else if (currentImportControl != null) {
 
     @Override
     public Set<String> getExternalResourceLocations() {
-        return Collections.singleton(file.toString());
+        return Collections.singleton(file.toASCIIString());
     }
 
     /**
@@ -608,7 +608,7 @@ public void setFile(URI uri) {
                 file = uri;
             }
             catch (CheckstyleException ex) {
-                throw new IllegalArgumentException(UNABLE_TO_LOAD + uri, ex);
+                throw new IllegalArgumentException(UNABLE_TO_LOAD + uri.toASCIIString(), ex);
             }
         }
     }

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java
@@ -271,11 +271,11 @@ private static PkgImportControl load(InputSource source,
             return loader.getRoot();
         }
         catch (ParserConfigurationException | SAXException ex) {
-            throw new CheckstyleException("unable to parse " + uri
+            throw new CheckstyleException("unable to parse " + uri.toASCIIString()
                     + " - " + ex.getMessage(), ex);
         }
         catch (IOException ex) {
-            throw new CheckstyleException("unable to read " + uri, ex);
+            throw new CheckstyleException("unable to read " + uri.toASCIIString(), ex);
         }
     }
 
@@ -292,10 +292,10 @@ private static PkgImportControl loadUri(URI uri) throws CheckstyleException {
             return load(source, uri);
         }
         catch (MalformedURLException ex) {
-            throw new CheckstyleException("syntax error in url " + uri, ex);
+            throw new CheckstyleException("syntax error in url " + uri.toASCIIString(), ex);
         }
         catch (IOException ex) {
-            throw new CheckstyleException("unable to find " + uri, ex);
+            throw new CheckstyleException("unable to find " + uri.toASCIIString(), ex);
         }
     }
 

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionsLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionsLoader.java
@@ -220,7 +220,7 @@ public static FilterSet loadSuppressions(String filename)
             throws CheckstyleException {
         // figure out if this is a File or a URL
         final URI uri = CommonUtil.getUriByFilename(filename);
-        final InputSource source = new InputSource(uri.toString());
+        final InputSource source = new InputSource(uri.toASCIIString());
         return loadSuppressions(source, filename);
     }
 
@@ -249,7 +249,7 @@ public static Set<TreeWalkerFilter> loadXpathSuppressions(String filename)
             throws CheckstyleException {
         // figure out if this is a File or a URL
         final URI uri = CommonUtil.getUriByFilename(filename);
-        final InputSource source = new InputSource(uri.toString());
+        final InputSource source = new InputSource(uri.toASCIIString());
         return loadXpathSuppressions(source, filename);
     }
 

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java
@@ -204,15 +204,17 @@ public void testSetHeaderTwice() {
     @Test
     public void testIoExceptionWhenLoadingHeaderFile() throws Exception {
         final HeaderCheck check = new HeaderCheck();
-        check.setHeaderFile(new URI("test://bad"));
+        final String nonAscii = "\u00e6\u00f8\u00e5";
+        final URI uri = new URI("test://bad" + nonAscii);
+        check.setHeaderFile(uri);
 
         final ReflectiveOperationException ex = assertThrows(ReflectiveOperationException.class,
             () -> TestUtil.invokeMethod(check, "loadHeaderFile"));
         assertWithMessage("Invalid exception cause message")
             .that(ex)
                 .hasCauseThat()
                     .hasMessageThat()
-                    .startsWith("unable to load header file ");
+                    .isEqualTo("unable to load header file " + uri.toASCIIString());
     }
 
     @Test
@@ -297,7 +299,7 @@ public void testExternalResource() throws Exception {
             .isEqualTo(1);
         assertWithMessage("Invalid resource location")
             .that(results.iterator().next())
-            .isEqualTo(uri.toString());
+            .isEqualTo(uri.toASCIIString());
     }
 
     @Test

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheckTest.java
@@ -26,6 +26,9 @@
 
 import java.io.File;
 import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -425,6 +428,41 @@ public void testFileNameNoExtension() throws Exception {
                 getPath("InputImportControlFileNameNoExtension"), expected);
     }
 
+    @Test
+    public void testPathWithNonAsciiCharacters() throws Exception {
+        final Path dirWithNonAsciiCharacters = temporaryFolder.toPath()
+                .resolve("\\u00e6\\u00f8\\u00e5")
+                .resolve("com/puppycrawl/tools/checkstyle/checks/imports/importcontrol");
+        dirWithNonAsciiCharacters.toFile().mkdirs();
+        final Path configInDirWithNonAsciiCharacters = dirWithNonAsciiCharacters.resolve(
+                "InputImportControl8.java");
+        for (String file : List.of("InputImportControl8.java", "InputImportControlBroken.xml")) {
+            Files.copy(Paths.get(getPath(file)), dirWithNonAsciiCharacters.resolve(file));
+        }
+
+        try {
+            final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
+            verifyWithInlineConfigParser(configInDirWithNonAsciiCharacters.toString(), expected);
+            assertWithMessage("Test should fail if exception was not thrown").fail();
+        }
+        catch (CheckstyleException ex) {
+            final String message = getCheckstyleExceptionMessage(ex);
+            final String messageStart = "Unable to load ";
+
+            assertWithMessage("Invalid message, should start with: %s", messageStart)
+                    .that(message)
+                    .startsWith(messageStart);
+
+            // Strip file:/// etc., since the authority component may be skipped if empty
+            final String asciiUri = dirWithNonAsciiCharacters.toUri().toASCIIString()
+                    .replaceAll("file:/*", "");
+            assertWithMessage("Invalid message, should contain URI of file: %s", asciiUri)
+                    .that(message)
+                    .contains(asciiUri);
+
+        }
+    }
+
     /**
      * Returns String message of original exception that was thrown in
      * ImportControlCheck.setUrl or ImportControlCheck.setFile