Skip to content

Commit

Permalink
Avoid crash when processing logs of files not under the source dir (#563
Browse files Browse the repository at this point in the history 
)

* Rename LogRecordHelper into LogRecordFormatter
  • Loading branch information
@abelsromero
abelsromero authored Dec 24, 2021
1 parent 7a0c6f8 commit fb79430
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/asciidoctor/maven/AsciidoctorMojo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -236,7 +236,7 @@ public void processSources(List<File> 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);
Expand Down
43 changes: 28 additions & 15 deletions ...sciidoctor/maven/log/LogRecordHelper.java → ...idoctor/maven/log/LogRecordFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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<String> 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());
Expand All @@ -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://");
}
}
6 changes: 3 additions & 3 deletions src/main/java/org/asciidoctor/maven/log/LogRecordsProcessors.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void processLogRecords(MemoryLogHandler memoryLogHandler) throws Exceptio
final List<LogRecord> 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));
}
Expand All @@ -39,7 +39,7 @@ public void processLogRecords(MemoryLogHandler memoryLogHandler) throws Exceptio
final List<LogRecord> 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));
}
Expand All @@ -48,7 +48,7 @@ public void processLogRecords(MemoryLogHandler memoryLogHandler) throws Exceptio
final List<LogRecord> 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));
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/asciidoctor/maven/site/AsciidoctorDoxiaParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
51 changes: 41 additions & 10 deletions ...doctor/maven/log/LogRecordHelperTest.java → ...tor/maven/log/LogRecordFormatterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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");
}
Expand All @@ -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");
}
Expand All @@ -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");
}
Expand All @@ -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();
}
Expand Down Expand Up @@ -116,5 +148,4 @@ public String getFile() {
return file;
}
}

}

0 comments on commit fb79430

Please sign in to comment.