filename filter maps

src/main/java/com/faforever/api/avatar/AvatarService.java

@@ -8,8 +8,8 @@
 import com.faforever.api.error.NotFoundApiException;
 import com.faforever.api.error.ProgrammingError;
 import com.faforever.api.security.Audit;
-import com.faforever.api.utils.FileNameUtil;
 import com.faforever.api.utils.FilePermissionUtil;
+import com.faforever.api.utils.NameUtil;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
 import org.jetbrains.annotations.NotNull;
@@ -46,7 +46,7 @@ public AvatarService(AvatarRepository avatarRepository, FafApiProperties propert
   @Audit(messageTemplate = "Avatar [''{0}'' - ''{1}''] created.", expressions = {"${avatarMetadata.name}", "${originalFilename}"})
   public void createAvatar(AvatarMetadata avatarMetadata, String originalFilename, InputStream imageDataInputStream, long avatarImageFileSize) {
     final Avatar avatarToCreate = new Avatar();
-    final String normalizedAvatarFileName = FileNameUtil.normalizeFileName(originalFilename);
+    final String normalizedAvatarFileName = NameUtil.normalizeFileName(originalFilename);
     String url = String.format(properties.getAvatar().getDownloadUrlFormat(), normalizedAvatarFileName);
     avatarRepository.findOneByUrl(url).ifPresent(existingAvatar -> {
       throw new ApiException(new Error(ErrorCode.AVATAR_NAME_CONFLICT, normalizedAvatarFileName));

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

@@ -89,7 +89,10 @@ public enum ErrorCode {
   CAN_NOT_REVEAL_RESULTS_WHEN_VOTING_IS_NOT_FINISHED(179, "Vote still ongoing", "You can reveal results when voting is ongoing. Please set reveal results only after voting finished."),
   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}''.");
+  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.");
+
 
   private final int code;
   private final String title;
@@ -104,5 +107,4 @@ public enum ErrorCode {
   public String codeAsString() {
     return String.valueOf(code);
   }
-
 }

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

@@ -10,6 +10,7 @@
 import com.faforever.api.error.ErrorCode;
 import com.faforever.api.error.ProgrammingError;
 import com.faforever.api.utils.FilePermissionUtil;
+import com.faforever.api.utils.NameUtil;
 import com.faforever.commons.io.Unzipper;
 import com.faforever.commons.io.Zipper;
 import com.faforever.commons.lua.LuaLoader;
@@ -55,6 +56,7 @@ public class MapService {
     "_script.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 final FafApiProperties fafApiProperties;
   private final MapRepository mapRepository;
   private final ContentService contentService;
@@ -73,6 +75,8 @@ public void uploadMap(byte[] mapData, String mapFilename, Player author, boolean
     Assert.notNull(author, "'author' must not be null");
     Assert.isTrue(mapData.length > 0, "'mapData' must not be empty");
 
+    mapFilename = NameUtil.normalizeFileName(mapFilename);
+
     MapUploadData progressData = new MapUploadData()
       .setBaseDir(contentService.createTempDir())
       .setUploadFileName(mapFilename)
@@ -82,20 +86,23 @@ public void uploadMap(byte[] mapData, String mapFilename, Player author, boolean
     progressData.setUploadedFile(progressData.getBaseDir().resolve(mapFilename));
     copyToTemporaryDirectory(mapData, progressData);
 
-    unzipFile(progressData);
-    postProcessZipFiles(progressData);
+    try {
+      unzipFile(progressData);
+      postProcessZipFiles(progressData);
 
-    parseScenarioLua(progressData);
-    checkLua(progressData);
-    postProcessLuaFile(progressData);
+      parseScenarioLua(progressData);
+      checkLua(progressData);
+      postProcessLuaFile(progressData);
 
-    updateMapEntities(progressData);
+      updateMapEntities(progressData);
 
-    renameFolderNameAndCorrectPathInLuaFiles(progressData);
-    generatePreview(progressData);
+      renameFolderNameAndCorrectPathInLuaFiles(progressData);
+      generatePreview(progressData);
 
-    zipMapData(progressData);
-    assert cleanup(progressData);
+      zipMapData(progressData);
+    } finally {
+      cleanup(progressData);
+    }
   }
 
   @SneakyThrows
@@ -164,6 +171,14 @@ private void checkLua(MapUploadData progressData) {
     if (scenarioInfo.get(ScenarioMapInfo.NAME) == LuaValue.NIL) {
       errors.add(new Error(ErrorCode.MAP_NAME_MISSING));
     }
+    String displayName = scenarioInfo.get(ScenarioMapInfo.NAME).tojstring();
+    if (displayName.length() > MAP_DISPLAY_NAME_MAX_LENGTH) {
+      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));
+    }
+
     if (scenarioInfo.get(ScenarioMapInfo.DESCRIPTION) == LuaValue.NIL) {
       errors.add(new Error(ErrorCode.MAP_DESCRIPTION_MISSING));
     }
@@ -238,7 +253,11 @@ private void updateMapEntities(MapUploadData progressData) {
     if (map == null) {
       map = new Map();
     }
-    map.setDisplayName(scenarioInfo.get(ScenarioMapInfo.NAME).toString())
+
+    String name = scenarioInfo.get(ScenarioMapInfo.NAME).toString();
+
+
+    map.setDisplayName(name)
       .setMapType(scenarioInfo.get(ScenarioMapInfo.TYPE).tojstring())
       .setBattleType(scenarioInfo.get(ScenarioMapInfo.CONFIGURATIONS).get(ScenarioMapInfo.CONFIGURATION_STANDARD).get(ScenarioMapInfo.CONFIGURATION_STANDARD_TEAMS).get(1)
         .get(ScenarioMapInfo.CONFIGURATION_STANDARD_TEAMS_NAME).tojstring())

src/main/java/com/faforever/api/mod/ModService.java

@@ -8,8 +8,8 @@
 import com.faforever.api.error.ApiException;
 import com.faforever.api.error.Error;
 import com.faforever.api.error.ErrorCode;
-import com.faforever.api.utils.FileNameUtil;
 import com.faforever.api.utils.FilePermissionUtil;
+import com.faforever.api.utils.NameUtil;
 import com.faforever.commons.mod.ModReader;
 import com.google.common.primitives.Ints;
 import lombok.SneakyThrows;
@@ -145,8 +145,13 @@ private void validateModInfo(com.faforever.commons.mod.Mod modInfo) {
     String name = modInfo.getName();
     if (name == null) {
       errors.add(new Error(ErrorCode.MOD_NAME_MISSING));
-    } else if (name.length() > properties.getMod().getMaxNameLength()) {
-      errors.add(new Error(ErrorCode.MOD_NAME_TOO_LONG));
+    } else {
+      if (name.length() > properties.getMod().getMaxNameLength()) {
+        errors.add(new Error(ErrorCode.MOD_NAME_TOO_LONG, properties.getMod().getMaxNameLength(), name.length()));
+      }
+      if (!NameUtil.isPrintableAsciiString(name)) {
+        errors.add(new Error(ErrorCode.MOD_NAME_INVALID));
+      }
     }
     if (modInfo.getUid() == null) {
       errors.add(new Error(ErrorCode.MOD_UID_MISSING));
@@ -203,7 +208,7 @@ private String generateZipFileName(String displayName, short version) {
   }
 
   private String generateFolderName(String displayName, short version) {
-    return String.format("%s.v%04d", FileNameUtil.normalizeFileName(displayName), version);
+    return String.format("%s.v%04d", NameUtil.normalizeFileName(displayName), version);
   }
 
   private void store(com.faforever.commons.mod.Mod modInfo, Optional<Path> thumbnailPath, Player uploader, String zipFileName) {

src/main/java/com/faforever/api/utils/NameUtil.java

@@ -0,0 +1,34 @@
+package com.faforever.api.utils;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.nio.file.Paths;
+import java.text.Normalizer;
+import java.text.Normalizer.Form;
+import java.util.Locale;
+
+public final class NameUtil {
+  private NameUtil() {
+    // Utility class
+  }
+
+  /**
+   * Returns true if the string only consists of all printable ASCII characters (see ASCII table from SP to ~ [SP being
+   * space])
+   */
+  public static boolean isPrintableAsciiString(String name) {
+    return !name.matches(".*[^\\x20-\\x7E]+.*");
+  }
+
+  public static String normalizeFileName(String originalFileName) {
+    return StringUtils.strip(
+      Normalizer.normalize(
+        Paths.get(originalFileName
+          .replaceAll("[/\\\\ :*?<>|\"]", "_")
+          .toLowerCase(Locale.US))
+          .getFileName()
+          .toString(),
+        Form.NFKC),
+      "_");
+  }
+}

src/test/java/com/faforever/api/avatar/AvatarServiceTest.java

@@ -6,6 +6,7 @@
 import com.faforever.api.error.ApiExceptionWithCode;
 import com.faforever.api.error.ErrorCode;
 import com.faforever.api.error.NotFoundApiException;
+import com.faforever.api.utils.NameUtil;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -93,14 +94,16 @@ public void newAvatarUploadWithValidName() throws Exception {
 
     final URL imageResource = loadResource(avatarFileName);
     try (final InputStream imageInputStream = imageResource.openStream()) {
-      avatarService.createAvatar(AVATAR_METADATA, "[./><" + avatarFileName, imageInputStream, VALID_FILE_SIZE);
+      String worstCasePrefix = "[./><";
+      avatarService.createAvatar(AVATAR_METADATA, worstCasePrefix + avatarFileName, imageInputStream, VALID_FILE_SIZE);
       ArgumentCaptor<Avatar> avatarCaptor = ArgumentCaptor.forClass(Avatar.class);
       verify(avatarRepository, times(1)).save(avatarCaptor.capture());
 
       final Avatar storedAvatar = avatarCaptor.getValue();
-      assertEquals(String.format(DOWNLOAD_URL_FORMAT, avatarFileName), storedAvatar.getUrl());
+      String expectedFilename = NameUtil.normalizeFileName(worstCasePrefix + avatarFileName);
+      assertEquals(String.format(DOWNLOAD_URL_FORMAT, expectedFilename), storedAvatar.getUrl());
       assertEquals(AVATAR_NAME, storedAvatar.getTooltip());
-      assertThat(avatarsPath.resolve(avatarFileName).toFile().length(), is(imageResource.openConnection().getContentLengthLong()));
+      assertThat(avatarsPath.resolve(expectedFilename).toFile().length(), is(imageResource.openConnection().getContentLengthLong()));
     }
   }
 
@@ -229,6 +232,7 @@ public void deleteAvatar() throws Exception {
     verify(avatarRepository, times(1)).delete(avatarToDelete);
     assertThat(avatarFilePath.toFile().exists(), is(false));
   }
+
   @Test
   public void deleteNotExistingAvatar() throws Exception {
     when(avatarRepository.findById(AVATAR_ID)).thenReturn(Optional.empty());

src/test/java/com/faforever/api/map/MapServiceTest.java

@@ -221,6 +221,24 @@ public void mapWithMoreRootFoldersInZip() throws IOException {
     }
   }
 
+  @Test
+  public void uploadMapWithInvalidCharactersName() throws IOException {
+    String zipFilename = "scmp_037_no_ascii.zip";
+    when(mapRepository.findOneByDisplayName(any())).thenReturn(Optional.empty());
+    try (InputStream inputStream = loadMapResourceAsStream(zipFilename)) {
+      byte[] mapData = ByteStreams.toByteArray(inputStream);
+
+      Path tmpDir = temporaryDirectory.getRoot().toPath();
+      try {
+        instance.uploadMap(mapData, zipFilename, author, true);
+      } catch (ApiException e) {
+        assertThat(e, apiExceptionWithCode(ErrorCode.MAP_NAME_INVALID));
+      }
+
+      verify(mapRepository, never()).save(any(com.faforever.api.data.domain.Map.class));
+    }
+  }
+
   @Test
   public void positiveUploadTest() throws IOException {
     String zipFilename = "scmp_037.zip";

src/test/java/com/faforever/api/utils/NameUtilTest.java

@@ -0,0 +1,23 @@
+package com.faforever.api.utils;
+
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+public class NameUtilTest {
+  @Test
+  public void testIsPrintableAsciiString() {
+    final boolean positiveResult = NameUtil.isPrintableAsciiString(" !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~");
+    assertThat(positiveResult, is(true));
+
+    final boolean negativeResult = NameUtil.isPrintableAsciiString("Hello this is ä test.");
+    assertThat(negativeResult, is(false));
+  }
+
+  @Test
+  public void generateFileName() throws Exception {
+    final String fileName = NameUtil.normalizeFileName(" :*?<>|\"{}:[]la/ \\la??");
+    assertThat(fileName, is("{}_[]la___la"));
+  }
+}