NIFI-10765 Added better error reporting in JASN1Reader

This closes #6703

Signed-off-by: David Handermann <exceptionfactory@apache.org>
This commit is contained in:
Tamas Palfy 2022-11-22 19:24:55 +01:00 committed by exceptionfactory
parent ee5d557d2a
commit f630540891
No known key found for this signature in database
GPG Key ID: 29B6A52D2AAE8DBA
8 changed files with 402 additions and 28 deletions

View File

@ -62,6 +62,12 @@
<artifactId>caffeine</artifactId>
<version>2.8.1</version>
</dependency>
<dependency>
<groupId>antlr</groupId>
<artifactId>antlr</artifactId>
<version>2.7.7</version>
</dependency>
<!-- For Test -->
<dependency>
@ -130,6 +136,9 @@
<exclude>src/test/resources/simple_types.asn</exclude>
<exclude>src/test/resources/complex_types.asn</exclude>
<exclude>src/test/resources/tbcd_string.asn</exclude>
<exclude>src/test/resources/cant_parse.asn</exclude>
<exclude>src/test/resources/cant_compile.asn</exclude>
<exclude>src/test/resources/cant_compile_mac_windows.asn</exclude>
<exclude>src/test/resources/examples/basic-types.dat</exclude>
<exclude>src/test/resources/examples/composite.dat</exclude>
<exclude>src/test/resources/examples/tbcd-string.dat</exclude>
@ -139,23 +148,14 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<executions>
<execution>
<id>check-style</id>
<phase>verify</phase>
<configuration>
<encoding>UTF-8</encoding>
<suppressionsLocation>${basedir}/checkstyle-suppressions.xml</suppressionsLocation>
<sourceDirectories>
<sourceDirectory>${project.build.sourceDirectory}</sourceDirectory>
<sourceDirectory>${project.build.testSourceDirectory}</sourceDirectory>
</sourceDirectories>
</configuration>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
<configuration>
<encoding>UTF-8</encoding>
<suppressionsLocation>${basedir}/checkstyle-suppressions.xml</suppressionsLocation>
<sourceDirectories>
<sourceDirectory>${project.build.sourceDirectory}</sourceDirectory>
<sourceDirectory>${project.build.testSourceDirectory}</sourceDirectory>
</sourceDirectories>
</configuration>
</plugin>
</plugins>
</build>

View File

@ -0,0 +1,37 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.beanit.asn1bean.compiler;
import com.beanit.asn1bean.compiler.model.AsnModule;
import java.nio.file.Path;
import java.util.HashMap;
public class BerClassWriterFactory {
public static BerClassWriter createBerClassWriter(HashMap<String, AsnModule> modulesByName, Path asnOutDir) {
BerClassWriter berClassWriter = new BerClassWriter(
modulesByName,
asnOutDir.toString(),
"",
true,
false,
false
);
return berClassWriter;
}
}

View File

