Skip to content

Commit 22eb8fd

Browse files
jlfsdtcjlfsdtc
authored
[MINOR] fix API And add host check
1 parent b715b53 commit 22eb8fd

File tree

19 files changed

+662
-34
lines changed

19 files changed

+662
-34
lines changed

‎src/common-server/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@
3838
<groupId>org.apache.kylin</groupId>
3939
<artifactId>kylin-core-common</artifactId>
4040
</dependency>
41+
<dependency>
42+
<groupId>org.apache.kylin</groupId>
43+
<artifactId>kylin-core-metadata</artifactId>
44+
<type>test-jar</type>
45+
<scope>test</scope>
46+
</dependency>
4147
<dependency>
4248
<groupId>org.apache.kylin</groupId>
4349
<artifactId>kylin-core-metadata</artifactId>

‎src/common-server/src/main/java/org/apache/kylin/rest/controller/NBasicController.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
import org.apache.kylin.common.KylinConfig;
8080
import org.apache.kylin.common.KylinConfigBase;
8181
import org.apache.kylin.common.exception.KylinException;
82+
import org.apache.kylin.common.exception.KylinRuntimeException;
8283
import org.apache.kylin.common.exception.ServerErrorCode;
8384
import org.apache.kylin.common.msg.Message;
8485
import org.apache.kylin.common.msg.MsgPicker;
@@ -96,7 +97,9 @@
9697
import org.apache.kylin.metadata.model.TableDesc;
9798
import org.apache.kylin.metadata.project.NProjectManager;
9899
import org.apache.kylin.metadata.project.ProjectInstance;
100+
import org.apache.kylin.metadata.resourcegroup.ResourceGroupManager;
99101
import org.apache.kylin.metadata.streaming.KafkaConfigManager;
102+
import org.apache.kylin.rest.cluster.ClusterManager;
100103
import org.apache.kylin.rest.constant.Constant;
101104
import org.apache.kylin.rest.exception.ForbiddenException;
102105
import org.apache.kylin.rest.exception.NotFoundException;
@@ -160,6 +163,9 @@ public class NBasicController {
160163
@Autowired
161164
protected UserService userService;
162165

166+
@Autowired
167+
protected ClusterManager clusterManager;
168+
163169
protected Logger getLogger() {
164170
return logger;
165171
}
@@ -701,4 +707,24 @@ private void initBinder(WebDataBinder binder) {
701707
int autoGrowCollectionLimit = KylinConfig.getInstanceFromEnv().getDataBinderAutoGrowCollectionLimit();
702708
binder.setAutoGrowCollectionLimit(autoGrowCollectionLimit);
703709
}
710+
711+
public void checkServer(String host) {
712+
if (StringUtils.isBlank(host)) {
713+
throw new KylinRuntimeException("Server cannot be null or empty");
714+
}
715+
716+
val rgManager = ResourceGroupManager.getInstance(KylinConfig.getInstanceFromEnv());
717+
val serverNotFoundInCluster = clusterManager.checkServer(host);
718+
719+
if (rgManager.isResourceGroupEnabled()) {
720+
val serverNotFoundInResourceGroup = rgManager.checkServer(host);
721+
if (serverNotFoundInCluster && serverNotFoundInResourceGroup) {
722+
throw new KylinRuntimeException(String.format(Locale.ROOT, "Server <%s> not found", host));
723+
}
724+
} else {
725+
if (serverNotFoundInCluster) {
726+
throw new KylinRuntimeException(String.format(Locale.ROOT, "Server <%s> not found", host));
727+
}
728+
}
729+
}
704730
}

