From fb79430745b96bd2fb02b763b34713bf8071fce4 Mon Sep 17 00:00:00 2001 From: Abel Salgado Romero Date: Fri, 24 Dec 2021 14:56:29 +0100 Subject: [PATCH] Avoid crash when processing logs of files not under the source dir (#563) * Rename LogRecordHelper into LogRecordFormatter --- CHANGELOG.adoc | 1 + .../asciidoctor/maven/AsciidoctorMojo.java | 4 +- ...ordHelper.java => LogRecordFormatter.java} | 43 ++++++++++------ .../maven/log/LogRecordsProcessors.java | 6 +-- .../maven/site/AsciidoctorDoxiaParser.java | 4 +- ...rTest.java => LogRecordFormatterTest.java} | 51 +++++++++++++++---- 6 files changed, 77 insertions(+), 32 deletions(-) rename src/main/java/org/asciidoctor/maven/log/{LogRecordHelper.java => LogRecordFormatter.java} (53%) rename src/test/java/org/asciidoctor/maven/log/{LogRecordHelperTest.java => LogRecordFormatterTest.java} (63%) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index ebb2e8a7..733f25b8 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -21,6 +21,7 @@ Documentation:: Bug Fixes:: * Exclude dot files and folders from conversion (#555) + * Fix `StringIndexOutOfBoundsException` parsing log records when cursor file is above source directory (#563) Build / Infrastructure:: diff --git a/src/main/java/org/asciidoctor/maven/AsciidoctorMojo.java b/src/main/java/org/asciidoctor/maven/AsciidoctorMojo.java index 47de8269..303ff49a 100644 --- a/src/main/java/org/asciidoctor/maven/AsciidoctorMojo.java +++ b/src/main/java/org/asciidoctor/maven/AsciidoctorMojo.java @@ -19,7 +19,7 @@ import org.asciidoctor.maven.extensions.ExtensionRegistry; import org.asciidoctor.maven.io.AsciidoctorFileScanner; import org.asciidoctor.maven.log.LogHandler; -import org.asciidoctor.maven.log.LogRecordHelper; +import org.asciidoctor.maven.log.LogRecordFormatter; import org.asciidoctor.maven.log.LogRecordsProcessors; import org.asciidoctor.maven.log.MemoryLogHandler; import org.asciidoctor.maven.process.AsciidoctorHelper; @@ -236,7 +236,7 @@ public void processSources(List sourceFiles, ResourcesProcessor resourcesP // register LogHandler to capture asciidoctor messages final Boolean outputToConsole = logHandler.getOutputToConsole() == null ? Boolean.TRUE : logHandler.getOutputToConsole(); final MemoryLogHandler memoryLogHandler = new MemoryLogHandler(outputToConsole, sourceDir, - logRecord -> getLog().info(LogRecordHelper.format(logRecord, sourceDir))); + logRecord -> getLog().info(LogRecordFormatter.format(logRecord, sourceDir))); asciidoctor.registerLogHandler(memoryLogHandler); // disable default console output of AsciidoctorJ Logger.getLogger("asciidoctor").setUseParentHandlers(false); diff --git a/src/main/java/org/asciidoctor/maven/log/LogRecordHelper.java b/src/main/java/org/asciidoctor/maven/log/LogRecordFormatter.java similarity index 53% rename from src/main/java/org/asciidoctor/maven/log/LogRecordHelper.java rename to src/main/java/org/asciidoctor/maven/log/LogRecordFormatter.java index f62ea66d..895fc192 100644 --- a/src/main/java/org/asciidoctor/maven/log/LogRecordHelper.java +++ b/src/main/java/org/asciidoctor/maven/log/LogRecordFormatter.java @@ -4,15 +4,11 @@ import org.asciidoctor.log.LogRecord; import java.io.File; -import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; -/** - * Utility class to manage AsciidoctorJ LogRecords. - */ -public class LogRecordHelper { +public class LogRecordFormatter { private static final String MESSAGE_HEADER = "asciidoctor"; @@ -26,14 +22,17 @@ public class LogRecordHelper { */ public static String format(LogRecord logRecord, File sourceDirectory) { final Cursor cursor = logRecord.getCursor(); - final String relativePath = calculateFileRelativePath(cursor, sourceDirectory); + + String sourcePath = calculateFileRelativePath(cursor, sourceDirectory); + if (sourcePath == null && cursor != null) + sourcePath = cursor.getFile(); final List messageParts = new ArrayList<>(); messageParts.add(MESSAGE_HEADER); messageParts.add(logRecord.getSeverity().toString()); - if (relativePath != null) - messageParts.add(relativePath); + if (sourcePath != null) + messageParts.add(sourcePath); if (cursor != null && cursor.getLineNumber() > 0) messageParts.add("line " + cursor.getLineNumber()); @@ -43,18 +42,32 @@ public static String format(LogRecord logRecord, File sourceDirectory) { return messageParts.stream().collect(Collectors.joining(": ")); } + /** + * Attempts to obtain the source path in relative format for ease and security. + * + * @return relative path or null if it was not possible + */ private static String calculateFileRelativePath(Cursor cursor, File sourceDirectory) { try { - if (cursor != null && cursor.getFile() != null) { - return new File(cursor.getFile()) - .getCanonicalPath() - .substring(sourceDirectory.getCanonicalPath().length() + 1); + if (isValidFile(cursor)) { + final String sourceFile = new File(cursor.getFile()).getCanonicalPath(); + final String sourceDir = sourceDirectory.getCanonicalPath(); + + if (sourceFile.startsWith(sourceDir)) { + return sourceFile.substring(sourceDirectory.getCanonicalPath().length() + 1); + } } - } catch (IOException e) { - // use the absolute path as fail-safe - return cursor.getFile(); + } catch (Exception e) { + return null; } return null; } + private static boolean isValidFile(Cursor cursor) { + return cursor != null && cursor.getFile() != null && !isHttpSource(cursor.getFile()); + } + + private static boolean isHttpSource(String filePath) { + return filePath.startsWith("http://") || filePath.startsWith("https://"); + } } diff --git a/src/main/java/org/asciidoctor/maven/log/LogRecordsProcessors.java b/src/main/java/org/asciidoctor/maven/log/LogRecordsProcessors.java index 35ad005b..f8a3df58 100644 --- a/src/main/java/org/asciidoctor/maven/log/LogRecordsProcessors.java +++ b/src/main/java/org/asciidoctor/maven/log/LogRecordsProcessors.java @@ -30,7 +30,7 @@ public void processLogRecords(MemoryLogHandler memoryLogHandler) throws Exceptio final List records = memoryLogHandler.filter(severity, textToSearch); if (records.size() > 0) { for (LogRecord record : records) { - errorMessageConsumer.accept(LogRecordHelper.format(record, sourceDirectory)); + errorMessageConsumer.accept(LogRecordFormatter.format(record, sourceDirectory)); } throw new Exception(String.format("Found %s issue(s) matching severity %s or higher and text '%s'", records.size(), severity, textToSearch)); } @@ -39,7 +39,7 @@ public void processLogRecords(MemoryLogHandler memoryLogHandler) throws Exceptio final List records = memoryLogHandler.filter(severity); if (records.size() > 0) { for (LogRecord record : records) { - errorMessageConsumer.accept(LogRecordHelper.format(record, sourceDirectory)); + errorMessageConsumer.accept(LogRecordFormatter.format(record, sourceDirectory)); } throw new Exception(String.format("Found %s issue(s) of severity %s or higher during conversion", records.size(), severity)); } @@ -48,7 +48,7 @@ public void processLogRecords(MemoryLogHandler memoryLogHandler) throws Exceptio final List records = memoryLogHandler.filter(textToSearch); if (records.size() > 0) { for (LogRecord record : records) { - errorMessageConsumer.accept(LogRecordHelper.format(record, sourceDirectory)); + errorMessageConsumer.accept(LogRecordFormatter.format(record, sourceDirectory)); } throw new Exception(String.format("Found %s issue(s) containing '%s'", records.size(), textToSearch)); } diff --git a/src/main/java/org/asciidoctor/maven/site/AsciidoctorDoxiaParser.java b/src/main/java/org/asciidoctor/maven/site/AsciidoctorDoxiaParser.java index cd99b2ee..cb10f868 100644 --- a/src/main/java/org/asciidoctor/maven/site/AsciidoctorDoxiaParser.java +++ b/src/main/java/org/asciidoctor/maven/site/AsciidoctorDoxiaParser.java @@ -7,7 +7,7 @@ import org.apache.maven.project.MavenProject; import org.asciidoctor.*; import org.asciidoctor.maven.log.LogHandler; -import org.asciidoctor.maven.log.LogRecordHelper; +import org.asciidoctor.maven.log.LogRecordFormatter; import org.asciidoctor.maven.log.LogRecordsProcessors; import org.asciidoctor.maven.log.MemoryLogHandler; import org.codehaus.plexus.component.annotations.Component; @@ -91,7 +91,7 @@ public void parse(Reader reader, Sink sink) throws ParseException { private MemoryLogHandler asciidoctorLoggingSetup(Asciidoctor asciidoctor, LogHandler logHandler, File siteDirectory) { final MemoryLogHandler memoryLogHandler = new MemoryLogHandler(logHandler.getOutputToConsole(), siteDirectory, - logRecord -> getLog().info(LogRecordHelper.format(logRecord, siteDirectory))); + logRecord -> getLog().info(LogRecordFormatter.format(logRecord, siteDirectory))); asciidoctor.registerLogHandler(memoryLogHandler); // disable default console output of AsciidoctorJ Logger.getLogger("asciidoctor").setUseParentHandlers(false); diff --git a/src/test/java/org/asciidoctor/maven/log/LogRecordHelperTest.java b/src/test/java/org/asciidoctor/maven/log/LogRecordFormatterTest.java similarity index 63% rename from src/test/java/org/asciidoctor/maven/log/LogRecordHelperTest.java rename to src/test/java/org/asciidoctor/maven/log/LogRecordFormatterTest.java index 632b8b76..28921b70 100644 --- a/src/test/java/org/asciidoctor/maven/log/LogRecordHelperTest.java +++ b/src/test/java/org/asciidoctor/maven/log/LogRecordFormatterTest.java @@ -11,7 +11,7 @@ import static org.assertj.core.api.Assertions.assertThat; -public class LogRecordHelperTest { +public class LogRecordFormatterTest { @Test public void should_apply_full_format_logRecord_with_all_data() { @@ -20,7 +20,7 @@ public void should_apply_full_format_logRecord_with_all_data() { final LogRecord logRecord = new LogRecord(Severity.INFO, cursor, "a message"); final File sourceDir = getParentFile(); // when - String formattedLogRecord = LogRecordHelper.format(logRecord, sourceDir); + String formattedLogRecord = LogRecordFormatter.format(logRecord, sourceDir); // then assertThat(normalizePath(formattedLogRecord)).isEqualTo("asciidoctor: INFO: asciidoctor-maven-plugin/file.adoc: line 3: a message"); } @@ -30,7 +30,7 @@ public void should_apply_simple_format_when_cursor_is_null() { // given final LogRecord logRecord = new LogRecord(Severity.INFO, null, "a message"); // when - String formattedLogRecord = LogRecordHelper.format(logRecord, null); + String formattedLogRecord = LogRecordFormatter.format(logRecord, null); // then assertThat(normalizePath(formattedLogRecord)).isEqualTo("asciidoctor: INFO: a message"); } @@ -41,7 +41,7 @@ public void should_apply_simple_format_when_cursor_is_empty() { final Cursor cursor = new TestCursor(null, 0, null, null); final LogRecord logRecord = new LogRecord(Severity.INFO, cursor, "a message"); // when - String formattedLogRecord = LogRecordHelper.format(logRecord, null); + String formattedLogRecord = LogRecordFormatter.format(logRecord, null); // then assertThat(normalizePath(formattedLogRecord)).isEqualTo("asciidoctor: INFO: a message"); } @@ -53,27 +53,59 @@ public void should_format_full_logRecord_with_file_absolute_path_when_sourceDir_ final LogRecord logRecord = new LogRecord(Severity.INFO, cursor, "a message"); final File sourceDir = Mockito.mock(File.class); Mockito.when(sourceDir.getCanonicalPath()).thenThrow(new IOException()); - // when - String formattedLogRecord = LogRecordHelper.format(logRecord, sourceDir); + String formattedLogRecord = LogRecordFormatter.format(logRecord, sourceDir); // then assertThat(normalizePath(formattedLogRecord)).matches("asciidoctor: INFO: .*/asciidoctor-maven-plugin/file.adoc: line 3: a message"); } @Test - public void should_format_logRecords_with_empty_lineNumber_absolute_path_when_sourceDir_is_not_valid2() throws IOException { + public void should_format_logRecords_with_empty_lineNumber_absolute_path_when_sourceDir_is_not_valid() throws IOException { // given final Cursor cursor = new TestCursor(new File("file.adoc").getAbsolutePath(), 0, "path", "dir"); final LogRecord logRecord = new LogRecord(Severity.INFO, cursor, "a message"); final File sourceDir = Mockito.mock(File.class); Mockito.when(sourceDir.getCanonicalPath()).thenThrow(new IOException()); - // when - String formattedLogRecord = LogRecordHelper.format(logRecord, sourceDir); + String formattedLogRecord = LogRecordFormatter.format(logRecord, sourceDir); // then assertThat(normalizePath(formattedLogRecord)).matches("asciidoctor: INFO: .*/asciidoctor-maven-plugin/file.adoc: a message"); } + @Test + public void should_format_logRecords_when_source_is_not_under_sourceDir() { + // given + final Cursor cursor = new TestCursor(new File("..", "../file.adoc").toString(), 2, "path", "dir"); + final LogRecord logRecord = new LogRecord(Severity.INFO, cursor, "a message"); + final File sourceDir = getParentFile(); + // when + String formattedLogRecord = LogRecordFormatter.format(logRecord, sourceDir); + // then + assertThat(normalizePath(formattedLogRecord)).matches("asciidoctor: INFO: ../../file.adoc: line 2: a message"); + } + + @Test + public void should_format_full_logRecord_when_cursor_is_http_source() { + // given + final TestCursor cursor = new TestCursor("http://something/source.adoc", 3, "path", "dir"); + final LogRecord logRecord = new LogRecord(Severity.INFO, cursor, "a message"); + // when + String formattedLogRecord = LogRecordFormatter.format(logRecord, null); + // then + assertThat(normalizePath(formattedLogRecord)).isEqualTo("asciidoctor: INFO: http://something/source.adoc: line 3: a message"); + } + + @Test + public void should_format_full_logRecord_when_cursor_is_https_source() { + // given + final TestCursor cursor = new TestCursor("https://something/source.adoc", 3, "path", "dir"); + final LogRecord logRecord = new LogRecord(Severity.INFO, cursor, "a message"); + // when + String formattedLogRecord = LogRecordFormatter.format(logRecord, null); + // then + assertThat(normalizePath(formattedLogRecord)).isEqualTo("asciidoctor: INFO: https://something/source.adoc: line 3: a message"); + } + private File getParentFile() { return new File(".").getAbsoluteFile().getParentFile().getParentFile(); } @@ -116,5 +148,4 @@ public String getFile() { return file; } } - }