@ -16,6 +16,14 @@
*/
package org.apache.nifi.jasn1;
import antlr.RecognitionException;
import antlr.TokenStreamException;
import com.beanit.asn1bean.compiler.BerClassWriter;
import com.beanit.asn1bean.compiler.BerClassWriterFactory;
import com.beanit.asn1bean.compiler.model.AsnModel;
import com.beanit.asn1bean.compiler.model.AsnModule;
import com.beanit.asn1bean.compiler.parser.ASNLexer;
import com.beanit.asn1bean.compiler.parser.ASNParser;
import org.apache.nifi.annotation.documentation.CapabilityDescription;
import org.apache.nifi.annotation.documentation.Tags;
import org.apache.nifi.annotation.lifecycle.OnDisabled;
@ -38,11 +46,14 @@ import org.apache.nifi.serialization.RecordReader;
import org.apache.nifi.serialization.RecordReaderFactory;
import org.apache.nifi.util.file.classloader.ClassLoaderUtils;
import javax.tools.DiagnosticCollector;
import javax.tools.JavaCompiler;
import javax.tools.JavaFileObject;
import javax.tools.StandardJavaFileManager;
import javax.tools.ToolProvider;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
@ -51,8 +62,13 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
@Tags({"asn", "ans1", "jasn.1", "jasn1", "record", "reader", "parser"})
@ -125,7 +141,7 @@ public class JASN1Reader extends AbstractConfigurableComponent implements Record
);
private String identifier;
private ComponentLog logger;
ComponentLog logger;
private RecordSchemaProvider schemaProvider = new RecordSchemaProvider();
@ -219,10 +235,38 @@ public class JASN1Reader extends AbstractConfigurableComponent implements Record
asnCompilerArguments.add("-o");
asnCompilerArguments.add(asnOutDir.toString());
HashMap<String, AsnModule> modulesByName = new HashMap<>();
Exception parseException = null;
for (String asn1File : asnFilePaths) {
logger.info("Parsing " + asn1File);
try {
AsnModel model = getJavaModelFromAsn1File(asn1File);
modulesByName.putAll(model.modulesByName);
} catch (FileNotFoundException e) {
logger.error("ASN.1 file not found [{}]", asn1File, e);
parseException = e;
} catch (TokenStreamException | RecognitionException e) {
logger.error("ASN.1 stream parsing failed [{}]", asn1File, e);
parseException = e;
} catch (Exception e) {
logger.error("ASN.1 parsing failed [{}]", asn1File, e);
parseException = e;
}
}
if (parseException != null) {
throw new ProcessException("ASN.1 parsing failed", parseException);
}
try {
com.beanit.asn1bean.compiler.Compiler.main(asnCompilerArguments.toArray(new String[0]));
logger.info("Writing ASN.1 classes to directory [{}]", asnOutDir);
BerClassWriter classWriter = BerClassWriterFactory.createBerClassWriter(modulesByName, asnOutDir);
classWriter.translate();
} catch (Exception e) {
throw new ProcessException("Couldn't compile asn files to java.", e);
throw new ProcessException("ASN.1 compilation failed", e);
}
List<File> javaFiles;
@ -234,7 +278,7 @@ public class JASN1Reader extends AbstractConfigurableComponent implements Record
.map(File::new)
.collect(Collectors.toList());
} catch (IOException e) {
throw new ProcessException("Couldn't access '" + asnOutDir + "'");
throw new ProcessException("Access directory failed " + asnOutDir);
}
JavaCompiler javaCompiler = ToolProvider.getSystemJavaCompiler();
@ -246,10 +290,17 @@ public class JASN1Reader extends AbstractConfigurableComponent implements Record
Iterable<? extends JavaFileObject> units;
units = fileManager.getJavaFileObjectsFromFiles(javaFiles);
JavaCompiler.CompilationTask task = javaCompiler.getTask(null, fileManager, null, optionList, null, units);
DiagnosticCollector<JavaFileObject> diagnosticListener = new DiagnosticCollector<>();
JavaCompiler.CompilationTask task = javaCompiler.getTask(null, fileManager, diagnosticListener, optionList, null, units);
Boolean success = task.call();
if (!success) {
throw new ProcessException("Couldn't compile java file.");
Set<String> errorMessages = new LinkedHashSet();
diagnosticListener.getDiagnostics().stream().map(d -> d.getMessage(Locale.getDefault())).forEach(errorMessages::add);
errorMessages.forEach(logger::error);
throw new ProcessException("Java compilation failed");
}
}
@ -266,7 +317,7 @@ public class JASN1Reader extends AbstractConfigurableComponent implements Record
.map(Path::toFile)
.forEach(File::delete);
} catch (IOException e) {
throw new ProcessException("Couldn't delete '" + asnOutDir + "'");
throw new ProcessException("Delete directory failed " + asnOutDir);
}
}
}
@ -294,6 +345,37 @@ public class JASN1Reader extends AbstractConfigurableComponent implements Record
return new JASN1RecordReader(rootClassName, recordField, schemaProvider, customClassLoader, iteratorProviderClassName, in, logger);
}
AsnModel getJavaModelFromAsn1File(String inputFileName)
throws FileNotFoundException, TokenStreamException, RecognitionException {
InputStream stream = new FileInputStream(inputFileName);
ASNLexer lexer = new ASNLexer(stream);
AtomicBoolean parseError = new AtomicBoolean(false);
ASNParser parser = new ASNParser(lexer) {
@Override
public void reportError(String s) {
logger.error("{} - {}", inputFileName, s);
parseError.set(true);
}
@Override
public void reportError(RecognitionException e) {
logger.error("{} - {}", inputFileName, e.toString());
parseError.set(true);
}
};
if (parseError.get()) {
throw new ProcessException("ASN.1 parsing failed");
}
AsnModel model = new AsnModel();
parser.module_definitions(model);
return model;
}
String guessRootClassName(String rootModelName) {
try {
StringBuilder rootClassNameBuilder = new StringBuilder();
@ -311,5 +393,4 @@ public class JASN1Reader extends AbstractConfigurableComponent implements Record
throw new ProcessException("Couldn't infer root model name from '" + rootModelName + "'", e);
}
}
}

