SONAR-9199 InputFile::content() and InputFile::inputStream() should filter BOM

plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/HasTagSensor.java

@@ -19,8 +19,9 @@
  */
 package org.sonar.xoo.rule;
 
+import java.io.BufferedReader;
 import java.io.IOException;
-import java.nio.file.Files;
+import java.io.InputStreamReader;
 import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.rule.ActiveRules;
@@ -58,20 +59,23 @@ public class HasTagSensor extends AbstractXooRuleSensor {
     }
     try {
       int[] lineCounter = {1};
-      Files.lines(inputFile.path(), inputFile.charset()).forEachOrdered(lineStr -> {
-        int startIndex = -1;
-        while ((startIndex = lineStr.indexOf(tag, startIndex + 1)) != -1) {
-          NewIssue newIssue = context.newIssue();
-          newIssue
-            .forRule(ruleKey)
-            .gap(context.settings().getDouble(EFFORT_TO_FIX_PROPERTY))
-            .at(newIssue.newLocation()
-              .on(inputFile)
-              .at(inputFile.newRange(lineCounter[0], startIndex, lineCounter[0], startIndex + tag.length())))
-            .save();
-        }
-        lineCounter[0]++;
-      });
+      try (InputStreamReader isr = new InputStreamReader(inputFile.inputStream(), inputFile.charset());
+        BufferedReader reader = new BufferedReader(isr)) {
+        reader.lines().forEachOrdered(lineStr -> {
+          int startIndex = -1;
+          while ((startIndex = lineStr.indexOf(tag, startIndex + 1)) != -1) {
+            NewIssue newIssue = context.newIssue();
+            newIssue
+              .forRule(ruleKey)
+              .gap(context.settings().getDouble(EFFORT_TO_FIX_PROPERTY))
+              .at(newIssue.newLocation()
+                .on(inputFile)
+                .at(inputFile.newRange(lineCounter[0], startIndex, lineCounter[0], startIndex + tag.length())))
+              .save();
+          }
+          lineCounter[0]++;
+        });
+      }
     } catch (IOException e) {
       throw new IllegalStateException("Fail to process " + inputFile, e);
     }

sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/IndexedFile.java

@@ -22,15 +22,9 @@ package org.sonar.api.batch.fs;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.nio.file.Files;
 import java.nio.file.Path;
-
 import javax.annotation.CheckForNull;
 
-import org.sonar.api.batch.fs.FileSystem;
-import org.sonar.api.batch.fs.InputFile;
-import org.sonar.api.batch.fs.InputPath;
-
 /**
  * Represents the indexed view of an {@link InputFile}. Accessing any of data exposed here won't trigger the expensive generation of 
  * metadata for the {@link InputFile}.
@@ -94,10 +88,7 @@ public interface IndexedFile extends InputPath {
   /**
    * Creates a stream of the file's contents. Depending on the runtime context, the source might be a file in a physical or virtual filesystem.
    * Typically, it won't be buffered. <b>The stream must be closed by the caller</b>.
-   * Note that there is a default implementation.
    * @since 6.2
    */
-  default InputStream inputStream() throws IOException {
-    return Files.newInputStream(path());
-  }
+  InputStream inputStream() throws IOException;
 }

sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/InputFile.java

