diff --git a/ngrinder-controller/build.gradle b/ngrinder-controller/build.gradle index 1faa1c1f0..a0dace928 100644 --- a/ngrinder-controller/build.gradle +++ b/ngrinder-controller/build.gradle @@ -59,7 +59,7 @@ dependencies { implementation (group: "jaxen", name: "jaxen", version: "1.1.4") implementation (group: "com.beust", name: "jcommander", version: "1.32") implementation (group: "org.pf4j", name: "pf4j", version: "3.0.1") - implementation (group: "com.esotericsoftware.yamlbeans", name: "yamlbeans", version: "1.17") + implementation (group: "org.yaml", name: "snakeyaml", version: "1.25") implementation (group: "commons-collections", name: "commons-collections", version: "3.2.1") implementation (group: "org.reflections", name: "reflections", version: "0.9.9") implementation (group: "com.hazelcast", name: "hazelcast", version: hazelcast_version) diff --git a/ngrinder-controller/src/main/java/org/ngrinder/script/service/GitHubFileEntryService.java b/ngrinder-controller/src/main/java/org/ngrinder/script/service/GitHubFileEntryService.java index c509b474f..18966fd81 100644 --- a/ngrinder-controller/src/main/java/org/ngrinder/script/service/GitHubFileEntryService.java +++ b/ngrinder-controller/src/main/java/org/ngrinder/script/service/GitHubFileEntryService.java @@ -1,6 +1,5 @@ package org.ngrinder.script.service; -import com.esotericsoftware.yamlbeans.YamlReader; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; @@ -27,6 +26,8 @@ import org.tmatesoft.svn.core.SVNURL; import org.tmatesoft.svn.core.auth.BasicAuthenticationManager; import org.tmatesoft.svn.core.wc.*; +import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.constructor.SafeConstructor; import java.io.File; import java.io.FileNotFoundException; @@ -280,24 +281,23 @@ public Set getAllGitHubConfig(User user) throws FileNotFoundExcept private Set getAllGithubConfig(FileEntry gitConfigYaml) { Set gitHubConfig = new HashSet<>(); - try (YamlReader reader = new YamlReader(gitConfigYaml.getContent())) { - Map gitConfigMap = cast(reader.read()); - while (gitConfigMap != null) { - gitConfigMap.put("revision", gitConfigYaml.getRevision()); - GitHubConfig config = objectMapper.convertValue(gitConfigMap, GitHubConfig.class); - - if (gitHubConfig.contains(config)) { - throw new InvalidGitHubConfigurationException("GitHub configuration '" - + config.getName() + "' is duplicated.\nPlease check your .gitconfig.yml"); - } - - gitHubConfig.add(config); + // Yaml is not thread safe. so create it every time. + Yaml yaml = new Yaml(new SafeConstructor()); + Iterable> gitConfigs = cast(yaml.loadAll(gitConfigYaml.getContent())); + for (Map configMap : gitConfigs) { + if (configMap == null) { + continue; + } + GitHubConfig config = objectMapper.convertValue(configMap, GitHubConfig.class); + config.setRevision(String.valueOf(gitConfigYaml.getRevision())); - gitConfigMap = cast(reader.read()); + if (gitHubConfig.contains(config)) { + throw new InvalidGitHubConfigurationException("GitHub configuration '" + + config.getName() + "' is duplicated.\nPlease check your .gitconfig.yml"); } - } catch (IOException e) { - throw new InvalidGitHubConfigurationException("Fail to read GitHub configuration.", e); - } + + gitHubConfig.add(config); + } return gitHubConfig; } diff --git a/ngrinder-controller/src/test/java/org/ngrinder/script/service/GitHubFileEntryServiceTest.java b/ngrinder-controller/src/test/java/org/ngrinder/script/service/GitHubFileEntryServiceTest.java index 6bd0a6e11..17f3579dd 100644 --- a/ngrinder-controller/src/test/java/org/ngrinder/script/service/GitHubFileEntryServiceTest.java +++ b/ngrinder-controller/src/test/java/org/ngrinder/script/service/GitHubFileEntryServiceTest.java @@ -1,88 +1,69 @@ package org.ngrinder.script.service; -import org.apache.commons.io.FileUtils; -import org.junit.Before; import org.junit.Test; import org.ngrinder.AbstractNGrinderTransactionalTest; -import org.ngrinder.common.util.CompressionUtils; -import org.ngrinder.model.User; +import org.ngrinder.common.exception.NGrinderRuntimeException; import org.ngrinder.script.model.FileEntry; -import org.ngrinder.script.model.GitHubConfig; -import org.ngrinder.script.repository.MockFileEntityRepository; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.core.io.ClassPathResource; +import org.yaml.snakeyaml.constructor.ConstructorException; -import java.io.File; -import java.io.IOException; -import java.util.Set; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.hasItems; -import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertTrue; public class GitHubFileEntryServiceTest extends AbstractNGrinderTransactionalTest { + @Autowired + private GitHubFileEntryService gitHubFileEntryService; - private static final String GITHUB_CONFIG_NAME = ".gitconfig.yml"; - - @Autowired - private GitHubFileEntryService gitHubFileEntryService; - - @Autowired - private MockFileEntityRepository mockFileEntityRepository; + @Test + public void testValidateInvalidConfigNameLength() { + FileEntry fileEntry = new FileEntry(); + fileEntry.setContent( + "name: My Long Long Long Long Long Long Github Config Name\n" + + "owner: naver\n" + + "repo: ngrinder\n" + + "access-token: e1a47e652762b60a...3ddc0713b07g13k\n" + ); + fileEntry.setRevision(-1L); - @Before - public void before() throws IOException { - File file = new File(System.getProperty("java.io.tmpdir"), "repo"); - FileUtils.deleteQuietly(file); - CompressionUtils.unzip(new ClassPathResource("TEST_USER.zip").getFile(), file); - mockFileEntityRepository.setUserRepository(new File(file, getTestUser().getUserId())); - } + try { + gitHubFileEntryService.validate(fileEntry); + } catch (Exception e) { + assertTrue(e instanceof NGrinderRuntimeException); + assertTrue(e.getMessage().contains("Configuration name must be shorter than")); + } + } - @Test - public void getAllGitHubConfig() throws Exception { - User testUser = getTestUser(); + @Test + public void testValidateInvalidYamlValue() { + FileEntry fileEntry = new FileEntry(); + fileEntry.setContent( + "!!com.sun.rowset.JdbcRowSetImpl\n " + + "dataSourceName: rmi://127.0.0.1:13243/jmxrmi\n " + + "autoCommit: true" + ); + fileEntry.setRevision(-1L); - FileEntry fileEntry = new FileEntry(); - fileEntry.setContent("name: My Github Config\n" + - "owner: naver\n" + - "repo: ngrinder\n" + - "access-token: e1a47e652762b60a...3ddc0713b07g13k\n" + - "---\n" + - "name: Another Config\n" + - "owner: naver\n" + - "repo: pinpoint\n" + - "access-token: t9a47e6ff262b60a...3dac0713b07e82a\n" + - "branch: feature/add-ngrinder-scripts\n" + - "base-url: https://api.mygithub.com\n" + - "script-root: ngrinder-scripts\n" - ); - fileEntry.setEncoding("UTF-8"); - fileEntry.setPath(GITHUB_CONFIG_NAME); - fileEntry.setDescription("for unit test"); - mockFileEntityRepository.save(testUser, fileEntry, fileEntry.getEncoding()); + try { + gitHubFileEntryService.validate(fileEntry); + } catch (Exception e) { + assertTrue(e instanceof ConstructorException); + assertTrue(e.getMessage().contains("could not determine a constructor for the tag")); + } + } - Set gitHubConfigs = gitHubFileEntryService.getAllGitHubConfig(testUser); + @Test + public void testValidateInvalidYamlValue2() { + FileEntry fileEntry = new FileEntry(); + fileEntry.setContent( + "some_var: !!javax.script.ScriptEngineManager " + + "[!!java.net.URLClassLoader [[!!java.net.URL [\"http://localhost:8080\"]]]]" + ); + fileEntry.setRevision(-1L); - assertThat(gitHubConfigs.size(), is(2)); - assertThat( - gitHubConfigs, - hasItems( - GitHubConfig.builder() - .name("My Github Config") - .owner("naver") - .repo("ngrinder") - .accessToken("e1a47e652762b60a...3ddc0713b07g13k") - .build(), - GitHubConfig.builder() - .name("Another Config") - .owner("naver") - .repo("pinpoint") - .accessToken("t9a47e6ff262b60a...3dac0713b07e82a") - .branch("feature/add-ngrinder-scripts") - .baseUrl("https://api.mygithub.com") - .scriptRoot("ngrinder-scripts") - .build() - ) - ); - } + try { + gitHubFileEntryService.validate(fileEntry); + } catch (Exception e) { + assertTrue(e instanceof ConstructorException); + assertTrue(e.getMessage().contains("could not determine a constructor for the tag")); + } + } }