View File

@ -27,13 +27,19 @@
This service creates record readers for ASN.1 input.
</p>
<p>
ASN.1 schema files (with full path) can be defined via the <i>Root Model Name</i> property as a comma separated list.
ASN.1 schema files (with full path) can be defined via the <i>ASN.1 Files</i> property as a comma separated list.
The controller service preprocesses these files and generates sources that it uses for parsing data later.<br/>
<b>
Note that this preprocessing may take a while, especially when the schema files are large.
The service remains in the <i>Enabling</i> state until this preprocessing is finished.
Processors using the service are ready to be started at this point but probably won't work properly until the service is fully <i>Enabled</i>.
</b>
<br />
<b>
Also note that the preprocessing phase can fail if there are problems with the ASN.1 schema files. The bulletin - as per usual -
will show error messages related to the failure but interpreting those messages may not be straightforward.
For help troubleshooting such messages please refer to the <i>Troubleshooting</i> section below.
</b>
</p>
<p>
The root model type can be defined via the <i>Root Model Name</i> property. It's format should be "MODULE-NAME.ModelType".
@ -58,5 +64,63 @@
To be able to set the property the service needs to be disabled in the end - and let it remove the directory,
however this shouldn't be an issue as the name of the root model class will be the same in the new temporary directory.)
</p>
<h3>Troubleshooting</h3>
<p>
The preprocessing is done in two phases:
<ol>
<li>
The first phase reads the ASN.1 schema files and parses them. Formatting errors are usually reported during this phase.
Here are some possible error messages and the potential root causes of the issues:
<ul>
<li>
<i>line NNN:MMM: unexpected token: someFieldName</i> - On the NNNth line, starting at the MMMth position, <i>someFieldName</i> is encountered
which was unexpected. Usually this means <i>someFieldName</i> itself is fine but the previous field declaration doesn't have a comma ',' at the end.
</li>
<li>
<i>line NNN:MMM: unexpected token: [</i> - On the NNNth line, starting at the MMMth position, the opening square bracket '[' is encountered
which was unexpected. Usually this is the index part of the field declaration and for some reason the field declaration is invalid. This can typically
occur if the field name is invalid, e.g. starts with an uppercase letter. (Field names must start with a lowercase letter.)
</li>
</ul>
</li>
<li>
The second phase compiles the ASN.1 schema files into Java classes. Even if the ASN.1 files meet the formal requirements, due to the nature of the created Java
files there are some extra limitations:
<ul>
<li>
On certain systems type names are treated as case-insensitive. Because of this, two types whose names only differ in the cases of their letters may cause errors.
For example if the ASN.1 schema files define both 'SameNameWithDifferentCase' and 'SAMENAMEWithDifferentCase', the following error may be reported:
<br />
<br />
<i>
class SAMENAMEWithDifferentCase is public, should be declared in a file named SAMENAMEWithDifferentCase.java
</i>
</li>
<li>
Certain keywords cannot be used as field names. Known reserved keywords and the corresponding reported error messages are:
</li>
<ul>
<li>
length
</li>
<br />
<i>
incompatible types: com.beanit.asn1bean.ber.types.BerInteger cannot be converted to com.beanit.asn1bean.ber.BerLength
</i>
<br />
<i>
incompatible types: boolean cannot be converted to java.io.OutputStream
</i>
<br />
<i>
Some messages have been simplified; recompile with -Xdiags:verbose to get full output
</i>
</ul>
</ul>
</li>
</ol>
</p>
</body>
</html>

View File

@ -19,18 +19,35 @@ package org.apache.nifi.jasn1;
import org.apache.nifi.controller.ConfigurationContext;
import org.apache.nifi.controller.ControllerServiceInitializationContext;
import org.apache.nifi.logging.ComponentLog;
import org.apache.nifi.processor.exception.ProcessException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import java.io.FileNotFoundException;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;
import java.util.StringJoiner;
import static org.apache.nifi.jasn1.JASN1Reader.ASN_FILES;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Answers.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
public class JASN1ReaderTest {
@ -66,7 +83,7 @@ public class JASN1ReaderTest {
// GIVEN
ConfigurationContext context = mock(ConfigurationContext.class, RETURNS_DEEP_STUBS);
when(context.getProperty(ASN_FILES).isSet()).thenReturn(true);
when(context.getProperty(ASN_FILES).evaluateAttributeExpressions().getValue()).thenReturn("src/test/resources/test.asn");
when(context.getProperty(ASN_FILES).evaluateAttributeExpressions().getValue()).thenReturn(Paths.get("src", "test", "resources", "test.asn").toString());
// WHEN
testSubject.onEnabled(context);
@ -78,4 +95,135 @@ public class JASN1ReaderTest {
assertEquals("org.apache.nifi.jasn1.test.RootType", actualRootModelName);
assertNotNull(actual);
}
@Test
public void testAsnFileDoesntExist() throws Exception {
// GIVEN
ConfigurationContext context = mock(ConfigurationContext.class, RETURNS_DEEP_STUBS);
when(context.getProperty(ASN_FILES).isSet()).thenReturn(true);
when(context.getProperty(ASN_FILES).evaluateAttributeExpressions().getValue()).thenReturn(
new StringJoiner(",")
.add(Paths.get("src", "test", "resources", "test.asn").toString())
.add(Paths.get("src", "test", "resources", "doesnt_exist.asn").toString())
.toString()
);
// WHEN
ProcessException processException = assertThrows(
ProcessException.class,
() -> testSubject.onEnabled(context)
);
Throwable cause = processException.getCause();
assertEquals(FileNotFoundException.class, cause.getClass());
assertThat(cause.getMessage(), containsString("doesnt_exist.asn"));
}
@Test
/*
* Checks reported messages of underlying libraries that are explained in additionalDetails.html.
* In case of changes to this test additionalDetails.html may need to be updated as well.
*/
public void testCantParseAsn() throws Exception {
// GIVEN
String asnFile = Paths.get("src", "test", "resources", "cant_parse.asn").toString();
List<String> expectedErrorMessages = Arrays.asList(
"line 11:5: unexpected token: field3",
"line 17:33: unexpected token: ["
);
// WHEN
// THEN
testParseError(asnFile, expectedErrorMessages);
}
@Test
/*
* Checks reported messages of underlying libraries that are explained in additionalDetails.html.
* In case of changes to this test additionalDetails.html may need to be updated as well.
*/
public void testCantCompileAsn() throws Exception {
// GIVEN
String asnFiles = Paths.get("src", "test", "resources", "cant_compile.asn").toString();
List<String> expectedErrorMessages = Arrays.asList(
".*com\\.beanit\\.asn1bean\\.ber\\.types\\.BerInteger.*com\\.beanit\\.asn1bean\\.ber\\.BerLength.*",
".*boolean.*java\\.io\\.OutputStream.*",
".*-Xdiags:verbose.*"
);
// WHEN
// THEN
testCompileError(asnFiles, expectedErrorMessages);
}
@EnabledOnOs({ OS.MAC, OS.WINDOWS })
@Test
/*
* Checks reported messages of underlying libraries that are explained in additionalDetails.html.
* In case of changes to this test additionalDetails.html may need to be updated as well.
*/
public void testCantCompileAsnOnMacWindows() throws Exception {
// GIVEN
String asnFiles = Paths.get("src", "test", "resources", "cant_compile_mac_windows.asn").toString();
List<String> expectedErrorMessages = Arrays.asList(
".*SAMENAMEWithDifferentCase.*SAMENAMEWithDifferentCase.*"
);
// WHEN
// THEN
testCompileError(asnFiles, expectedErrorMessages);
}
private void testParseError(String asnFile, List<String> expectedErrorMessages) {
// GIVEN
ConfigurationContext context = mock(ConfigurationContext.class, RETURNS_DEEP_STUBS);
when(context.getProperty(ASN_FILES).isSet()).thenReturn(true);
when(context.getProperty(ASN_FILES).evaluateAttributeExpressions().getValue())
.thenReturn(asnFile);
// WHEN
assertThrows(
ProcessException.class,
() -> testSubject.onEnabled(context)
);
// THEN
ArgumentCaptor<String> errorCaptor = ArgumentCaptor.forClass(String.class);
verify(testSubject.logger, atLeastOnce()).error(eq("{} - {}"), anyString(), errorCaptor.capture());
List<String> actualErrorMessages = errorCaptor.getAllValues();
assertEquals(expectedErrorMessages, actualErrorMessages);
}
private void testCompileError(String asnFiles, List<String> expectedErrorMessages) {
// GIVEN
ConfigurationContext context = mock(ConfigurationContext.class, RETURNS_DEEP_STUBS);
when(context.getProperty(ASN_FILES).isSet()).thenReturn(true);
when(context.getProperty(ASN_FILES).evaluateAttributeExpressions().getValue())
.thenReturn(asnFiles);
// WHEN
assertThrows(
ProcessException.class,
() -> testSubject.onEnabled(context)
);
// THEN
ArgumentCaptor<String> errorCaptor = ArgumentCaptor.forClass(String.class);
verify(testSubject.logger, atLeastOnce()).error(errorCaptor.capture());
List<String> actualErrorMessages = errorCaptor.getAllValues();
assertEquals(expectedErrorMessages.size(), actualErrorMessages.size());
for (int errorMessageIndex = 0; errorMessageIndex < actualErrorMessages.size(); errorMessageIndex++) {
String expectedErrorMessage = expectedErrorMessages.get(errorMessageIndex);
String actualErrorMessage = actualErrorMessages.get(errorMessageIndex);
assertTrue(actualErrorMessage.matches(expectedErrorMessage), "Expected string matching '" + expectedErrorMessage + "', got '" + actualErrorMessage + "'");
}
}
}

View File

@ -0,0 +1,12 @@
ORG-APACHE-NIFI-JASN1-TEST
DEFINITIONS IMPLICIT TAGS ::=
BEGIN
TypeWithReservedJavaKeyword ::= SEQUENCE
{
length [0] INTEGER
}
END

View File

@ -0,0 +1,11 @@
ORG-APACHE-NIFI-JASN1-TEST
DEFINITIONS IMPLICIT TAGS ::=
BEGIN
SameNameWithDifferentCase ::= OCTET STRING
SAMENAMEWithDifferentCase ::= OCTET STRING
END

View File

@ -0,0 +1,21 @@
ORG-APACHE-NIFI-JASN1-TEST
DEFINITIONS IMPLICIT TAGS ::=
BEGIN
MissingComma ::= SEQUENCE
{
field1 [0] BOOLEAN,
field2MissingComma [1] BOOLEAN
field3 [2] BOOLEAN
}
Uppercase ::= SEQUENCE
{
field1 [0] BOOLEAN,
Field2Uppercase [1] BOOLEAN,
field3 [2] BOOLEAN
}
END