Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid crash when processing logs of files not under the source dir #563

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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
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
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://");
}
}
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
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
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;
}
}

}