@@ -23,7 +23,6 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.Charset;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import javax.annotation.CheckForNull;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
@@ -112,22 +111,18 @@ public interface InputFile extends IndexedFile {
   /**
    * Creates a stream of the file's contents. Depending on the runtime context, the source might be a file in a physical or virtual filesystem.
    * Typically, it won't be buffered. <b>The stream must be closed by the caller</b>.
-   * Note that there is a default implementation.
+   * Since 6.4 BOM is automatically filtered out.
    * @since 6.2
    */
   @Override
-  default InputStream inputStream() throws IOException {
-    return Files.newInputStream(path());
-  }
+  InputStream inputStream() throws IOException;
 
   /**
    * Fetches the entire contents of the file, decoding with the {@link #charset}.
-   * Note that there is a default implementation.
+   * Since 6.4 BOM is automatically filtered out.
    * @since 6.2
    */
-  default String contents() throws IOException {
-    return new String(Files.readAllBytes(path()), charset());
-  }
+  String contents() throws IOException;
 
   /**
    * Status regarding previous analysis

sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultIndexedFile.java

@@ -20,11 +20,12 @@
 package org.sonar.api.batch.fs.internal;
 
 import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
 import java.nio.file.Path;
-
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-
 import org.sonar.api.batch.fs.IndexedFile;
 import org.sonar.api.batch.fs.InputFile.Type;
 import org.sonar.api.utils.PathUtils;
@@ -83,6 +84,11 @@ public class DefaultIndexedFile extends DefaultInputComponent implements Indexed
     return moduleBaseDir.resolve(relativePath);
   }
 
+  @Override
+  public InputStream inputStream() throws IOException {
+    return Files.newInputStream(path());
+  }
+
   @CheckForNull
   @Override
   public String language() {

sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/internal/DefaultInputFile.java

@@ -20,8 +20,8 @@
 package org.sonar.api.batch.fs.internal;
 
 import com.google.common.base.Preconditions;
-
 import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -30,10 +30,10 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.function.Consumer;
-
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-
+import org.apache.commons.io.ByteOrderMark;
+import org.apache.commons.io.input.BOMInputStream;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.TextPointer;
 import org.sonar.api.batch.fs.TextRange;
@@ -43,6 +43,9 @@ import org.sonar.api.batch.fs.TextRange;
  * To create {@link InputFile} in tests, use {@link TestInputFileBuilder}.
  */
 public class DefaultInputFile extends DefaultInputComponent implements InputFile {
+
+  private static final int DEFAULT_BUFFER_SIZE = 1024 * 4;
+
   private final DefaultIndexedFile indexedFile;
   private final Consumer<DefaultInputFile> metadataGenerator;
   private Status status;
@@ -55,6 +58,7 @@ public class DefaultInputFile extends DefaultInputComponent implements InputFile
     this(indexedFile, metadataGenerator, null);
   }
 
+  // For testing
   public DefaultInputFile(DefaultIndexedFile indexedFile, Consumer<DefaultInputFile> metadataGenerator, @Nullable String contents) {
     super(indexedFile.batchId());
     this.indexedFile = indexedFile;
@@ -72,12 +76,24 @@ public class DefaultInputFile extends DefaultInputComponent implements InputFile
 
   @Override
   public InputStream inputStream() throws IOException {
-    return contents != null ? new ByteArrayInputStream(contents.getBytes(charset())) : Files.newInputStream(path());
+    return contents != null ? new ByteArrayInputStream(contents.getBytes(charset())) : new BOMInputStream(Files.newInputStream(path()),
+      ByteOrderMark.UTF_8, ByteOrderMark.UTF_16LE, ByteOrderMark.UTF_16BE, ByteOrderMark.UTF_32LE, ByteOrderMark.UTF_32BE);
   }
 
   @Override
   public String contents() throws IOException {
-    return contents != null ? contents : new String(Files.readAllBytes(path()), charset());
+    if (contents != null) {
+      return contents;
+    } else {
+      ByteArrayOutputStream result = new ByteArrayOutputStream();
+      byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
+      int length;
+      InputStream inputStream = inputStream();
+      while ((length = inputStream.read(buffer)) != -1) {
+        result.write(buffer, 0, length);
+      }
+      return result.toString(charset().name());
+    }
   }
 
   /**

sonar-plugin-api/src/test/java/org/sonar/api/batch/fs/internal/DefaultInputFileTest.java

@@ -20,7 +20,9 @@
 package org.sonar.api.batch.fs.internal;
 
 import java.io.BufferedReader;
+import java.io.BufferedWriter;
 import java.io.File;
+import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -29,8 +31,8 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
 import java.util.stream.Collectors;
-
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -72,7 +74,11 @@ public class DefaultInputFileTest {
     Path baseDir = temp.newFolder().toPath();
     Path testFile = baseDir.resolve("src").resolve("Foo.php");
     Files.createDirectories(testFile.getParent());
-    Files.write(testFile, "test string".getBytes(StandardCharsets.UTF_8));
+    String content = "test é string";
+    Files.write(testFile, content.getBytes(StandardCharsets.ISO_8859_1));
+
+    assertThat(Files.readAllLines(testFile, StandardCharsets.ISO_8859_1).get(0)).hasSize(content.length());
+
     Metadata metadata = new Metadata(42, 30, "", new int[0], 0);
 
     DefaultInputFile inputFile = new DefaultInputFile(new DefaultIndexedFile("ABCDE", baseDir, "src/Foo.php", InputFile.Type.TEST, 0)
@@ -80,10 +86,38 @@ public class DefaultInputFileTest {
         .setStatus(InputFile.Status.ADDED)
         .setCharset(StandardCharsets.ISO_8859_1);
 
-    assertThat(inputFile.contents()).isEqualTo("test string");
+    assertThat(inputFile.contents()).isEqualTo(content);
+    try (InputStream inputStream = inputFile.inputStream()) {
+      String result = new BufferedReader(new InputStreamReader(inputStream, inputFile.charset())).lines().collect(Collectors.joining());
+      assertThat(result).isEqualTo(content);
+    }
+
+  }
+
+  @Test
+  public void test_content_exclude_bom() throws IOException {
+    Path baseDir = temp.newFolder().toPath();
+    Path testFile = baseDir.resolve("src").resolve("Foo.php");
+    Files.createDirectories(testFile.getParent());
+    try (BufferedWriter out = new BufferedWriter(new FileWriter(testFile.toFile()))) {
+      out.write('\ufeff');
+    }
+    String content = "test é string €";
+    Files.write(testFile, content.getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND);
+
+    assertThat(Files.readAllLines(testFile, StandardCharsets.UTF_8).get(0)).hasSize(content.length() + 1);
+
+    Metadata metadata = new Metadata(42, 30, "", new int[0], 0);
+
+    DefaultInputFile inputFile = new DefaultInputFile(new DefaultIndexedFile("ABCDE", baseDir, "src/Foo.php", InputFile.Type.TEST, 0)
+      .setLanguage("php"), f -> f.setMetadata(metadata))
+        .setStatus(InputFile.Status.ADDED)
+        .setCharset(StandardCharsets.UTF_8);
+
+    assertThat(inputFile.contents()).isEqualTo(content);
     try (InputStream inputStream = inputFile.inputStream()) {
-      String result = new BufferedReader(new InputStreamReader(inputStream)).lines().collect(Collectors.joining());
-      assertThat(result).isEqualTo("test string");
+      String result = new BufferedReader(new InputStreamReader(inputStream, inputFile.charset())).lines().collect(Collectors.joining());
+      assertThat(result).isEqualTo(content);
     }
 
   }