‎src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ public EnvelopeResponse<String> simulateInsertMeta(
190190
public EnvelopeResponse<String> broadcastMetadataBackup(@RequestBody MetadataBackupRequest request) {
191191
log.info("ResourceGroup[{}] broadcastMetadataBackup tmpFilePath : {}", request.getResourceGroupId(),
192192
request.getTmpFilePath());
193+
checkServer(request.getFromHost());
193194
fileService.saveBroadcastMetadataBackup(request.getBackupDir(), request.getTmpFilePath(),
194195
request.getTmpFileSize(), request.getResourceGroupId(), request.getFromHost());
195196
return new EnvelopeResponse<>(CODE_SUCCESS, "", "");

‎src/common-server/src/test/java/org/apache/kylin/rest/controller/NBasicControllerTest.java‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,17 @@
4040

4141
import org.apache.commons.lang3.StringUtils;
4242
import org.apache.kylin.common.exception.KylinException;
43+
import org.apache.kylin.common.exception.KylinRuntimeException;
4344
import org.apache.kylin.common.extension.KylinInfoExtension;
4445
import org.apache.kylin.common.msg.MsgPicker;
46+
import org.apache.kylin.common.util.AddressUtil;
4547
import org.apache.kylin.common.util.NLocalFileMetadataTestCase;
4648
import org.apache.kylin.common.util.Pair;
4749
import org.apache.kylin.metadata.model.PartitionDesc;
4850
import org.apache.kylin.metadata.model.TableDesc;
51+
import org.apache.kylin.metadata.resourcegroup.ResourceGroupManagerTest;
52+
import org.apache.kylin.rest.cluster.ClusterManager;
53+
import org.apache.kylin.rest.cluster.MockClusterManager;
4954
import org.apache.kylin.rest.controller.fixture.FixtureController;
5055
import org.apache.kylin.rest.exception.ForbiddenException;
5156
import org.apache.kylin.rest.exception.NotFoundException;
@@ -56,6 +61,7 @@
5661
import org.junit.Before;
5762
import org.junit.Rule;
5863
import org.junit.Test;
64+
import org.junit.jupiter.api.Assertions;
5965
import org.junit.rules.ExpectedException;
6066
import org.junit.runner.RunWith;
6167
import org.mockito.InjectMocks;
@@ -64,6 +70,7 @@
6470
import org.powermock.core.classloader.annotations.PrepareForTest;
6571
import org.powermock.modules.junit4.PowerMockRunner;
6672
import org.springframework.security.access.AccessDeniedException;
73+
import org.springframework.test.util.ReflectionTestUtils;
6774
import org.springframework.test.web.servlet.MockMvc;
6875
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
6976
import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
@@ -333,4 +340,68 @@ public void testEncodeAndDecodeHost() {
333340
Assert.assertEquals("ip", nBasicController.decodeHost("ip"));
334341
}
335342

343+
@Test
344+
public void testCheckServerWithResourceGroupEnable() {
345+
ClusterManager clusterManager = new MockClusterManager();
346+
347+
String project = "default";
348+
ResourceGroupManagerTest.mockResourceGroup(AddressUtil.getLocalInstance(), project);
349+
ReflectionTestUtils.setField(nBasicController, "clusterManager", clusterManager);
350+
351+
// Test valid server - should not throw exception
352+
Assertions.assertDoesNotThrow(() -> nBasicController.checkServer(AddressUtil.getLocalInstance()));
353+
Assertions.assertDoesNotThrow(() -> nBasicController.checkServer("127.0.0.1:7070"));
354+
355+
// Test null host - should throw KylinRuntimeException
356+
KylinRuntimeException nullException = Assertions.assertThrows(KylinRuntimeException.class,
357+
() -> nBasicController.checkServer(null));
358+
Assertions.assertEquals("Server cannot be null or empty", nullException.getMessage());
359+
360+
// Test empty host - should throw KylinRuntimeException
361+
KylinRuntimeException emptyException = Assertions.assertThrows(KylinRuntimeException.class,
362+
() -> nBasicController.checkServer(""));
363+
Assertions.assertEquals("Server cannot be null or empty", emptyException.getMessage());
364+
KylinRuntimeException emptyException2 = Assertions.assertThrows(KylinRuntimeException.class,
365+
() -> nBasicController.checkServer(" "));
366+
Assertions.assertEquals("Server cannot be null or empty", emptyException2.getMessage());
367+
368+
// Test host not found in servers - should throw KylinRuntimeException
369+
KylinRuntimeException notFoundException = Assertions.assertThrows(KylinRuntimeException.class,
370+
() -> nBasicController.checkServer("192.168.1.1:8080"));
371+
Assertions.assertEquals("Server <192.168.1.1:8080> not found", notFoundException.getMessage());
372+
}
373+
374+
@Test
375+
public void testCheckServerWithoutResourceGroupEnable() {
376+
ClusterManager clusterManager = new MockClusterManager();
377+
378+
ReflectionTestUtils.setField(nBasicController, "clusterManager", clusterManager);
379+
380+
// Test valid server - should not throw exception
381+
Assertions.assertDoesNotThrow(() -> nBasicController.checkServer("127.0.0.1:7070"));
382+
383+
// Test null host - should throw KylinRuntimeException
384+
KylinRuntimeException nullException = Assertions.assertThrows(KylinRuntimeException.class,
385+
() -> nBasicController.checkServer(null));
386+
Assertions.assertEquals("Server cannot be null or empty", nullException.getMessage());
387+
388+
// Test empty host - should throw KylinRuntimeException
389+
KylinRuntimeException emptyException = Assertions.assertThrows(KylinRuntimeException.class,
390+
() -> nBasicController.checkServer(""));
391+
Assertions.assertEquals("Server cannot be null or empty", emptyException.getMessage());
392+
KylinRuntimeException emptyException2 = Assertions.assertThrows(KylinRuntimeException.class,
393+
() -> nBasicController.checkServer(" "));
394+
Assertions.assertEquals("Server cannot be null or empty", emptyException2.getMessage());
395+
396+
// Test host not found in servers - should throw KylinRuntimeException
397+
KylinRuntimeException notFoundException = Assertions.assertThrows(KylinRuntimeException.class,
398+
() -> nBasicController.checkServer("192.168.1.1:8080"));
399+
Assertions.assertEquals("Server <192.168.1.1:8080> not found", notFoundException.getMessage());
400+
401+
// Test current host not found in servers - should throw KylinRuntimeException
402+
KylinRuntimeException notFoundException2 = Assertions.assertThrows(KylinRuntimeException.class,
403+
() -> nBasicController.checkServer(AddressUtil.getLocalInstance()));
404+
Assertions.assertEquals("Server <" + AddressUtil.getLocalInstance() + "> not found",
405+
notFoundException2.getMessage());
406+
}
336407
}

‎src/common-service/src/main/java/org/apache/kylin/rest/service/FileService.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import static org.apache.kylin.common.constant.Constants.BACKSLASH;
2222
import static org.apache.kylin.common.constant.Constants.METADATA_FILE;
23+
import static org.apache.kylin.common.constant.Constants.SYSTEM_TMP_DIR;
2324
import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON;
2425

2526
import java.io.File;
@@ -28,6 +29,7 @@
2829
import java.io.IOException;
2930
import java.io.InputStream;
3031
import java.nio.file.Files;
32+
import java.nio.file.Paths;
3133
import java.util.Arrays;
3234
import java.util.Locale;
3335

@@ -60,12 +62,31 @@
6062
@Slf4j
6163
@Service
6264
public class FileService extends BasicService {
65+
protected static final String METADATA_TMP_PREFIX = "KylinMetadataBackupTmp-";
66+
6367
@Autowired
6468
@Qualifier("normalRestTemplate")
6569
private RestTemplate restTemplate;
6670

71+
public static String getSafeAbsolutePath(String filePath) {
72+
val basePath = Paths.get(SYSTEM_TMP_DIR).toAbsolutePath().normalize();
73+
val targetPath = Paths.get(filePath).toAbsolutePath().normalize();
74+
75+
if (!targetPath.startsWith(basePath)) {
76+
throw new SecurityException("Path outside base directory: " + filePath);
77+
}
78+
79+
val metadataTmpPrefix = Paths.get(basePath.toString(), METADATA_TMP_PREFIX).toAbsolutePath().normalize();
80+
if (!StringUtils.startsWith(targetPath.toString(), metadataTmpPrefix.toString())) {
81+
throw new SecurityException("Path not kylin metadata tmp directory: " + filePath);
82+
}
83+
84+
return targetPath.toString();
85+
}
86+
6787
public InputStream getMetadataBackupFromTmpPath(String tmpFilePath, Long fileSize) throws IOException {
68-
val metadataBackupTmp = new File(tmpFilePath);
88+
val realTempPath = getSafeAbsolutePath(tmpFilePath);
89+
val metadataBackupTmp = new File(realTempPath);
6990
if (metadataBackupTmp.isFile()) {
7091
if (metadataBackupTmp.length() != fileSize) {
7192
throw new FileNotFoundException("Metadata backup temp file length does not right: " + tmpFilePath
@@ -84,7 +105,7 @@ public Pair<String, Long> saveMetadataBackupInTmpPath(String path) throws IOExce
84105
val filePath = new Path(path);
85106
if (fileSystem.isFile(filePath)) {
86107
val fileStatus = fileSystem.getFileStatus(filePath);
87-
val tempDirectory = Files.createTempDirectory("MetadataBackupTmp-").toFile();
108+
val tempDirectory = Files.createTempDirectory(Paths.get(SYSTEM_TMP_DIR), METADATA_TMP_PREFIX).toFile();
88109
fileSystem.copyToLocalFile(false, filePath, new Path(tempDirectory.getAbsolutePath()), true);
89110
val tmpFile = new File(tempDirectory, METADATA_FILE);
90111
if (fileStatus.getLen() != tmpFile.length()) {
@@ -99,7 +120,7 @@ public Pair<String, Long> saveMetadataBackupInTmpPath(String path) throws IOExce
99120
}
100121

101122
public String saveMetadataBackupTmpFromRequest(Long fileSize, InputStream inputStream) throws IOException {
102-
val tmpDirectory = Files.createTempDirectory("MetadataBackupTmp-").toFile();
123+
val tmpDirectory = Files.createTempDirectory(METADATA_TMP_PREFIX).toFile();
103124
val tmpFile = new File(tmpDirectory, METADATA_FILE);
104125
try (val os = new FileOutputStream(tmpFile)) {
105126
IOUtils.copy(inputStream, os);

‎src/common-service/src/main/java/org/apache/kylin/rest/service/ProjectService.java‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,15 @@
2626
import static org.apache.kylin.common.constant.Constants.KYLIN_SOURCE_JDBC_SOURCE_ENABLE_KEY;
2727
import static org.apache.kylin.common.constant.Constants.KYLIN_SOURCE_JDBC_SOURCE_NAME_KEY;
2828
import static org.apache.kylin.common.constant.Constants.KYLIN_SOURCE_JDBC_USER_KEY;
29+
import static org.apache.kylin.common.constant.Constants.MAX_FILENAME_LENGTH;
2930
import static org.apache.kylin.common.constant.NonCustomProjectLevelConfig.DATASOURCE_TYPE;
3031
import static org.apache.kylin.common.exception.ServerErrorCode.DATABASE_NOT_EXIST;
3132
import static org.apache.kylin.common.exception.ServerErrorCode.DUPLICATE_PROJECT_NAME;
3233
import static org.apache.kylin.common.exception.ServerErrorCode.EMPTY_EMAIL;
3334
import static org.apache.kylin.common.exception.ServerErrorCode.EMPTY_PARAMETER;
3435
import static org.apache.kylin.common.exception.ServerErrorCode.FILE_TYPE_MISMATCH;
3536
import static org.apache.kylin.common.exception.ServerErrorCode.INVALID_JDBC_SOURCE_CONFIG;
37+
import static org.apache.kylin.common.exception.ServerErrorCode.INVALID_KERBEROS_FILE;
3638
import static org.apache.kylin.common.exception.ServerErrorCode.INVALID_PARAMETER;
3739
import static org.apache.kylin.common.exception.ServerErrorCode.PERMISSION_DENIED;
3840
import static org.apache.kylin.common.exception.ServerErrorCode.PROJECT_DROP_FAILED;
@@ -172,6 +174,8 @@ public class ProjectService extends BasicService {
172174
private static final String SNAPSHOT_AUTO_REFRESH_TIME_MODES = "DAY, HOURS, MINUTE";
173175
private static final String KYLIN_QUERY_PUSHDOWN_RUNNER_CLASS_NAME = "kylin.query.pushdown.runner-class-name";
174176
private static final String DEFAULT_VAL = "default";
177+
private static final Pattern INVALID_FILENAME_CHARS = Pattern.compile("[\\\\/:*?\"'<>|\\u0000-\\u001F\\s]|^[.]+$|[.]$");
178+
175179
@Autowired
176180
UserService userService;
177181
@Autowired
@@ -1242,11 +1246,26 @@ public File backupAndDeleteKeytab(String principal) throws IOException {
12421246
return kFile;
12431247
}
12441248

1245-
public File generateTempKeytab(String principal, MultipartFile keytabFile) throws IOException {
1246-
Message msg = MsgPicker.getMsg();
1249+
public static void checkPrincipal(String principal, Message msg) {
12471250
if (null == principal || principal.isEmpty()) {
12481251
throw new KylinException(EMPTY_PARAMETER, msg.getPrincipalEmpty());
12491252
}
1253+
1254+
// principal length
1255+
if (principal.length() > MAX_FILENAME_LENGTH - KerberosLoginManager.TMP_KEYTAB_SUFFIX.length()) {
1256+
throw new KylinException(INVALID_KERBEROS_FILE, msg.getKerberosInfoError());
1257+
}
1258+
1259+
// invalid characters
1260+
if (INVALID_FILENAME_CHARS.matcher(principal).find()) {
1261+
throw new KylinException(INVALID_KERBEROS_FILE, msg.getKerberosInfoError());
1262+
}
1263+
}
1264+
1265+
public File generateTempKeytab(String principal, MultipartFile keytabFile) throws IOException {
1266+
Message msg = MsgPicker.getMsg();
1267+
checkPrincipal(principal, msg);
1268+
12501269
val originalFilename = keytabFile.getOriginalFilename();
12511270
if (originalFilename == null || !originalFilename.endsWith(".keytab")) {
12521271
throw new KylinException(FILE_TYPE_MISMATCH, msg.getKeytabFileTypeMismatch());

0 commit comments

Comments
 (0)