Skip to content

Commit

Permalink
Merge pull request #1018 from naver/feature/yaml
Browse files Browse the repository at this point in the history
Change to use snakeyaml with SafeConstructor
  • Loading branch information
imbyungjun authored May 16, 2024
2 parents 6c7c6f1 + 6df6b32 commit cac2c82
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 90 deletions.
2 changes: 1 addition & 1 deletion ngrinder-controller/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -280,24 +281,23 @@ public Set<GitHubConfig> getAllGitHubConfig(User user) throws FileNotFoundExcept

private Set<GitHubConfig> getAllGithubConfig(FileEntry gitConfigYaml) {
Set<GitHubConfig> gitHubConfig = new HashSet<>();
try (YamlReader reader = new YamlReader(gitConfigYaml.getContent())) {
Map<String, Object> 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<Map<String, String>> gitConfigs = cast(yaml.loadAll(gitConfigYaml.getContent()));
for (Map<String, String> 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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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<GitHubConfig> 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"));
}
}
}

0 comments on commit cac2c82

Please sign in to comment.