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

Implemented Regex invert and revamped template validation #277

Merged
merged 6 commits into from
May 21, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ private ResponseEntity<?> alterTests(
structureTemplate = null;
}

CheckResult<Pair<TestEntity, ProjectEntity>> updateCheckResult = testUtil.checkForTestUpdate(projectId, user, dockerImage, dockerScript, dockerTemplate, httpMethod);
CheckResult<Pair<TestEntity, ProjectEntity>> updateCheckResult = testUtil.checkForTestUpdate(projectId, user, dockerImage, dockerScript, dockerTemplate, structureTemplate, httpMethod);


if (!updateCheckResult.getStatus().equals(HttpStatus.OK)) {
Expand Down Expand Up @@ -220,7 +220,7 @@ public ResponseEntity<?> getTests(@PathVariable("projectid") long projectId, Aut
@DeleteMapping(ApiRoutes.PROJECT_BASE_PATH + "/{projectid}/tests")
@Roles({UserRole.teacher, UserRole.student})
public ResponseEntity<?> deleteTestById(@PathVariable("projectid") long projectId, Auth auth) {
CheckResult<Pair<TestEntity, ProjectEntity>> updateCheckResult = testUtil.checkForTestUpdate(projectId, auth.getUserEntity(), null, null, null, HttpMethod.DELETE);
CheckResult<Pair<TestEntity, ProjectEntity>> updateCheckResult = testUtil.checkForTestUpdate(projectId, auth.getUserEntity(), null, null, null, null, HttpMethod.DELETE);
if (!updateCheckResult.getStatus().equals(HttpStatus.OK)) {
return ResponseEntity.status(updateCheckResult.getStatus()).body(updateCheckResult.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,14 +334,14 @@ public static boolean imageExists(String image) {
}
}

public static boolean isValidTemplate(String template) {
public static void tryTemplate(String template) {
// lines with @ should be the first of a string
// @ is always the first character
// ">" options under the template should be "required, optional or description="..."
boolean atLeastOne = false; // Template should not be empty
String[] lines = template.split("\n");
if (lines[0].charAt(0) != '@') {
return false;
throw new IllegalArgumentException("Template should start with a '@'");
}
boolean isConfigurationLine = false;
for (String line : lines) {
Expand All @@ -359,14 +359,25 @@ public static boolean isValidTemplate(String template) {
// option lines
if (!line.equalsIgnoreCase(">Required") && !line.equalsIgnoreCase(">Optional")
&& !isDescription) {
return false;
throw new IllegalArgumentException("Invalid option in template");
}
} else {
isConfigurationLine = false;
}
}
}
return atLeastOne;
if(! atLeastOne){
throw new IllegalArgumentException("Template should not be empty");
}
}

public static boolean isValidTemplate(String template){
try{
tryTemplate(template);
return true;
}catch (Exception e){
return false;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,39 @@ public class SubmissionTemplateModel {
private static class FileEntry {
public String name;
Pattern pattern;
private String invert(String name){
// invert . with \. for simpler filenames
List<Integer> dotLocations = new ArrayList<>();
List<Integer> escapedDotLocations = new ArrayList<>();
for (int i = 0; i < name.length(); i++) {
if(i > 0 && name.charAt(i - 1) == '\\' && name.charAt(i) == '.'){
escapedDotLocations.add(i);
}else if (name.charAt(i) == '.') {
dotLocations.add(i);
}
}
StringBuilder sb = new StringBuilder();
for(int i = 0; i < name.length(); i++) {
if(escapedDotLocations.contains(i + 1)){
// skip the break
continue;
}else if(escapedDotLocations.contains(i)){
sb.append(".");
}else if(dotLocations.contains(i)){
sb.append("\\.");
}else{
sb.append(name.charAt(i));
}
}
return sb.toString();

}

public FileEntry(String name) {
this.name = name;
pattern = Pattern.compile("^" + name + "$"); // hat for defining start of the string, $ defines the end

this.name = invert(name);

pattern = Pattern.compile("^" + this.name + "$"); // hat for defining start of the string, $ defines the end
}

public boolean matches(String fileName) {
Expand Down Expand Up @@ -52,7 +81,6 @@ public void parseSubmissionTemplate(String templateString) {
mostSpaces = spaceAmount;
}
lines[i] = "\t".repeat(tabsPerSpaces.get(spaceAmount)) + line.replaceAll(" ", "");
;
}

// Create folder stack for keeping track of all the folders while exploring the insides
Expand Down Expand Up @@ -175,4 +203,66 @@ public SubmissionResult checkSubmission(String file) throws IOException {
return checkSubmission(new ZipFile(file));
}

// will throw error if there are errors in the template
public static void tryTemplate(String template) throws IllegalArgumentException {
List<String> lines = List.of(template.split("\n"));
// check if the template is valid, control if every line contains a file parsable string
// check if the file is in a valid folder location (indentation is correct)
// check if the first file has indentation 0
List<Integer> indentionAmounts = new ArrayList<>();
indentionAmounts.add(0);
if(getIndentation(lines.get(0)) != 0){
throw new IllegalArgumentException("First file should not have any spaces or tabs.");
}
boolean newFolder = false;
for(int line_index = 0; line_index < lines.size(); line_index++){
String line = lines.get(line_index);
int indentation = getIndentation(line);
if(line.isEmpty()){
throw new IllegalArgumentException("Empty file name in template, remove blank lines");
}
if(newFolder && indentation > indentionAmounts.get(indentionAmounts.size() - 1)){
// since the indentation is larger than the previous, we are dealing with the first file in a new folder
indentionAmounts.add(indentation);
newFolder = false;
}else{
// we are dealing with a file in a folder, thus the indentation should be equal to one of the previous folders
for(int i = indentionAmounts.size() - 1; i >= 0; i--){
if(indentionAmounts.get(i) == indentation){
break;
}
if(i == 0){
throw new IllegalArgumentException("File at line "+ line_index + " is not in a valid folder location (indentation is incorrect)");
}
}
// check if file is correct, since location is correct

// first check if file contains valid file names
if(line.substring(0,line.length() - 1).contains("/")){
throw new IllegalArgumentException("File/folder at line "+ (line_index+1) + " contains invalid characters");
}
// check if file is a folder
if(line.charAt(line.length() - 1) == '/') {
newFolder = true;
}
}
if(line.charAt(line.length() - 1) == '/'){
// new folder start!
newFolder = true;
}
}
}


private static int getIndentation(String line){
int length = line.length();
// one space is equal to a tab
for(int i = 0; i < length; i++){
if(line.charAt(i) != ' ' && line.charAt(i) != '\t'){
return i;
}
}
return length - 1;
}

}
14 changes: 11 additions & 3 deletions backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.ugent.pidgeon.controllers.ApiRoutes;
import com.ugent.pidgeon.model.json.TestJson;
import com.ugent.pidgeon.model.submissionTesting.DockerSubmissionTestModel;
import com.ugent.pidgeon.model.submissionTesting.SubmissionTemplateModel;
import com.ugent.pidgeon.postgre.models.ProjectEntity;
import com.ugent.pidgeon.postgre.models.TestEntity;
import com.ugent.pidgeon.postgre.models.UserEntity;
Expand Down Expand Up @@ -50,6 +51,7 @@ public CheckResult<Pair<TestEntity, ProjectEntity>> checkForTestUpdate(
String dockerImage,
String dockerScript,
String dockerTemplate,
String structureTemplate,
HttpMethod httpMethod
) {

Expand Down Expand Up @@ -79,7 +81,6 @@ public CheckResult<Pair<TestEntity, ProjectEntity>> checkForTestUpdate(
return new CheckResult<>(HttpStatus.BAD_REQUEST, "A docker image is required if u add a script", null);
}

// This returns false if the image isn't pullt yet! FIX PLS
if(dockerImage != null && !DockerSubmissionTestModel.imageExists(dockerImage)) {
return new CheckResult<>(HttpStatus.BAD_REQUEST, "A valid docker image is required in a docker test.", null);
}
Expand All @@ -96,10 +97,17 @@ public CheckResult<Pair<TestEntity, ProjectEntity>> checkForTestUpdate(
return new CheckResult<>(HttpStatus.BAD_REQUEST, "No docker test script is configured for this test", null);
}

if(dockerTemplate != null && !DockerSubmissionTestModel.isValidTemplate(dockerTemplate)) {
return new CheckResult<>(HttpStatus.BAD_REQUEST, "Invalid docker template", null);
try {
// throws error if there are issues in the template
if(dockerTemplate != null) DockerSubmissionTestModel.tryTemplate(dockerTemplate);
if(structureTemplate != null) SubmissionTemplateModel.tryTemplate(structureTemplate);

} catch(IllegalArgumentException e){
return new CheckResult<>(HttpStatus.BAD_REQUEST, e.getMessage(), null);
}



return new CheckResult<>(HttpStatus.OK, "", new Pair<>(testEntity, projectEntity));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ public void testUpdateTest() throws Exception {
eq(dockerImage),
eq(dockerTestScript),
eq(dockerTestTemplate),
eq(structureTemplate),
eq(HttpMethod.POST)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(null, project)));

Expand Down Expand Up @@ -219,6 +220,7 @@ public void testUpdateTest() throws Exception {
eq(null),
eq(null),
eq(null),
eq(null),
eq(HttpMethod.POST)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(null, project)));

Expand Down Expand Up @@ -268,6 +270,7 @@ public void testUpdateTest() throws Exception {
eq(dockerImage),
eq(dockerTestScript),
eq(dockerTestTemplate),
eq(structureTemplate),
eq(HttpMethod.POST)
)).thenReturn(new CheckResult<>(HttpStatus.I_AM_A_TEAPOT, "I'm a teapot", null));

Expand Down Expand Up @@ -326,6 +329,7 @@ public void testPutTest() throws Exception {
eq(dockerImage),
eq(dockerTestScript),
eq(dockerTestTemplate),
eq(structureTemplate),
eq(HttpMethod.PUT)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project)));

Expand Down Expand Up @@ -380,6 +384,7 @@ public void testPutTest() throws Exception {
eq(null),
eq(null),
eq(null),
eq(null),
eq(HttpMethod.PUT)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project)));

Expand Down Expand Up @@ -449,6 +454,7 @@ public void testPutTest() throws Exception {
eq(dockerImage),
eq(dockerTestScript),
eq(dockerTestTemplate),
eq(structureTemplate),
eq(HttpMethod.PUT)
)).thenReturn(new CheckResult<>(HttpStatus.I_AM_A_TEAPOT, "I'm a teapot", null));

Expand Down Expand Up @@ -482,6 +488,7 @@ public void testGetPatch() throws Exception {
eq(dockerImage),
eq(null),
eq(null),
eq(null),
eq(HttpMethod.PATCH)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project)));

