Merge e8805a5 into 355a8b0

build.gradle

@@ -204,6 +204,14 @@ dependencyManagement {
   }
 }
 
+test {
+  useJUnitPlatform {
+  }
+  testLogging {
+    events("passed", "skipped", "failed")
+  }
+}
+
 dependencies {
   compileOnly("org.projectlombok:lombok:${lombokVersion}")
   annotationProcessor("org.projectlombok:lombok:${lombokVersion}")
@@ -251,7 +259,6 @@ dependencies {
   compile("org.luaj:luaj-jse:${luajVersion}")
   compile("com.github.micheljung:nocatch:${nocatchVersion}")
   compile("junit-addons:junit-addons:${junitAddonsVersion}")
-  compile("com.googlecode.zohhak:zohhak:${zohhakVersion}")
   compile("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${jacksonDatatypeJsr310Version}")
   compile("com.mandrillapp.wrapper.lutung:lutung:${lutungVersion}")
   compile("org.apache.commons:commons-compress:${commonsCompressVersion}")
@@ -265,9 +272,17 @@ dependencies {
   testCompile("org.springframework.boot:spring-boot-starter-test")
   testCompile("org.springframework.restdocs:spring-restdocs-mockmvc")
   testCompile("org.springframework.security:spring-security-test")
+  testImplementation("junit:junit")
+  testImplementation("org.junit.jupiter:junit-jupiter:${jUnit5Version}")
+  testImplementation("org.junit.jupiter:junit-jupiter-params:${jUnit5Version}")
+  testImplementation("org.mockito:mockito-junit-jupiter:${mockitoVersion}")
+  testRuntimeOnly("org.junit.vintage:junit-vintage-engine:${jUnit5Version}")
   testCompile("com.h2database:h2:${h2Version}")
   testCompile("com.jayway.jsonpath:json-path:${jsonPath}")
   testCompile("com.jayway.jsonpath:json-path-assert:${jsonPathAssert}")
 
   codacy("com.github.codacy:codacy-coverage-reporter:${codacyCoverageReporterVersion}")
 }
+
+ext["mockito.version"] = "${mockitoVersion}"
+ext["junit-jupiter.version"] = "${jUnit5Version}"

gradle.properties

@@ -16,13 +16,12 @@ springBootAdminClientVersion=2.0.6
 luajVersion=3.0.1
 nocatchVersion=1.1
 junitAddonsVersion=1.4
-zohhakVersion=1.1.1
 githubApiVersion=1.84
 jgitVersionn=4.5.0.201609210915-r
 fafCommonsVersion=8860b91752f6b5713926036dbb4740197d27211b
 h2Version=1.4.193
 jacksonDatatypeJsr310Version=2.9.7
-mockitoVersion=2.19.0
+mockitoVersion=2.23.4
 lutungVersion=0.0.7
 commonsCompressVersion=1.13
 jsonPath=2.2.0
@@ -32,3 +31,4 @@ codacyCoverageReporterVersion=4.0.0
 jsonVersion=20180813
 javaxInterceptorApiVersion=1.2
 lombokVersion=1.18.4
+jUnit5Version=5.5.1

src/main/java/com/faforever/api/config/security/WebSecurityConfig.java

@@ -24,7 +24,6 @@
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.web.servlet.config.annotation.CorsRegistry;
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
-import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
 
 import javax.inject.Inject;
 import javax.servlet.http.HttpServletRequest;
@@ -91,7 +90,7 @@ public boolean matches(HttpServletRequest request) {
 
   @Bean
   public WebMvcConfigurer corsConfigurer() {
-    return new WebMvcConfigurerAdapter() {
+    return new WebMvcConfigurer() {
       @Override
       public void addCorsMappings(CorsRegistry registry) {
         registry.addMapping("/**")

src/main/java/com/faforever/api/error/ApiException.java

@@ -15,8 +15,12 @@ public ApiException(Error error) {
     this(new Error[]{error});
   }
 
-  public ApiException(Error[] errors) {
+  public ApiException(Error... errors) {
     super(Arrays.toString(errors));
     this.errors = errors;
   }
+
+  public static ApiException of(ErrorCode errorCode, Object... args) {
+    return new ApiException(new Error(errorCode, args));
+  }
 }

src/main/java/com/faforever/api/error/ErrorCode.java

@@ -90,9 +90,12 @@ public enum ErrorCode {
   VOTING_SUBJECT_DOES_NOT_EXIST(180, "Voting subject does not exist", "There is no voting subject with the ID ''{0}''."),
   VOTING_CHOICE_DOES_NOT_EXIST(181, "Invalid choice", "There is no voting choice with the ID ''{0}''."),
   STEAM_ID_ALREADY_LINKED(182, " Steam account already linked to a FAF account", "You linked this account already to user with name ''{0}''."),
-  MAP_NAME_INVALID(183, "Map name invalid", "The name of the map in the scenario file can only contain printable ASCII characters and blanks."),
-  MOD_NAME_INVALID(184, "Mod name invalid", "The name of the mod in the scenario file can only contain printable ASCII characters and blanks.");
-
+  MAP_NAME_INVALID_CHARACTER(183, "Map name invalid", "Only latin characters, numbers, blanks and the minus are allowed."),
+  MOD_NAME_INVALID(184, "Mod name invalid", "The name of the mod in the scenario file can only contain printable ASCII characters and blanks."),
+  MAP_NAME_INVALID_MINUS_OCCURENCE(185, "Map name invalid", "Only a maximum of {0} minus characters are allowed."),
+  MAP_SCRIPT_LINE_MISSING(186, "Missing scenario.lua line", "The scenario.lua has to contain the following line: {0}"),
+  MAP_NAME_TOO_SHORT(187, "Map name invalid", "The map name must have at least {0, number} characters, was: {1, number}"),
+  MAP_NAME_DOES_NOT_START_WITH_LETTER(188, "Map name invalid", "The map name has to begin with a letter");
 
   private final int code;
   private final String title;

src/main/java/com/faforever/api/map/MapService.java

@@ -15,57 +15,136 @@
 import com.faforever.commons.io.Zipper;
 import com.faforever.commons.lua.LuaLoader;
 import com.faforever.commons.map.PreviewGenerator;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import lombok.AllArgsConstructor;
 import lombok.Data;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.compress.archivers.ArchiveException;
 import org.luaj.vm2.LuaValue;
 import org.springframework.cache.annotation.CacheEvict;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.Assert;
 import org.springframework.util.FileSystemUtils;
+import org.springframework.util.StringUtils;
 
 import javax.imageio.ImageIO;
-import javax.inject.Inject;
 import java.awt.image.BufferedImage;
-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
+import java.io.IOException;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
+import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import java.util.zip.ZipInputStream;
-import java.util.zip.ZipOutputStream;
 
 import static com.github.nocatch.NoCatch.noCatch;
+import static java.text.MessageFormat.format;
 
 @Service
 @Slf4j
+@AllArgsConstructor
 public class MapService {
+  private static final Pattern MAP_NAME_INVALID_CHARACTER_PATTERN = Pattern.compile("[a-zA-Z0-9\\- ]+");
+  private static final Pattern MAP_NAME_DOES_NOT_START_WITH_LETTER_PATTERN = Pattern.compile("^[^a-zA-Z]+");
+  private static final Pattern ADAPTIVE_MAP_PATTERN = Pattern.compile("AdaptiveMap\\w=\\wtrue");
+  private static final int MAP_NAME_MINUS_MAX_OCCURENCE = 3;
+  private static final int MAP_NAME_MIN_LENGTH = 4;
+  private static final int MAP_NAME_MAX_LENGTH = 50;
+
   private static final String[] REQUIRED_FILES = new String[]{
     ".scmap",
     "_save.lua",
     "_scenario.lua",
-    "_script.lua"};
+    "_script.lua",
+  };
+
+  private static final String[] ADAPTIVE_REQUIRED_FILES = new String[]{
+    "_options.lua",
+    "_tables.lua",
+  };
+
   private static final Charset MAP_CHARSET = StandardCharsets.ISO_8859_1;
   private static final String STUPID_MAP_FOLDER_PREFIX = "maps/";
-  public static final int MAP_DISPLAY_NAME_MAX_LENGTH = 100;
+  private static final int MAP_DISPLAY_NAME_MAX_LENGTH = 100;
   private final FafApiProperties fafApiProperties;
   private final MapRepository mapRepository;
   private final ContentService contentService;
 
-  @Inject
-  public MapService(FafApiProperties fafApiProperties, MapRepository mapRepository, ContentService contentService) {
-    this.fafApiProperties = fafApiProperties;
-    this.mapRepository = mapRepository;
-    this.contentService = contentService;
+  @VisibleForTesting
+  void validate(MapValidationRequest mapValidationRequest) {
+    String mapName = mapValidationRequest.getName();
+    Assert.notNull(mapName, "The map name is mandatory.");
+    String scenarioLua = mapValidationRequest.getScenarioLua();
+
+    Collection<Error> mapNameErrors = validateMapName(mapName);
+    if (!mapNameErrors.isEmpty()) {
+      throw new ApiException(mapNameErrors.toArray(new Error[0]));
+    }
+
+    if (scenarioLua != null) {
+      Collection<Error> scenarioLuaErrors = validateScenarioLua(mapName, scenarioLua);
+      if (!scenarioLuaErrors.isEmpty()) {
+        throw new ApiException(scenarioLuaErrors.toArray(new Error[0]));
+      }
+    }
+  }
+
+  private Collection<Error> validateMapName(String mapName) {
+    List<Error> errors = new ArrayList<>();
+
+    if (!MAP_NAME_INVALID_CHARACTER_PATTERN.matcher(mapName).matches()) {
+      errors.add(new Error(ErrorCode.MAP_NAME_INVALID_CHARACTER));
+    }
+
+    if (mapName.length() < MAP_NAME_MIN_LENGTH) {
+      errors.add(new Error(ErrorCode.MAP_NAME_TOO_SHORT, MAP_NAME_MIN_LENGTH, mapName.length()));
+    }
+
+    if (mapName.length() > MAP_NAME_MAX_LENGTH) {
+      errors.add(new Error(ErrorCode.MAP_NAME_TOO_LONG, MAP_NAME_MAX_LENGTH, mapName.length()));
+    }
+
+    if (StringUtils.countOccurrencesOf(mapName, "-") > MAP_NAME_MINUS_MAX_OCCURENCE) {
+      errors.add(new Error(ErrorCode.MAP_NAME_INVALID_MINUS_OCCURENCE, MAP_NAME_MINUS_MAX_OCCURENCE));
+    }
+
+    if (MAP_NAME_DOES_NOT_START_WITH_LETTER_PATTERN.matcher(mapName).find()) {
+      errors.add(new Error(ErrorCode.MAP_NAME_DOES_NOT_START_WITH_LETTER));
+    }
+
+    return errors;
+  }
+
+  private Collection<Error> validateScenarioLua(String validMapName, String scenarioLua) {
+    List<Error> errors = new ArrayList<>();
+
+    java.util.Map<String, String> declarations = ImmutableMap.<String, String>builder()
+      .put("map", ".scmap")
+      .put("save", "_save.lua")
+      .put("script", "_script.lua")
+      .build();
+
+    for (Entry entry : declarations.entrySet()) {
+      Pattern pattern = Pattern.compile(format("{0}\\s=\\s''\\/maps\\/{2}(\\.v\\d{4})?\\/{2}{1}'',", entry.getKey(), entry.getValue(), validMapName));
+      if (!pattern.matcher(scenarioLua).find()) {
+        errors.add(new Error(ErrorCode.MAP_SCRIPT_LINE_MISSING, format(
+          "{0} = ''/maps/{2}/{2}{1}'',", entry.getKey(), entry.getValue(), validMapName
+        )));
+      }
+    }
+
+    return errors;
   }
 
   @Transactional
@@ -103,20 +182,17 @@ public void uploadMap(byte[] mapData, String mapFilename, Player author, boolean
     }
   }
 
-  @SneakyThrows
-  private Path copyToTemporaryDirectory(byte[] mapData, MapUploadData progressData) {
+  private Path copyToTemporaryDirectory(byte[] mapData, MapUploadData progressData) throws IOException {
     return Files.write(progressData.getUploadedFile(), mapData);
   }
 
-  @SneakyThrows
-  private void unzipFile(MapUploadData mapData) {
-      Unzipper.from(mapData.getUploadedFile())
-        .to(mapData.getBaseDir())
-        .unzip();
+  private void unzipFile(MapUploadData mapData) throws IOException, ArchiveException {
+    Unzipper.from(mapData.getUploadedFile())
+      .to(mapData.getBaseDir())
+      .unzip();
   }
 
-  @SneakyThrows
-  private void postProcessZipFiles(MapUploadData mapUploadData) {
+  private void postProcessZipFiles(MapUploadData mapUploadData) throws IOException {
     Optional<Path> mapFolder;
     try (Stream<Path> mapFolderStream = Files.list(mapUploadData.getBaseDir())) {
       mapFolder = mapFolderStream
@@ -150,8 +226,7 @@ private void postProcessZipFiles(MapUploadData mapUploadData) {
     }
   }
 
-  @SneakyThrows
-  private void parseScenarioLua(MapUploadData progressData) {
+  private void parseScenarioLua(MapUploadData progressData) throws IOException {
     try (Stream<Path> mapFilesStream = Files.list(progressData.getOriginalMapFolder())) {
       Path scenarioLuaPath = noCatch(() -> mapFilesStream)
         .filter(myFile -> myFile.toString().endsWith("_scenario.lua"))
@@ -173,7 +248,7 @@ private void checkLua(MapUploadData progressData) {
       throw new ApiException(new Error(ErrorCode.MAP_NAME_TOO_LONG, MAP_DISPLAY_NAME_MAX_LENGTH, displayName.length()));
     }
     if (!NameUtil.isPrintableAsciiString(displayName)) {
-      throw new ApiException(new Error(ErrorCode.MAP_NAME_INVALID));
+      throw new ApiException(new Error(ErrorCode.MAP_NAME_INVALID_CHARACTER));
     }
 
     if (scenarioInfo.get(ScenarioMapInfo.DESCRIPTION) == LuaValue.NIL) {
@@ -288,15 +363,13 @@ private void updateMapEntities(MapUploadData progressData) {
     mapRepository.save(map);
   }
 
-  @SneakyThrows
-  private void renameFolderNameAndCorrectPathInLuaFiles(MapUploadData progressData) {
+  private void renameFolderNameAndCorrectPathInLuaFiles(MapUploadData progressData) throws IOException {
     progressData.setNewMapFolder(progressData.getBaseDir().resolve(progressData.getNewFolderName()));
     Files.move(progressData.getOriginalMapFolder(), progressData.getNewMapFolder());
     updateLuaFiles(progressData);
   }
 
-  @SneakyThrows
-  private void updateLuaFiles(MapUploadData mapData) {
+  private void updateLuaFiles(MapUploadData mapData) throws IOException {
     String oldNameFolder = "/maps/" + mapData.getUploadFolderName();
     String newNameFolder = "/maps/" + mapData.getNewFolderName();
     try (Stream<Path> mapFileStream = Files.list(mapData.getNewMapFolder())) {
@@ -311,9 +384,7 @@ private void updateLuaFiles(MapUploadData mapData) {
     }
   }
 
-
-  @SneakyThrows
-  private void generatePreview(MapUploadData mapData) {
+  private void generatePreview(MapUploadData mapData) throws IOException {
     String previewFilename = mapData.getNewFolderName() + ".png";
     generateImage(
       fafApiProperties.getMap().getDirectoryPreviewPathSmall().resolve(previewFilename),
@@ -326,8 +397,7 @@ private void generatePreview(MapUploadData mapData) {
       fafApiProperties.getMap().getPreviewSizeLarge());
   }
 
-  @SneakyThrows
-  private void zipMapData(MapUploadData progressData) {
+  private void zipMapData(MapUploadData progressData) throws IOException, ArchiveException {
     cleanupBaseDir(progressData);
     Path finalZipFile = progressData.getFinalZipFile();
     Files.createDirectories(finalZipFile.getParent(), FilePermissionUtil.directoryPermissionFileAttributes());
@@ -339,8 +409,7 @@ private void zipMapData(MapUploadData progressData) {
     FilePermissionUtil.setDefaultFilePermission(finalZipFile);
   }
 
-  @SneakyThrows
-  private void cleanupBaseDir(MapUploadData progressData) {
+  private void cleanupBaseDir(MapUploadData progressData) throws IOException {
     Files.delete(progressData.getUploadedFile());
     try (Stream<Path> stream = Files.list(progressData.getBaseDir())) {
       if (stream.count() != 1) {
@@ -349,8 +418,7 @@ private void cleanupBaseDir(MapUploadData progressData) {
     }
   }
 
-  @SneakyThrows
-  private void generateImage(Path target, Path baseDir, int size) {
+  private void generateImage(Path target, Path baseDir, int size) throws IOException {
     BufferedImage image = PreviewGenerator.generatePreview(baseDir, size, size);
     if (target.getNameCount() > 0) {
       Files.createDirectories(target.getParent(), FilePermissionUtil.directoryPermissionFileAttributes());

src/main/java/com/faforever/api/map/MapValidationRequest.java

@@ -0,0 +1,9 @@
+package com.faforever.api.map;
+
+import lombok.Value;
+
+@Value
+public class MapValidationRequest {
+  String name;
+  String scenarioLua;
+}