Skip to content

Commit

Permalink
Merge pull request #19 from nimakarimipour/nimak/add-gaurd-configpath
Browse files Browse the repository at this point in the history
This PR is a follow up to GH-18 and resolves #17. With this PR, an exception will be thrown at the time a wrong configuration for the Scanner checker (null value for Output or ConfigPath) and will prevent propagation of null value through the program.
  • Loading branch information
nimakarimipour authored Jul 19, 2022
2 parents 0aaa9e1 + bb44dac commit aa37cdb
Show file tree
Hide file tree
Showing 9 changed files with 303 additions and 58 deletions.
2 changes: 2 additions & 0 deletions scanner/src/main/java/edu/ucr/cs/riple/scanner/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import com.google.common.base.Preconditions;
import java.nio.file.Path;
import javax.annotation.Nonnull;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand All @@ -49,6 +50,7 @@ public interface Config {

Serializer getSerializer();

@Nonnull
Path getOutputDirectory();

class Builder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
package edu.ucr.cs.riple.scanner;

import java.nio.file.Path;
import javax.annotation.Nonnull;

public class DummyOptionsConfig implements Config {

Expand Down Expand Up @@ -56,6 +57,7 @@ public Serializer getSerializer() {
throw new IllegalStateException(ERROR_MESSAGE);
}

@Nonnull
@Override
public Path getOutputDirectory() {
throw new IllegalStateException(ERROR_MESSAGE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import javax.annotation.Nonnull;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand All @@ -37,43 +38,37 @@

public class ErrorProneCLIFlagsConfig implements Config {

public final Path outputDirectory;
public final boolean methodTrackerIsActive;
public final boolean fieldTrackerIsActive;
public final boolean callTrackerIsActive;
public final boolean classTrackerIsActive;
public final Serializer serializer;
@Nonnull private final Path outputDirectory;
private final boolean methodTrackerIsActive;
private final boolean fieldTrackerIsActive;
private final boolean callTrackerIsActive;
private final boolean classTrackerIsActive;
private final Serializer serializer;
static final String EP_FL_NAMESPACE = "Scanner";
static final String FL_CONFIG_PATH = EP_FL_NAMESPACE + ":ConfigPath";

public ErrorProneCLIFlagsConfig() {
this.methodTrackerIsActive = false;
this.fieldTrackerIsActive = false;
this.callTrackerIsActive = false;
this.classTrackerIsActive = false;
this.outputDirectory = null;
this.serializer = new Serializer(this);
}

public ErrorProneCLIFlagsConfig(ErrorProneFlags flags) {
String configFilePath = flags.get(FL_CONFIG_PATH).orElse(null);
Document document = null;
if (configFilePath != null) {
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = factory.newDocumentBuilder();
document = builder.parse(Files.newInputStream(Paths.get(configFilePath)));
document.normalize();
} catch (IOException | SAXException | ParserConfigurationException e) {
throw new RuntimeException("Error in reading/parsing config at path: " + configFilePath, e);
}
if (configFilePath == null) {
throw new IllegalStateException(
"Error in Scanner Checker configuration, should be set with via error prone flag: (-XepOpt:Scanner:ConfigPath)");
}
Document document;
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = factory.newDocumentBuilder();
document = builder.parse(Files.newInputStream(Paths.get(configFilePath)));
document.normalize();
} catch (IOException | SAXException | ParserConfigurationException e) {
throw new RuntimeException("Error in reading/parsing config at path: " + configFilePath, e);
}
String outputDirectoryPathInString =
XMLUtil.getValueFromTag(document, "/scanner/path", String.class).orElse(null);
// Here we do not throw an exception if outputDirectoryPathInString is null , since this
// constructor can still be called when the checker is not activated.
this.outputDirectory =
(outputDirectoryPathInString != null) ? Paths.get(outputDirectoryPathInString) : null;
if (outputDirectoryPathInString == null) {
throw new IllegalArgumentException(
"Output path cannot be null, should be set it in config file within <path> tag");
}
this.outputDirectory = Paths.get(outputDirectoryPathInString);
this.methodTrackerIsActive =
XMLUtil.getValueFromAttribute(document, "/scanner/method", "active", Boolean.class)
.orElse(false);
Expand All @@ -86,7 +81,7 @@ public ErrorProneCLIFlagsConfig(ErrorProneFlags flags) {
this.classTrackerIsActive =
XMLUtil.getValueFromAttribute(document, "/scanner/class", "active", Boolean.class)
.orElse(false);
this.serializer = (this.outputDirectory == null) ? null : new Serializer(this);
this.serializer = new Serializer(this);
}

public boolean callTrackerIsActive() {
Expand All @@ -109,6 +104,7 @@ public Serializer getSerializer() {
return serializer;
}

@Nonnull
public Path getOutputDirectory() {
return outputDirectory;
}
Expand Down
16 changes: 8 additions & 8 deletions scanner/src/main/java/edu/ucr/cs/riple/scanner/Serializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ public class Serializer {
public static final String METHOD_INFO_FILE_NAME = "method_info.tsv";
public static final String CLASS_INFO_FILE_NAME = "class_info.tsv";

public Serializer(ErrorProneCLIFlagsConfig config) {
Path outputDirectory = config.outputDirectory;
public Serializer(Config config) {
Path outputDirectory = config.getOutputDirectory();
this.fieldGraphPath = outputDirectory.resolve(FIELD_GRAPH_FILE_NAME);
this.callGraphPath = outputDirectory.resolve(CALL_GRAPH_FILE_NAME);
this.methodInfoPath = outputDirectory.resolve(METHOD_INFO_FILE_NAME);
Expand Down Expand Up @@ -116,19 +116,19 @@ private void initializeFile(Path path, String header) {
}

/** Initializes every file which will be re-generated in the new run of NullAway. */
private void initializeOutputFiles(ErrorProneCLIFlagsConfig config) {
private void initializeOutputFiles(Config config) {
try {
Files.createDirectories(config.outputDirectory);
if (config.callTrackerIsActive) {
Files.createDirectories(config.getOutputDirectory());
if (config.callTrackerIsActive()) {
initializeFile(callGraphPath, TrackerNode.header());
}
if (config.fieldTrackerIsActive) {
if (config.fieldTrackerIsActive()) {
initializeFile(fieldGraphPath, TrackerNode.header());
}
if (config.methodTrackerIsActive) {
if (config.methodTrackerIsActive()) {
initializeFile(methodInfoPath, MethodInfo.header());
}
if (config.classTrackerIsActive) {
if (config.classTrackerIsActive()) {
initializeFile(classInfoPath, ClassInfo.header());
}
} catch (IOException e) {
Expand Down
15 changes: 4 additions & 11 deletions scanner/src/main/java/edu/ucr/cs/riple/scanner/XMLUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import javax.annotation.Nullable;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand All @@ -29,17 +28,14 @@ public class XMLUtil {
* Helper method for reading attributes of node located at /key_1/key_2/.../key_n (in the form of
* {@code Xpath} query) from a {@link Document}.
*
* @param doc XML object to read values from, if null passed, the default value will be returned.
* @param doc XML object to read values from.
* @param key Key to locate the value, can be nested in the form of {@code Xpath} query (e.g.
* /key1/key2:.../key_n).
* @param klass Class type of the value in doc.
* @return The value in the specified keychain cast to the class type given in parameter.
*/
public static <T> DefaultXMLValueProvider<T> getValueFromAttribute(
@Nullable Document doc, String key, String attr, Class<T> klass) {
if (doc == null) {
return new DefaultXMLValueProvider<>(null, klass);
}
Document doc, String key, String attr, Class<T> klass) {
try {
XPath xPath = XPathFactory.newInstance().newXPath();
Node node = (Node) xPath.compile(key).evaluate(doc, XPathConstants.NODE);
Expand All @@ -57,17 +53,14 @@ public static <T> DefaultXMLValueProvider<T> getValueFromAttribute(
* Helper method for reading value of a node located at /key_1/key_2/.../key_n (in the form of
* {@code Xpath} query) from a {@link Document}.
*
* @param doc XML object to read values from, if null passed, the default value will be returned.
* @param doc XML object to read values from.
* @param key Key to locate the value, can be nested in the form of {@code Xpath} query (e.g.
* /key1/key2/.../key_n).
* @param klass Class type of the value in doc.
* @return The value in the specified keychain cast to the class type given in parameter.
*/
public static <T> DefaultXMLValueProvider<T> getValueFromTag(
@Nullable Document doc, String key, Class<T> klass) {
if (doc == null) {
return new DefaultXMLValueProvider<>(null, klass);
}
Document doc, String key, Class<T> klass) {
try {
XPath xPath = XPathFactory.newInstance().newXPath();
Node node = (Node) xPath.compile(key).evaluate(doc, XPathConstants.NODE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,33 @@ public ClassInfoTest() {
}

@Test
public void BasicTest() {
public void basicTest() {
tester
.addSourceLines("edu/ucr/A.java", "package edu.ucr;", "public class A", "{", "}")
.setExpectedOutputs(new ClassInfoDisplay("edu.ucr.A", "edu/ucr/A.java"))
.doTest();
}

@Test
public void checkClassesAreWrittenInFlatName() {
tester
.addSourceFile("SampleClassForTest.java")
.setExpectedOutputs(
new ClassInfoDisplay(
"edu.ucr.cs.riple.scanner.testdata.SampleClassForTest",
"edu/ucr/cs/riple/scanner/SampleClassForTest.java"),
new ClassInfoDisplay(
"edu.ucr.cs.riple.scanner.testdata.SampleClassForTest$Inner",
"edu/ucr/cs/riple/scanner/SampleClassForTest.java"),
new ClassInfoDisplay(
"edu.ucr.cs.riple.scanner.testdata.SampleClassForTest$1InnerMethod",
"edu/ucr/cs/riple/scanner/SampleClassForTest.java"),
new ClassInfoDisplay(
"edu.ucr.cs.riple.scanner.testdata.SampleClassForTest$1InnerMethod$1",
"edu/ucr/cs/riple/scanner/SampleClassForTest.java"),
new ClassInfoDisplay(
"edu.ucr.cs.riple.scanner.testdata.Run",
"edu/ucr/cs/riple/scanner/SampleClassForTest.java"))
.doTest();
}
}
125 changes: 125 additions & 0 deletions scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* MIT License
*
* Copyright (c) 2022 Nima Karimipour
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package edu.ucr.cs.riple.scanner;

import edu.ucr.cs.riple.scanner.tools.ClassInfoDisplay;
import edu.ucr.cs.riple.scanner.tools.Display;
import edu.ucr.cs.riple.scanner.tools.DisplayFactory;
import edu.ucr.cs.riple.scanner.tools.SerializationTestHelper;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerException;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.w3c.dom.Document;
import org.w3c.dom.Element;

/** Includes tests that check wrong configurations are correctly reported. */
@RunWith(JUnit4.class)
public class ConfigurationTest {

@Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder();
// Just a dummy factory to run the tests with, tests will not even start with a null factory.
protected DisplayFactory<Display> factory = values -> new ClassInfoDisplay("Unknown", "Unknown");
protected SerializationTestHelper<Display> tester;
protected Path root;

@Before
public void setup() {
root = Paths.get(temporaryFolder.getRoot().getAbsolutePath());
}

@Test
public void checkExceptionIsThrownIfConfigPathNotSet() {
// -XepOpt:Scanner:ConfigPath is not set should expect an error.
tester =
new SerializationTestHelper<>(root)
.setArgs(
Arrays.asList(
"-d", temporaryFolder.getRoot().getAbsolutePath(), "-Xep:Scanner:ERROR"))
.setOutputFileNameAndHeader("Unknown", "Unknown")
.addSourceFile("SampleClassForTest.java")
.setFactory(factory);
tester.doTestWithExpectingError(
IllegalStateException.class,
"Error in Scanner Checker configuration, should be set with via error prone flag: (-XepOpt:Scanner:ConfigPath)");
}

@Test
public void checkOutputDirIsNonnull() {
// <scanner>...<output> tag is not set and should expect an error.
Path config = root.resolve("scanner.xml");
try {
Files.createDirectories(root);
Files.createFile(config);
DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance();
try {
// Make empty <scanner> tag with no <output> as child.
DocumentBuilder docBuilder = docFactory.newDocumentBuilder();
Document doc = docBuilder.newDocument();
Element rootElement = doc.createElement("scanner");
doc.appendChild(rootElement);
TransformerFactory transformerFactory = TransformerFactory.newInstance();
Transformer transformer = transformerFactory.newTransformer();
DOMSource source = new DOMSource(doc);
StreamResult result = new StreamResult(config.toFile());
transformer.transform(source, result);
} catch (ParserConfigurationException | TransformerException e) {
throw new RuntimeException("Error happened while writing config.", e);
}
} catch (IOException ex) {
throw new UncheckedIOException(ex);
}
tester =
new SerializationTestHelper<>(root)
.setArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-Xep:Scanner:ERROR",
"-XepOpt:Scanner:ConfigPath=" + config))
.setOutputFileNameAndHeader("Unknown", "Unknown")
.addSourceFile("SampleClassForTest.java")
.setFactory(factory);
tester.doTestWithExpectingError(
IllegalArgumentException.class,
"Output path cannot be null, should be set it in config file within <path> tag");
}
}
Loading

0 comments on commit aa37cdb

Please sign in to comment.