Expand Down Expand Up @@ -523,6 +530,7 @@ public void testGetPatch() throws Exception {
eq(null),
eq(dockerTestScript),
eq(null),
eq(null),
eq(HttpMethod.PATCH)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project)));

Expand Down Expand Up @@ -555,6 +563,7 @@ public void testGetPatch() throws Exception {
eq(null),
eq(null),
eq(dockerTestTemplate),
eq(null),
eq(HttpMethod.PATCH)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project)));

Expand Down Expand Up @@ -587,6 +596,7 @@ public void testGetPatch() throws Exception {
eq(null),
eq(null),
eq(null),
eq(structureTemplate),
eq(HttpMethod.PATCH)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project)));

Expand Down Expand Up @@ -619,6 +629,7 @@ public void testGetPatch() throws Exception {
eq(dockerImage),
eq(null),
eq(null),
eq(null),
eq(HttpMethod.PATCH)
)).thenReturn(new CheckResult<>(HttpStatus.I_AM_A_TEAPOT, "I'm a teapot", null));

Expand Down Expand Up @@ -679,6 +690,7 @@ public void testDeleteTest() throws Exception {
eq(null),
eq(null),
eq(null),
eq(null),
eq(HttpMethod.DELETE)
)).thenReturn(new CheckResult<>(HttpStatus.OK, "", new Pair<>(test, project)));

