From 6e92ec3bc34db6b0eeb1cae161a8b95fa807be1f Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Sun, 18 Aug 2019 23:46:11 +0200 Subject: [PATCH] MapUpload: Add validation endpoint / form for map name and scenario.lua --- .../config/security/WebSecurityConfig.java | 3 +- .../com/faforever/api/error/ApiException.java | 6 +- .../com/faforever/api/error/ErrorCode.java | 7 +- .../com/faforever/api/map/MapService.java | 81 +++++++- .../api/map/MapValidationRequest.java | 9 + .../com/faforever/api/map/MapsController.java | 31 ++- src/main/resources/config/application-dev.yml | 3 +- src/main/resources/templates/login.html | 4 +- .../templates/oauth_confirm_access.html | 4 +- .../templates/validate_map_metadata.html | 184 ++++++++++++++++++ .../com/faforever/api/map/MapServiceTest.java | 2 +- 11 files changed, 308 insertions(+), 26 deletions(-) create mode 100644 src/main/java/com/faforever/api/map/MapValidationRequest.java create mode 100644 src/main/resources/templates/validate_map_metadata.html diff --git a/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java b/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java index b6cf75d40..802e16b36 100644 --- a/src/main/java/com/faforever/api/config/security/WebSecurityConfig.java +++ b/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("/**") diff --git a/src/main/java/com/faforever/api/error/ApiException.java b/src/main/java/com/faforever/api/error/ApiException.java index 4aa304289..5f4fdc113 100644 --- a/src/main/java/com/faforever/api/error/ApiException.java +++ b/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)); + } } diff --git a/src/main/java/com/faforever/api/error/ErrorCode.java b/src/main/java/com/faforever/api/error/ErrorCode.java index e66c73569..d2df417f5 100644 --- a/src/main/java/com/faforever/api/error/ErrorCode.java +++ b/src/main/java/com/faforever/api/error/ErrorCode.java @@ -90,9 +90,10 @@ 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}"); private final int code; private final String title; diff --git a/src/main/java/com/faforever/api/map/MapService.java b/src/main/java/com/faforever/api/map/MapService.java index f61fe549b..946e6c102 100644 --- a/src/main/java/com/faforever/api/map/MapService.java +++ b/src/main/java/com/faforever/api/map/MapService.java @@ -15,6 +15,8 @@ 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; @@ -26,6 +28,7 @@ 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 java.awt.image.BufferedImage; @@ -37,22 +40,37 @@ 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 static com.github.nocatch.NoCatch.noCatch; +import static java.text.MessageFormat.format; @Service @Slf4j @AllArgsConstructor public class MapService { + private static final Pattern MAP_NAME_VALIDATION_PATTERN = Pattern.compile("[a-zA-Z0-9\\- ]+"); + 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 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/"; private static final int MAP_DISPLAY_NAME_MAX_LENGTH = 100; @@ -60,6 +78,59 @@ public class MapService { private final MapRepository mapRepository; private final ContentService contentService; + @VisibleForTesting + void validate(MapValidationRequest mapValidationRequest) { + String mapName = mapValidationRequest.getName(); + Assert.notNull(mapName, "The map name is mandatory."); + String scenarioLua = mapValidationRequest.getScenarioLua(); + + Collection mapNameErrors = validateMapName(mapName); + if (!mapNameErrors.isEmpty()) { + throw new ApiException(mapNameErrors.toArray(new Error[0])); + } + + if (scenarioLua != null) { + Collection scenarioLuaErrors = validateScenarioLua(mapName, scenarioLua); + if (!scenarioLuaErrors.isEmpty()) { + throw new ApiException(scenarioLuaErrors.toArray(new Error[0])); + } + } + } + + private Collection validateMapName(String mapName) { + List errors = new ArrayList<>(); + + if (!MAP_NAME_VALIDATION_PATTERN.matcher(mapName).matches()) { + errors.add(new Error(ErrorCode.MAP_NAME_INVALID_CHARACTER)); + } + + if (StringUtils.countOccurrencesOf(mapName, "-") > MAP_NAME_MINUS_MAX_OCCURENCE) { + errors.add(new Error(ErrorCode.MAP_NAME_INVALID_MINUS_OCCURENCE, MAP_NAME_MINUS_MAX_OCCURENCE)); + } + + return errors; + } + + private Collection validateScenarioLua(String validMapName, String scenarioLua) { + List errors = new ArrayList<>(); + + java.util.Map declarations = ImmutableMap.builder() + .put("map", ".scmap") + .put("save", "_save.lua") + .put("script", "_script.lua") + .build(); + + for (Entry entry : declarations.entrySet()) { + if (!scenarioLua.matches(format("{0}\\w=\\w'\\/maps\\/{0}(\\.v\\d{4})?/{0}{1}'", entry.getKey(), entry.getValue()))) { + errors.add(new Error(ErrorCode.MAP_SCRIPT_LINE_MISSING, format( + "{0} = ''/maps/{2}/{2}{1}'',", entry.getKey(), entry.getValue(), validMapName + ))); + } + } + + return errors; + } + @Transactional @SneakyThrows @CacheEvict(value = {Map.TYPE_NAME, MapVersion.TYPE_NAME}, allEntries = true) @@ -100,9 +171,9 @@ private Path copyToTemporaryDirectory(byte[] mapData, MapUploadData progressData } private void unzipFile(MapUploadData mapData) throws IOException, ArchiveException { - Unzipper.from(mapData.getUploadedFile()) - .to(mapData.getBaseDir()) - .unzip(); + Unzipper.from(mapData.getUploadedFile()) + .to(mapData.getBaseDir()) + .unzip(); } private void postProcessZipFiles(MapUploadData mapUploadData) throws IOException { @@ -161,7 +232,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) { diff --git a/src/main/java/com/faforever/api/map/MapValidationRequest.java b/src/main/java/com/faforever/api/map/MapValidationRequest.java new file mode 100644 index 000000000..90102dacc --- /dev/null +++ b/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; +} diff --git a/src/main/java/com/faforever/api/map/MapsController.java b/src/main/java/com/faforever/api/map/MapsController.java index 9d996172b..21109eea5 100644 --- a/src/main/java/com/faforever/api/map/MapsController.java +++ b/src/main/java/com/faforever/api/map/MapsController.java @@ -12,34 +12,51 @@ import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; +import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.springframework.http.MediaType; import org.springframework.security.core.Authentication; +import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.multipart.MultipartFile; +import org.springframework.web.servlet.ModelAndView; -import javax.inject.Inject; import java.io.IOException; +import java.util.Map; import static org.springframework.http.MediaType.APPLICATION_JSON_UTF8_VALUE; @RestController @RequestMapping(path = "/maps") @Slf4j +@AllArgsConstructor public class MapsController { private final MapService mapService; private final FafApiProperties fafApiProperties; private final ObjectMapper objectMapper; private final PlayerService playerService; - @Inject - public MapsController(MapService mapService, FafApiProperties fafApiProperties, ObjectMapper objectMapper, PlayerService playerService) { - this.mapService = mapService; - this.fafApiProperties = fafApiProperties; - this.objectMapper = objectMapper; - this.playerService = playerService; + + @RequestMapping(path = "/validate", method = RequestMethod.GET, produces = APPLICATION_JSON_UTF8_VALUE) + public ModelAndView showValidationForm(Map model) { + return new ModelAndView("validate_map_metadata.html"); + } + + @ApiOperation("Validate map metadata") + @ApiResponses(value = { + @ApiResponse(code = 200, message = "Success"), + @ApiResponse(code = 422, message = "Containing information about the errors in the payload")}) + @RequestMapping( + path = "/validate", + method = RequestMethod.POST, + produces = APPLICATION_JSON_UTF8_VALUE, + consumes = MediaType.APPLICATION_JSON_VALUE + ) + public void validateMapMetadata(@RequestBody MapValidationRequest mapValidationRequest) { + mapService.validate(mapValidationRequest); } @ApiOperation("Upload a map") diff --git a/src/main/resources/config/application-dev.yml b/src/main/resources/config/application-dev.yml index 7c405ce76..c6ccaba5e 100644 --- a/src/main/resources/config/application-dev.yml +++ b/src/main/resources/config/application-dev.yml @@ -71,4 +71,5 @@ spring: password: ${ADMIN_CLIENT_PASSWORD:banana} logging: level: - com.faforever.api: trace + com.faforever.api: debug + diff --git a/src/main/resources/templates/login.html b/src/main/resources/templates/login.html index de6681ed1..5d69605bc 100644 --- a/src/main/resources/templates/login.html +++ b/src/main/resources/templates/login.html @@ -3,9 +3,7 @@ Log-in - - - + + + + + + + diff --git a/src/test/java/com/faforever/api/map/MapServiceTest.java b/src/test/java/com/faforever/api/map/MapServiceTest.java index 3b996b154..fd1495e79 100644 --- a/src/test/java/com/faforever/api/map/MapServiceTest.java +++ b/src/test/java/com/faforever/api/map/MapServiceTest.java @@ -80,7 +80,7 @@ void setUp() throws Exception { "MAP_MISSING_MAP_FOLDER_INSIDE_ZIP,empty.zip", "MAP_FIRST_TEAM_FFA,wrong_team_name.zip", "MAP_INVALID_ZIP,scmp_037_invalid.zip", // map with more than 1 root folders in zip - "MAP_NAME_INVALID,scmp_037_no_ascii.zip", + "MAP_NAME_INVALID_CHARACTER,scmp_037_no_ascii.zip", "MAP_FILE_INSIDE_ZIP_MISSING,without_savelua.zip", "MAP_FILE_INSIDE_ZIP_MISSING,without_scenariolua.zip", "MAP_FILE_INSIDE_ZIP_MISSING,without_scmap.zip",