Skip to content

Commit 846e704

Browse files
MarkusPaulsenMarkusPaulsen
authored
Merge pull request #18 from ls1intum/chore/fix-failing-tests-remove-aspectconfig
`Ares`: ArchUnit Network access, JVM termination and reflection tests for post-compile mode
2 parents 10a756c + 616c5c2 commit 846e704

File tree

15 files changed

+852
-365
lines changed

15 files changed

+852
-365
lines changed

‎src/main/java/de/tum/cit/ase/ares/api/architecturetest/ArchitectureTestCase.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package de.tum.cit.ase.ares.api.architecturetest;
22

3+
import com.tngtech.archunit.core.domain.JavaClasses;
4+
35
/**
46
* Interface for the architecture test cases in any programming language and abstract product of the abstract factory design pattern.
57
*
@@ -19,5 +21,5 @@ public interface ArchitectureTestCase {
1921
/**
2022
* Runs the architecture test case in any programming language.
2123
*/
22-
void runArchitectureTestCase();
24+
void runArchitectureTestCase(JavaClasses classes);
2325
}

‎src/main/java/de/tum/cit/ase/ares/api/architecturetest/java/FileHandlerConstants.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88
*/
99
public class FileHandlerConstants {
1010

11-
public static final Path JAVA_FILESYSTEM_INTERACTION_METHODS = Path.of("src" + File.separator + "main" + File.separator + "resources" + File.separator + "archunit" + File.separator + "files" + File.separator + "java" + File.separator + "methods" + File.separator + "file-system-access-methods.txt");
12-
public static final Path JAVA_FILESYSTEM_INTERACTION_CONTENT = Path.of("src" + File.separator + "main" + File.separator + "resources" + File.separator + "archunit" + File.separator + "files" + File.separator + "java" + File.separator + "rules" + File.separator + "file-system-arch-rule.txt");
11+
private static final String JAVA_METHODS_DIRECTORY = "src" + File.separator + "main" + File.separator + "resources" + File.separator + "archunit" + File.separator + "files" + File.separator + "java" + File.separator + "methods" + File.separator;
12+
public static final Path JAVA_FILESYSTEM_INTERACTION_METHODS = Path.of(JAVA_METHODS_DIRECTORY + "file-system-access-methods.txt");
13+
public static final Path JAVA_NETWORK_ACCESS_METHODS = Path.of(JAVA_METHODS_DIRECTORY + "network-access-methods.txt");
14+
public static final Path JAVA_JVM_TERMINATION_METHODS = Path.of(JAVA_METHODS_DIRECTORY + "jvm-termination-methods.txt");
15+
public static final Path JAVA_REFLECTION_METHODS = Path.of(JAVA_METHODS_DIRECTORY + "reflection-methods.txt");
1316

1417
private FileHandlerConstants() {
1518
throw new IllegalArgumentException("FileHandlerConstants is a utility class and should not be instantiated");

‎src/main/java/de/tum/cit/ase/ares/api/architecturetest/java/JavaArchitectureTestCase.java‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
package de.tum.cit.ase.ares.api.architecturetest.java;
22

33
import com.tngtech.archunit.core.domain.JavaClasses;
4-
import com.tngtech.archunit.core.importer.ClassFileImporter;
54
import de.tum.cit.ase.ares.api.architecturetest.ArchitectureTestCase;
65
import de.tum.cit.ase.ares.api.architecturetest.java.postcompile.JavaArchitectureTestCaseCollection;
7-
import de.tum.cit.ase.ares.api.util.ProjectSourcesFinder;
6+
import de.tum.cit.ase.ares.api.policy.PackageImport;
87

9-
import java.io.File;
8+
import java.io.IOException;
109
import java.nio.file.Path;
10+
import java.util.ArrayList;
11+
import java.util.HashSet;
12+
import java.util.List;
13+
import java.util.Set;
14+
import java.util.stream.Collectors;
1115

1216
import static de.tum.cit.ase.ares.api.architecturetest.java.postcompile.JavaArchitectureTestCaseCollection.getArchitectureRuleFileContent;
1317

@@ -25,43 +29,58 @@ public class JavaArchitectureTestCase implements ArchitectureTestCase {
2529
* Selects the supported architecture test case in the Java programming language.
2630
*/
2731
private final JavaSupportedArchitectureTestCase javaSupportedArchitectureTestCase;
28-
private final Path withinPath;
32+
33+
/**
34+
* List of allowed packages to be imported.
35+
*/
36+
private final Set<String> allowedPackages;
2937

3038
/**
3139
* Constructor for JavaArchitectureTestCase.
3240
*
3341
* @param javaSupportedArchitectureTestCase Selects the supported architecture test case in the Java programming language
3442
*/
35-
public JavaArchitectureTestCase(JavaSupportedArchitectureTestCase javaSupportedArchitectureTestCase, Path withinPath) {
43+
public JavaArchitectureTestCase(JavaSupportedArchitectureTestCase javaSupportedArchitectureTestCase) {
3644
super();
45+
this.allowedPackages = new HashSet<>();
3746
this.javaSupportedArchitectureTestCase = javaSupportedArchitectureTestCase;
38-
this.withinPath = withinPath;
47+
}
48+
49+
public JavaArchitectureTestCase(JavaSupportedArchitectureTestCase javaSupportedArchitectureTestCase, Set<PackageImport> packages) {
50+
super();
51+
this.javaSupportedArchitectureTestCase = javaSupportedArchitectureTestCase;
52+
this.allowedPackages = packages.stream().map(PackageImport::iAllowTheStudentsToImportTheFollowingPackage).collect(Collectors.toSet());
3953
}
4054

4155
/**
4256
* Returns the content of the architecture test case file in the Java programming language.
4357
*/
4458
@Override
4559
public String createArchitectureTestCaseFileContent() {
46-
return getArchitectureRuleFileContent(this.javaSupportedArchitectureTestCase.name());
60+
try {
61+
return getArchitectureRuleFileContent(this.javaSupportedArchitectureTestCase.name());
62+
} catch (AssertionError e) {
63+
throw new SecurityException("Ares Security Error (Stage: Execution): Illegal Statement found: " + e.getMessage());
64+
} catch (IOException e) {
65+
throw new IllegalStateException("Could not load the architecture rule file content", e);
66+
}
4767
}
4868

4969
/**
5070
* Runs the architecture test case in the Java programming language.
5171
*/
5272
@Override
53-
public void runArchitectureTestCase() {
54-
JavaClasses classes = new ClassFileImporter().importPath((ProjectSourcesFinder.isGradleProject() ? "build" : "target") + File.separator + withinPath.toString());
73+
public void runArchitectureTestCase(JavaClasses classes) {
5574
try {
5675
switch (this.javaSupportedArchitectureTestCase) {
5776
case FILESYSTEM_INTERACTION ->
5877
JavaArchitectureTestCaseCollection.NO_CLASS_SHOULD_ACCESS_FILE_SYSTEM.check(classes);
59-
case PACKAGE_IMPORT -> throw new UnsupportedOperationException("Package import not implemented yet");
78+
case PACKAGE_IMPORT -> JavaArchitectureTestCaseCollection.noClassesShouldImportForbiddenPackages(allowedPackages).check(classes);
6079
case THREAD_CREATION -> throw new UnsupportedOperationException("Thread creation not implemented yet");
6180
case COMMAND_EXECUTION ->
6281
throw new UnsupportedOperationException("Command execution not implemented yet");
6382
case NETWORK_CONNECTION ->
64-
throw new UnsupportedOperationException("Network connection not implemented yet");
83+
JavaArchitectureTestCaseCollection.NO_CLASSES_SHOULD_ACCESS_NETWORK.check(classes);
6584
default -> throw new UnsupportedOperationException("Not implemented yet");
6685
}
6786
} catch (AssertionError e) {

‎src/main/java/de/tum/cit/ase/ares/api/architecturetest/java/postcompile/CustomClassResolver.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public CustomClassResolver() {
2121
// TODO: We definitely need to improve this. We should not import all classes as it is not memory efficient.
2222
// https://www.javadoc.io/doc/com.tngtech.archunit/archunit/0.10.2/com/tngtech/archunit/core/importer/ClassFileImporter.html
2323
allClasses = new ClassFileImporter()
24+
.withImportOption(location -> !location.contains("jrt"))
2425
.importClasspath();
2526
}
2627

‎src/main/java/de/tum/cit/ase/ares/api/architecturetest/java/postcompile/JavaArchitectureTestCaseCollection.java‎

Lines changed: 89 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,21 @@
33
import com.google.common.collect.ImmutableMap;
44
import com.tngtech.archunit.base.DescribedPredicate;
55
import com.tngtech.archunit.core.domain.JavaAccess;
6+
import com.tngtech.archunit.core.domain.JavaClass;
67
import com.tngtech.archunit.lang.ArchRule;
78
import com.tngtech.archunit.lang.syntax.ArchRuleDefinition;
9+
import de.tum.cit.ase.ares.api.architecturetest.java.FileHandlerConstants;
10+
import de.tum.cit.ase.ares.api.architecturetest.java.JavaSupportedArchitectureTestCase;
811

912
import java.io.IOException;
1013
import java.nio.file.Files;
1114
import java.nio.file.Path;
15+
import java.nio.file.Paths;
1216
import java.util.HashSet;
13-
import java.util.List;
14-
import java.util.Optional;
1517
import java.util.Set;
1618

1719
import static de.tum.cit.ase.ares.api.architecturetest.java.JavaSupportedArchitectureTestCase.FILESYSTEM_INTERACTION;
20+
import static de.tum.cit.ase.ares.api.architecturetest.java.JavaSupportedArchitectureTestCase.NETWORK_CONNECTION;
1821

1922
/**
2023
* This class runs the security rules on the architecture for the post-compile mode.
@@ -25,29 +28,12 @@ private JavaArchitectureTestCaseCollection() {
2528
throw new IllegalArgumentException("This class should not be instantiated");
2629
}
2730

31+
public static final String LOAD_FORBIDDEN_METHODS_FROM_FILE_FAILED = "Could not load the architecture rule file content";
2832
/**
2933
* Map to store the forbidden methods for the supported architectural test cases
3034
*/
3135
private static final ImmutableMap.Builder<String, Set<String>> FORBIDDEN_METHODS_FOR_SUPPORTED_ARCHITECTURAL_TEST_CASE = ImmutableMap.builder();
3236

33-
/**
34-
* Map to store the content of the architecture test case files
35-
*/
36-
private static final ImmutableMap.Builder<String, String> ARCHITECTURAL_RULES_CONTENT_MAP = ImmutableMap.builder();
37-
38-
/**
39-
* The packages that should not be accessed by the student submission.
40-
*/
41-
private static final List<String> BANNED_FILESYSTEM_ACCESS_PACKAGES = List.of(
42-
"java.nio.file",
43-
"java.util.prefs",
44-
"sun.print",
45-
"java.util.jar",
46-
"java.util.zip",
47-
"sun.awt.X11",
48-
"javax.imageio",
49-
"javax.sound.midi",
50-
"javax.swing.filechooser");
5137

5238
/**
5339
* Load pre file contents
@@ -57,14 +43,6 @@ public static void loadForbiddenMethodsFromFile(Path filePath, String key) throw
5743
FORBIDDEN_METHODS_FOR_SUPPORTED_ARCHITECTURAL_TEST_CASE.put(key, content);
5844
}
5945

60-
/**
61-
* Load the content of the architecture test case files
62-
*/
63-
public static void loadArchitectureRuleFileContent(Path filePath, String key) throws IOException {
64-
String content = Files.readString(filePath);
65-
ARCHITECTURAL_RULES_CONTENT_MAP.put(key, content);
66-
}
67-
6846
/**
6947
* Get the content of a file from the architectural rules storage
7048
*/
@@ -75,23 +53,99 @@ public static Set<String> getForbiddenMethods(String key) {
7553
/**
7654
* Get the content of a file from the architectural rules storage
7755
*/
78-
public static String getArchitectureRuleFileContent(String key) {
79-
return ARCHITECTURAL_RULES_CONTENT_MAP.build().get(key);
56+
public static String getArchitectureRuleFileContent(String key) throws IOException {
57+
return Files.readString(Paths.get("src", "main", "resources", "archunit", "files", "java", "rules", "%s.txt".formatted(key)));
8058
}
8159

8260
/**
8361
* This method checks if any class in the given package accesses the file system.
8462
*/
8563
public static final ArchRule NO_CLASS_SHOULD_ACCESS_FILE_SYSTEM = ArchRuleDefinition.noClasses()
8664
.should(new TransitivelyAccessesMethodsCondition(new DescribedPredicate<>("accesses file system") {
65+
private Set<String> forbiddenMethods;
66+
67+
@Override
68+
public boolean test(JavaAccess<?> javaAccess) {
69+
if (forbiddenMethods == null) {
70+
try {
71+
loadForbiddenMethodsFromFile(FileHandlerConstants.JAVA_FILESYSTEM_INTERACTION_METHODS, JavaSupportedArchitectureTestCase.FILESYSTEM_INTERACTION.name());
72+
} catch (IOException e) {
73+
throw new IllegalStateException(LOAD_FORBIDDEN_METHODS_FROM_FILE_FAILED, e);
74+
}
75+
forbiddenMethods = getForbiddenMethods(FILESYSTEM_INTERACTION.name());
76+
}
77+
78+
return forbiddenMethods.stream().anyMatch(method -> javaAccess.getTarget().getFullName().startsWith(method));
79+
}
80+
}));
81+
82+
/**
83+
* This method checks if any class in the given package accesses the network.
84+
*/
85+
public static final ArchRule NO_CLASSES_SHOULD_ACCESS_NETWORK = ArchRuleDefinition.noClasses()
86+
.should(new TransitivelyAccessesMethodsCondition(new DescribedPredicate<>("accesses network") {
87+
private Set<String> forbiddenMethods;
88+
89+
@Override
90+
public boolean test(JavaAccess<?> javaAccess) {
91+
if (forbiddenMethods == null) {
92+
try {
93+
loadForbiddenMethodsFromFile(FileHandlerConstants.JAVA_NETWORK_ACCESS_METHODS, JavaSupportedArchitectureTestCase.NETWORK_CONNECTION.name());
94+
} catch (IOException e) {
95+
throw new IllegalStateException(LOAD_FORBIDDEN_METHODS_FROM_FILE_FAILED, e);
96+
}
97+
forbiddenMethods = getForbiddenMethods(NETWORK_CONNECTION.name());
98+
}
99+
100+
return forbiddenMethods.stream().anyMatch(method -> javaAccess.getTarget().getFullName().startsWith(method));
101+
}
102+
}));
103+
104+
/**
105+
* This method checks if any class in the given package imports forbidden packages.
106+
*/
107+
public static ArchRule noClassesShouldImportForbiddenPackages(Set<String> allowedPackages) {
108+
return ArchRuleDefinition.noClasses()
109+
.should()
110+
.transitivelyDependOnClassesThat(new DescribedPredicate<>("imports package") {
111+
@Override
112+
public boolean test(JavaClass javaClass) {
113+
return !allowedPackages.contains(javaClass.getPackageName());
114+
}
115+
});
116+
}
117+
118+
/**
119+
* This method checks if any class in the given package uses reflection.
120+
*/
121+
public static final ArchRule NO_CLASSES_SHOULD_USE_REFLECTION = ArchRuleDefinition.noClasses()
122+
.should(new TransitivelyAccessesMethodsCondition(new DescribedPredicate<>("uses reflection") {
123+
@Override
124+
public boolean test(JavaAccess<?> javaAccess) {
125+
return javaAccess.getTarget().getFullName().startsWith("java.lang.reflect")
126+
|| javaAccess.getTarget().getFullName().startsWith("sun.reflect.misc");
127+
}
128+
}));
129+
130+
/**
131+
* This method checks if any class in the given package uses the command line.
132+
*/
133+
public static final ArchRule NO_CLASSES_SHOULD_TERMINATE_JVM = ArchRuleDefinition.noClasses()
134+
.should(new TransitivelyAccessesMethodsCondition((new DescribedPredicate<>("terminates JVM") {
135+
private Set<String> forbiddenMethods;
136+
87137
@Override
88138
public boolean test(JavaAccess<?> javaAccess) {
89-
if (BANNED_FILESYSTEM_ACCESS_PACKAGES.stream().anyMatch(p -> javaAccess.getTarget().getFullName().startsWith(p))) {
90-
return true;
139+
if (forbiddenMethods == null) {
140+
try {
141+
loadForbiddenMethodsFromFile(FileHandlerConstants.JAVA_JVM_TERMINATION_METHODS, "JVM_TERMINATION");
142+
} catch (IOException e) {
143+
throw new IllegalStateException(LOAD_FORBIDDEN_METHODS_FROM_FILE_FAILED, e);
144+
}
145+
forbiddenMethods = getForbiddenMethods("JVM_TERMINATION");
91146
}
92147

93-
Optional<Set<String>> bannedMethods = Optional.ofNullable(JavaArchitectureTestCaseCollection.getForbiddenMethods(FILESYSTEM_INTERACTION.name()));
94-
return bannedMethods.map(strings -> strings.contains(javaAccess.getTarget().getName())).orElse(false);
148+
return forbiddenMethods.stream().anyMatch(method -> javaAccess.getTarget().getFullName().startsWith(method));
95149
}
96-
}, FILESYSTEM_INTERACTION));
150+
})));
97151
}

0 commit comments

Comments
 (0)