Expand All @@ -700,6 +712,7 @@ public void testDeleteTest() throws Exception {
eq(null),
eq(null),
eq(null),
eq(null),
eq(HttpMethod.DELETE)
)).thenReturn(new CheckResult<>(HttpStatus.I_AM_A_TEAPOT, "I'm a teapot", null));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.ugent.pidgeon.docker;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.ugent.pidgeon.model.submissionTesting.DockerSubmissionTestModel;
Expand Down Expand Up @@ -202,13 +204,15 @@ void dockerImageDoesNotExist(){
}

@Test
void isValidTemplate(){
assertFalse(DockerSubmissionTestModel.isValidTemplate("This is not a valid template"));
assertTrue(DockerSubmissionTestModel.isValidTemplate("@HelloWorld\n" +
void tryTemplate(){
assertThrows(IllegalArgumentException.class,() -> DockerSubmissionTestModel.tryTemplate("This is not a valid template"));


assertDoesNotThrow(() -> DockerSubmissionTestModel.tryTemplate("@HelloWorld\n" +
">Description=\"Test for hello world!\"\n" +
">Required\n" +
"HelloWorld!"));
assertTrue(DockerSubmissionTestModel.isValidTemplate("@helloworld\n"
assertDoesNotThrow(() -> DockerSubmissionTestModel.tryTemplate("@helloworld\n"
+ ">required\n"
+ ">description=\"Helloworldtest\"\n"
+ "Hello World\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
import java.nio.file.Files;
import java.nio.file.Path;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class FileStructureTest {
Expand Down Expand Up @@ -43,6 +45,17 @@ void isEmpty(){
void denyFileExtension(){
assertFalse(runTest("noClassExtensions"));
}

@Test
void tryTemplateTest(){
assertDoesNotThrow(() -> SubmissionTemplateModel.tryTemplate("template"));
assertDoesNotThrow(() -> SubmissionTemplateModel.tryTemplate("src/\n index.js\n seconfilehehe.file"));
assertThrows(IllegalArgumentException.class, () -> SubmissionTemplateModel.tryTemplate("src/\n index.js\n seconfilehehe.file\n thirdfile"));
//check trailing newline
assertDoesNotThrow(() -> SubmissionTemplateModel.tryTemplate("src/\n\tindex.js\n"));

}

private boolean runTest(String testpath){
SubmissionTemplateModel model = new SubmissionTemplateModel();
if(testpath.lastIndexOf('/') != testpath.length() - 1){
Expand Down
Loading
Loading