This is an automated email from the ASF dual-hosted git repository.

caolu pushed a commit to branch kylin5
in repository https://gitbox.apache.org/repos/asf/kylin.git


The following commit(s) were added to refs/heads/kylin5 by this push:
     new 721b7a5e02 KYLIN-6036 Skip tables without permissions when loading 
tables
721b7a5e02 is described below

commit 721b7a5e02696e9ce6398b9537579586937539b8
Author: jlf <[email protected]>
AuthorDate: Fri Jul 12 13:49:46 2024 +0800

    KYLIN-6036 Skip tables without permissions when loading tables
    
    * validate host pattern
    
    ---
    
    Co-authored-by: longfei.jiang <[email protected]>
    Co-authored-by: sheng.huang <[email protected]>
---
 .../org/apache/kylin/common/KylinConfigBase.java   |   4 +-
 .../org/apache/kylin/common/util/AddressUtil.java  |   6 +
 .../org/apache/kylin/common/util/StringHelper.java |   4 +
 .../apache/kylin/common/KylinConfigBaseTest.java   |  17 +++
 .../apache/kylin/common/util/AddressUtilTest.java  |  21 ++++
 .../apache/kylin/common/util/StringHelperTest.java |  31 +++++
 src/datasource-service/pom.xml                     |  10 ++
 .../apache/kylin/rest/service/TableExtService.java |   3 +-
 .../apache/kylin/rest/service/TableService.java    |  39 +++---
 .../kylin/rest/service/TableExtServiceTest.java    |  16 +--
 .../kylin/rest/service/TableServiceTest.java       | 133 +++++++++++++++++++++
 .../kylin/rest/controller/OpsController.java       |   6 +
 .../kylin/rest/service/SparderUIService.java       |   3 +
 .../kylin/rest/service/SparderUIServiceTest.java   |  61 +++++++---
 14 files changed, 314 insertions(+), 40 deletions(-)

diff --git 
a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java 
b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
index 709b959634..0ca67fdceb 100644
--- a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
+++ b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
@@ -726,7 +726,9 @@ public abstract class KylinConfigBase implements 
Serializable {
         // Caution: config 'kylin.server.address' is essential in yarn cluster 
mode.
         // The value may be the address of loadbalancer
         // format: ip:port
-        return getOptional("kylin.server.address", getDefaultServerAddress());
+        String serverAddress = getOptional("kylin.server.address", 
getDefaultServerAddress());
+        AddressUtil.validateHost(serverAddress);
+        return serverAddress;
     }
 
     private String getDefaultServerAddress() {
diff --git 
a/src/core-common/src/main/java/org/apache/kylin/common/util/AddressUtil.java 
b/src/core-common/src/main/java/org/apache/kylin/common/util/AddressUtil.java
index 85a60f82b7..24eda85407 100644
--- 
a/src/core-common/src/main/java/org/apache/kylin/common/util/AddressUtil.java
+++ 
b/src/core-common/src/main/java/org/apache/kylin/common/util/AddressUtil.java
@@ -121,4 +121,10 @@ public class AddressUtil {
     public static void clearLocalIpAddressCache() {
         localIpAddressCache = null;
     }
+
+    public static void validateHost(String host) {
+        if (StringUtils.isNotBlank(host) && !StringHelper.validateHost(host)) {
+            throw new IllegalArgumentException("Url contains disallowed chars, 
host: " + host);
+        }
+    }
 }
diff --git 
a/src/core-common/src/main/java/org/apache/kylin/common/util/StringHelper.java 
b/src/core-common/src/main/java/org/apache/kylin/common/util/StringHelper.java
index 9eb1f7c296..791b6f9ece 100644
--- 
a/src/core-common/src/main/java/org/apache/kylin/common/util/StringHelper.java
+++ 
b/src/core-common/src/main/java/org/apache/kylin/common/util/StringHelper.java
@@ -149,6 +149,10 @@ public class StringHelper {
         return 
Pattern.compile("^(http(s)?://)?[a-zA-Z0-9._-]+(:[0-9]+)?(/[a-zA-Z0-9._-]+)*/?$").matcher(s).matches();
     }
 
+    public static boolean validateHost(String s) {
+        return 
Pattern.compile("^(http(s)?://)?[a-zA-Z0-9._-]+(:[0-9]+)?").matcher(s).matches();
+    }
+
     public static boolean validateDbName(String s) {
         return Pattern.compile("^[0-9a-zA-Z_-]+$").matcher(s).matches();
     }
diff --git 
a/src/core-common/src/test/java/org/apache/kylin/common/KylinConfigBaseTest.java
 
b/src/core-common/src/test/java/org/apache/kylin/common/KylinConfigBaseTest.java
index 0d2f359f19..7017a3fa06 100644
--- 
a/src/core-common/src/test/java/org/apache/kylin/common/KylinConfigBaseTest.java
+++ 
b/src/core-common/src/test/java/org/apache/kylin/common/KylinConfigBaseTest.java
@@ -1562,6 +1562,23 @@ class KylinConfigBaseTest {
         val withoutExpected = "," + celebornJar.getAbsolutePath() + "," + 
mysqlJar.getAbsolutePath();
         Assertions.assertEquals(withoutExpected, withoutGluten);
     }
+
+    void testGetServerAddress() {
+        KylinConfig config = KylinConfig.getInstanceFromEnv();
+        config.setProperty("kylin.server.address", "127.0.0.1");
+        Assertions.assertEquals("127.0.0.1", config.getServerAddress());
+        config.setProperty("kylin.server.address", "8080");
+        Assertions.assertEquals("8080", config.getServerAddress());
+
+        try {
+            config.setProperty("kylin.server.address", "8080>");
+            config.getServerAddress();
+            Assertions.fail();
+        } catch (Exception e) {
+            Assertions.assertInstanceOf(IllegalArgumentException.class, e);
+            Assertions.assertTrue(e.getMessage().contains("Url contains 
disallowed chars, host: "));
+        }
+    }
 }
 
 class EnvironmentUpdateUtils {
diff --git 
a/src/core-common/src/test/java/org/apache/kylin/common/util/AddressUtilTest.java
 
b/src/core-common/src/test/java/org/apache/kylin/common/util/AddressUtilTest.java
index 0d49fd6c25..fd0dd5f2e3 100644
--- 
a/src/core-common/src/test/java/org/apache/kylin/common/util/AddressUtilTest.java
+++ 
b/src/core-common/src/test/java/org/apache/kylin/common/util/AddressUtilTest.java
@@ -80,6 +80,27 @@ class AddressUtilTest {
         }
     }
 
+    @Test
+    void testCheckHost() {
+        AddressUtil.validateHost("127.0.0.1:7070");
+        AddressUtil.validateHost("127.0.0.1");
+        AddressUtil.validateHost("707");
+        AddressUtil.validateHost("");
+        testCheckHostFail("127.0.0.1:");
+        testCheckHostFail(":7070");
+        testCheckHostFail("127.0.0.1:7070>");
+    }
+
+    void testCheckHostFail(String host) {
+        try {
+            AddressUtil.validateHost(host);
+            Assertions.fail();
+        } catch (Exception e) {
+            Assertions.assertInstanceOf(IllegalArgumentException.class, e);
+            Assertions.assertTrue(e.getMessage().contains("Url contains 
disallowed chars, host: "));
+        }
+    }
+
     @Test
     void testIsSameHost() {
         
Assertions.assertTrue(AddressUtil.isSameHost(hostInfoFetcher.getHostname()));
diff --git 
a/src/core-common/src/test/java/org/apache/kylin/common/util/StringHelperTest.java
 
b/src/core-common/src/test/java/org/apache/kylin/common/util/StringHelperTest.java
index 07c11e28d2..72cbda2471 100644
--- 
a/src/core-common/src/test/java/org/apache/kylin/common/util/StringHelperTest.java
+++ 
b/src/core-common/src/test/java/org/apache/kylin/common/util/StringHelperTest.java
@@ -143,6 +143,37 @@ class StringHelperTest {
         Assertions.assertFalse(StringHelper.validateUrl(""));
     }
 
+    @Test
+    void testValidateHost() {
+        Assertions.assertTrue(StringHelper.validateHost("127.0.0.1"));
+        Assertions.assertTrue(StringHelper.validateHost("kylin.apache.org"));
+        Assertions.assertTrue(StringHelper.validateHost("kylin"));
+        Assertions.assertTrue(StringHelper.validateHost("http://127.0.0.1";));
+        
Assertions.assertTrue(StringHelper.validateHost("http://127.0.0.1:80";));
+        
Assertions.assertTrue(StringHelper.validateHost("https://kylin.apache.org";));
+        Assertions.assertTrue(StringHelper.validateHost("http://kylin";));
+        Assertions.assertTrue(StringHelper.validateHost("http://ky-lin";));
+        Assertions.assertTrue(StringHelper.validateHost("http://ky.lin";));
+        Assertions.assertTrue(StringHelper.validateHost("http://ky_lin";));
+        Assertions.assertTrue(StringHelper.validateHost("http://ky_lin:80";));
+    }
+
+    @Test
+    void testValidIllegalHost() {
+        
Assertions.assertFalse(StringHelper.validateHost("http://127.0.0.1/a_p.i";));
+        
Assertions.assertFalse(StringHelper.validateHost("http://127.0.0.1:80/";));
+        Assertions.assertFalse(StringHelper.validateHost("http://kylin/";));
+        Assertions.assertFalse(StringHelper.validateHost("http://kylin/$(rm 
-rf /)"));
+        Assertions.assertFalse(StringHelper.validateHost("http://kylin/`rm 
-rf`"));
+        Assertions.assertFalse(StringHelper.validateHost("http://kylin/'&ls"));
+        Assertions.assertFalse(StringHelper.validateHost("http://kylin/;ls";));
+        Assertions.assertFalse(StringHelper.validateHost("http://kylin/>ls"));
+        Assertions.assertFalse(StringHelper.validateHost(""));
+        Assertions.assertFalse(StringHelper.validateHost("http://ky-lin:80/";));
+        Assertions.assertFalse(StringHelper.validateHost("http://ky.lin/";));
+        Assertions.assertFalse(StringHelper.validateHost("http://ky_lin/";));
+    }
+
     @Test
     void testValidateDB() {
         Assertions.assertTrue(StringHelper.validateDbName("db_TEST-01"));
diff --git a/src/datasource-service/pom.xml b/src/datasource-service/pom.xml
index cc4a08dabd..1ee600064c 100644
--- a/src/datasource-service/pom.xml
+++ b/src/datasource-service/pom.xml
@@ -91,6 +91,16 @@
             <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-module-junit4</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-api-mockito2</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.awaitility</groupId>
             <artifactId>awaitility</artifactId>
diff --git 
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableExtService.java
 
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableExtService.java
index 019382d96a..2154b51908 100644
--- 
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableExtService.java
+++ 
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableExtService.java
@@ -428,7 +428,8 @@ public class TableExtService extends BasicService {
         KylinConfig config = KylinConfig.getInstanceFromEnv();
         return ugi.doAs((PrivilegedExceptionAction<List<Pair<TableDesc, 
TableExtDesc>>>) () -> {
             ProjectInstance projectInstance = 
getManager(NProjectManager.class).getProject(project);
-            List<Pair<TableDesc, TableExtDesc>> extractTableMetas = 
tableService.extractTableMeta(tables, project);
+            List<Pair<TableDesc, TableExtDesc>> extractTableMetas = 
tableService.extractTableMeta(tables, project,
+                    tableResponse);
             if (config.getTableAccessFilterEnable() && 
projectInstance.isProjectKerberosEnabled()) {
                 return extractTableMetas.stream().map(pair -> {
                     TableDesc tableDesc = pair.getFirst();
diff --git 
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
 
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
index 130e9bbb3e..feddd5487c 100644
--- 
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
+++ 
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
@@ -43,6 +43,7 @@ import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -156,6 +157,7 @@ import org.apache.kylin.rest.request.S3TableExtInfo;
 import org.apache.kylin.rest.request.TableDescRequest;
 import org.apache.kylin.rest.response.AutoMergeConfigResponse;
 import org.apache.kylin.rest.response.EnvelopeResponse;
+import org.apache.kylin.rest.response.LoadTableResponse;
 import org.apache.kylin.rest.response.NHiveTableNameResponse;
 import org.apache.kylin.rest.response.NInitTablesResponse;
 import org.apache.kylin.rest.response.OpenPreReloadTableResponse;
@@ -345,6 +347,11 @@ public class TableService extends BasicService {
     }
 
     public List<Pair<TableDesc, TableExtDesc>> extractTableMeta(String[] 
tables, String project) {
+        return extractTableMeta(tables, project, null);
+    }
+
+    public List<Pair<TableDesc, TableExtDesc>> extractTableMeta(String[] 
tables, String project,
+            LoadTableResponse tableResponse) {
         // de-dup
         SetMultimap<String, String> databaseTables = 
LinkedHashMultimap.create();
         for (String fullTableName : tables) {
@@ -356,7 +363,10 @@ public class TableService extends BasicService {
         // load all tables first Pair<TableDesc, TableExtDesc>
         ProjectInstance projectInstance = 
getManager(NProjectManager.class).getProject(project);
         ISourceMetadataExplorer explr = 
SourceFactory.getSource(projectInstance).getSourceMetadataExplorer();
-        List<Pair<Map.Entry<String, String>, Object>> results = 
databaseTables.entries().parallelStream().map(entry -> {
+
+        List<Pair<TableDesc, TableExtDesc>> successResults = 
Collections.synchronizedList(new ArrayList<>());
+        List<String> failedTableNames = Collections.synchronizedList(new 
ArrayList<>());
+        databaseTables.entries().parallelStream().forEach(entry -> {
             try {
                 Pair<TableDesc, TableExtDesc> pair = 
explr.loadTableMetadata(entry.getKey(), entry.getValue(), project);
                 TableDesc tableDesc = pair.getFirst();
@@ -366,24 +376,23 @@ public class TableService extends BasicService {
                         entry.getKey().toUpperCase(Locale.ROOT) + "." + 
entry.getValue().toUpperCase(Locale.ROOT)));
                 TableExtDesc extDesc = pair.getSecond();
                 
Preconditions.checkState(tableDesc.getIdentity().equals(extDesc.getIdentity()));
-                return new Pair<Map.Entry<String, String>, Object>(entry, 
pair);
+                successResults.add(pair);
             } catch (Exception e) {
-                return new Pair<Map.Entry<String, String>, Object>(entry, e);
+                logger.error("Failed to extract meta data of table:{}.{}", 
entry.getKey(), entry.getValue(), e);
+                failedTableNames.add(entry.getKey() + "." + entry.getValue());
             }
-        }).collect(Collectors.toList());
-        List<Pair<Map.Entry<String, String>, Object>> errorList = 
results.stream()
-                .filter(pair -> pair.getSecond() instanceof 
Throwable).collect(Collectors.toList());
-        if (!errorList.isEmpty()) {
-            errorList.forEach(e -> logger.error(e.getFirst().getKey() + "." + 
e.getFirst().getValue(),
-                    (Throwable) (e.getSecond())));
-            String errorTables = StringUtils
-                    .join(errorList.stream().map(error -> 
error.getFirst().getKey() + "." + error.getFirst().getValue())
-                            .collect(Collectors.toList()), ",");
+        });
+        if (!failedTableNames.isEmpty()) {
+            String errorTables = StringUtils.join(failedTableNames, "\n");
             String errorMessage = String.format(Locale.ROOT, 
MsgPicker.getMsg().getHiveTableNotFound(), errorTables);
-            throw new KylinException(TABLE_NOT_EXIST, errorMessage);
+            if (Objects.isNull(tableResponse)) {
+                throw new KylinException(TABLE_NOT_EXIST, errorMessage);
+            } else {
+                tableResponse.getFailed().addAll(failedTableNames);
+                logger.error("Skip load these failed tables:\n{}", 
errorTables);
+            }
         }
-        return results.stream().map(pair -> (Pair<TableDesc, TableExtDesc>) 
pair.getSecond())
-                .collect(Collectors.toList());
+        return successResults;
     }
 
     public List<String> getSourceDbNames(String project) throws Exception {
diff --git 
a/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableExtServiceTest.java
 
b/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableExtServiceTest.java
index 2195d90ca6..8574410b0e 100644
--- 
a/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableExtServiceTest.java
+++ 
b/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableExtServiceTest.java
@@ -95,7 +95,7 @@ public class TableExtServiceTest extends 
NLocalFileMetadataTestCase {
         String[] tables = { "DEFAULT.TEST_KYLIN_FACT", "DEFAULT.TEST_ACCOUNT" 
};
         String[] tableNames = { "TEST_KYLIN_FACT", "TEST_ACCOUNT" };
         List<Pair<TableDesc, TableExtDesc>> result = mockTablePair(2, 
"DEFAULT");
-        
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any());
+        
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any(), Mockito.any());
         
Mockito.doNothing().when(tableExtService).loadTable(result.get(0).getFirst(), 
result.get(0).getSecond(),
                 "default");
         
Mockito.doNothing().when(tableExtService).loadTable(result.get(1).getFirst(), 
result.get(1).getSecond(),
@@ -123,7 +123,7 @@ public class TableExtServiceTest extends 
NLocalFileMetadataTestCase {
         crossAccountTableReq.add(s3TableExtInfo1);
         crossAccountTableReq.add(s3TableExtInfo2);
         List<Pair<TableDesc, TableExtDesc>> result = mockTablePair(2, 
"DEFAULT", "TABLE");
-        
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any());
+        
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any(), Mockito.any());
         
Mockito.doNothing().when(tableExtService).loadTable(result.get(0).getFirst(), 
result.get(0).getSecond(),
                 "default");
         
Mockito.doNothing().when(tableExtService).loadTable(result.get(1).getFirst(), 
result.get(1).getSecond(),
@@ -193,7 +193,7 @@ public class TableExtServiceTest extends 
NLocalFileMetadataTestCase {
         List<Pair<TableDesc, TableExtDesc>> result = mockTablePair(3, "EDW");
         
Mockito.doNothing().when(tableExtService).loadTable(result.get(1).getFirst(), 
result.get(1).getSecond(),
                 "default");
-        
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any());
+        
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any(), Mockito.any());
         loadTableResponse.setLoaded(Sets.newHashSet(tableIdentities));
 
         
Mockito.doReturn(Lists.newArrayList(tableNames)).when(tableService).getSourceTableNames(Mockito.any(),
@@ -220,7 +220,7 @@ public class TableExtServiceTest extends 
NLocalFileMetadataTestCase {
         NTableMetadataManager tableManager = 
NTableMetadataManager.getInstance(getTestConfig(), "default");
         tableManager.removeSourceTable("EDW.TEST_CAL_DT");
         
Mockito.doReturn(Lists.newArrayList("EDW")).when(tableService).getSourceDbNames("default");
-        
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any());
+        
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any(), Mockito.any());
         LoadTableResponse response = tableExtService.loadDbTables(new String[] 
{ "EDW" }, "default", true);
         Assert.assertEquals(0, response.getLoaded().size());
     }
@@ -551,7 +551,7 @@ public class TableExtServiceTest extends 
NLocalFileMetadataTestCase {
     @Test
     public void testLoadTablesWithShortCircuit() throws Exception {
         List<Pair<TableDesc, TableExtDesc>> lt1000 = mockTablePair(8, "TB");
-        
Mockito.doReturn(lt1000).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any());
+        
Mockito.doReturn(lt1000).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any(), Mockito.any());
         TableLoadRequest request = new TableLoadRequest();
         request.setDatabases(new String[] { "DEFAULT" });
         request.setProject("default");
@@ -559,7 +559,7 @@ public class TableExtServiceTest extends 
NLocalFileMetadataTestCase {
         Assert.assertEquals(8, lt1000response.getFailed().size());
 
         List<Pair<TableDesc, TableExtDesc>> gt1000 = mockTablePair(1001, "TB");
-        
Mockito.doReturn(gt1000).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any());
+        
Mockito.doReturn(gt1000).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any(), Mockito.any());
         Assert.assertThrows(KylinException.class, () -> 
tableExtService.loadTablesWithShortCircuit(request));
 
         request.setTables(mockInputDBOrTable());
@@ -571,13 +571,13 @@ public class TableExtServiceTest extends 
NLocalFileMetadataTestCase {
         request.setDatabases(null);
         gt1000.forEach(
                 t -> 
Mockito.doNothing().when(tableExtService).loadTable(t.getFirst(), 
t.getSecond(), "default"));
-        
Mockito.doReturn(gt1000).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any());
+        
Mockito.doReturn(gt1000).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any(), Mockito.any());
         Assert.assertThrows(KylinException.class, () -> 
tableExtService.loadTablesWithShortCircuit(request));
 
         request.setDatabases(null);
         request.setTables(new String[] { "TEST_KYLIN_FACT" });
         List<Pair<TableDesc, TableExtDesc>> table8 = mockTablePair(8, "TB");
-        
Mockito.doReturn(table8).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any());
+        
Mockito.doReturn(table8).when(tableService).extractTableMeta(Mockito.any(), 
Mockito.any(), Mockito.any());
         LoadTableResponse response1 = 
tableExtService.loadTablesWithShortCircuit(request);
         Assert.assertEquals(8, response1.getFailed().size());
 
diff --git 
a/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableServiceTest.java
 
b/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableServiceTest.java
new file mode 100644
index 0000000000..29193b9c6b
--- /dev/null
+++ 
b/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableServiceTest.java
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kylin.rest.service;
+
+import java.io.IOException;
+import java.security.AccessControlException;
+import java.util.List;
+
+import org.apache.kylin.common.exception.KylinException;
+import org.apache.kylin.common.util.NLocalFileMetadataTestCase;
+import org.apache.kylin.common.util.Pair;
+import org.apache.kylin.engine.spark.mockup.CsvSource;
+import org.apache.kylin.metadata.model.TableDesc;
+import org.apache.kylin.metadata.model.TableExtDesc;
+import org.apache.kylin.rest.constant.Constant;
+import org.apache.kylin.rest.response.LoadTableResponse;
+import org.apache.kylin.rest.util.AclEvaluate;
+import org.apache.kylin.rest.util.AclUtil;
+import org.apache.kylin.source.ISourceMetadataExplorer;
+import org.apache.kylin.source.SourceFactory;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.springframework.security.authentication.TestingAuthenticationToken;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.test.util.ReflectionTestUtils;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ SourceFactory.class, ISourceMetadataExplorer.class })
+@PowerMockIgnore({ "com.sun.security.*", "org.w3c.*", "javax.xml.*", 
"org.xml.*", "org.w3c.dom.*", "org.apache.cxf.*",
+        "javax.management.*", "javax.script.*", 
"org.apache.hadoop.security.*", "org.apache.hadoop.*",
+        "javax.security.*", "java.security.*", "javax.crypto.*", 
"javax.net.ssl.*" })
+public class TableServiceTest extends NLocalFileMetadataTestCase {
+
+    @Mock
+    private final TableService tableService = Mockito.spy(TableService.class);
+
+    @Mock
+    private final AclEvaluate aclEvaluate = Mockito.spy(AclEvaluate.class);
+
+    @InjectMocks
+    private final TableExtService tableExtService = Mockito.spy(new 
TableExtService());
+
+    @Before
+    public void setup() throws IOException {
+        overwriteSystemProp("HADOOP_USER_NAME", "root");
+        createTestMetadata();
+        SecurityContextHolder.getContext()
+                .setAuthentication(new TestingAuthenticationToken("ADMIN", 
"ADMIN", Constant.ROLE_ADMIN));
+        ReflectionTestUtils.setField(aclEvaluate, "aclUtil", 
Mockito.spy(AclUtil.class));
+        ReflectionTestUtils.setField(tableService, "aclEvaluate", aclEvaluate);
+        ReflectionTestUtils.setField(tableExtService, "aclEvaluate", 
aclEvaluate);
+        ReflectionTestUtils.setField(tableExtService, "tableService", 
tableService);
+    }
+
+    @After
+    public void tearDown() {
+        cleanupTestMetadata();
+    }
+
+    @Test
+    public void testLoadTablesSuccessfully() {
+        String[] tables = { "DEFAULT.TEST_KYLIN_FACT", "DEFAULT.TEST_ACCOUNT" 
};
+        LoadTableResponse tableResponse = new LoadTableResponse();
+        List<Pair<TableDesc, TableExtDesc>> canLoadTables = 
tableService.extractTableMeta(tables, "default",
+                tableResponse);
+        Assert.assertEquals(0, tableResponse.getFailed().size());
+        Assert.assertEquals(2, canLoadTables.size());
+    }
+
+    @Test
+    public void testLoadTablesError() throws Exception {
+        String[] tables = { "DEFAULT.TEST_KYLIN_FACT", "DEFAULT.TEST_ACCOUNT" 
};
+        LoadTableResponse tableResponse = new LoadTableResponse();
+        CsvSource csvSource = PowerMockito.mock(CsvSource.class);
+        PowerMockito.whenNew(CsvSource.class);
+        ISourceMetadataExplorer mockExplorer = 
Mockito.mock(ISourceMetadataExplorer.class);
+        PowerMockito.mockStatic(SourceFactory.class);
+        PowerMockito.doReturn(csvSource).when(SourceFactory.class, 
"getSource", Mockito.any());
+        
Mockito.when((csvSource).getSourceMetadataExplorer()).thenReturn(mockExplorer);
+        Mockito.when(mockExplorer.loadTableMetadata(Mockito.anyString(), 
Mockito.anyString(), Mockito.anyString()))
+                .thenThrow(new AccessControlException("Mock can not fetch 
table meta"));
+        List<Pair<TableDesc, TableExtDesc>> canLoadTables = 
tableService.extractTableMeta(tables, "default",
+                tableResponse);
+        Assert.assertEquals(2, tableResponse.getFailed().size());
+        Assert.assertEquals(0, canLoadTables.size());
+
+        Assert.assertThrows(KylinException.class, () -> {
+            tableService.extractTableMeta(tables, "default");
+        });
+    }
+
+    @Test
+    public void testLoadTablesErrorAndThrowException() throws Exception {
+        String[] tables = { "DEFAULT.TEST_KYLIN_FACT", "DEFAULT.TEST_ACCOUNT" 
};
+        CsvSource csvSource = PowerMockito.mock(CsvSource.class);
+        PowerMockito.whenNew(CsvSource.class);
+        ISourceMetadataExplorer mockExplorer = 
Mockito.mock(ISourceMetadataExplorer.class);
+        PowerMockito.mockStatic(SourceFactory.class);
+        PowerMockito.doReturn(csvSource).when(SourceFactory.class, 
"getSource", Mockito.any());
+        
Mockito.when((csvSource).getSourceMetadataExplorer()).thenReturn(mockExplorer);
+        Mockito.when(mockExplorer.loadTableMetadata(Mockito.anyString(), 
Mockito.anyString(), Mockito.anyString()))
+                .thenThrow(new AccessControlException("Mock can not fetch 
table meta"));
+        Assert.assertThrows(KylinException.class, () -> {
+            tableService.extractTableMeta(tables, "default");
+        });
+    }
+}
diff --git 
a/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java
 
b/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java
index 3d2fddf9aa..07a9aa6c3d 100644
--- 
a/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java
+++ 
b/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java
@@ -36,6 +36,7 @@ import org.apache.commons.lang3.StringUtils;
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.exception.KylinException;
 import org.apache.kylin.common.persistence.transaction.UnitOfWork;
+import org.apache.kylin.common.util.AddressUtil;
 import org.apache.kylin.common.util.Pair;
 import org.apache.kylin.guava30.shaded.common.annotations.VisibleForTesting;
 import org.apache.kylin.guava30.shaded.common.collect.Maps;
@@ -134,6 +135,7 @@ public class OpsController extends NBasicController {
             @RequestBody DiagPackageRequest diagPackageRequest, @RequestHeader 
HttpHeaders headers,
             final HttpServletRequest request) throws Exception {
         host = decodeHost(host);
+        AddressUtil.validateHost(host);
         if (StringUtils.isNotBlank(diagPackageRequest.getJobId())) {
             diagPackageRequest.setStart("");
             diagPackageRequest.setEnd("");
@@ -162,6 +164,7 @@ public class OpsController extends NBasicController {
             @RequestBody QueryDiagPackageRequest queryDiagPackageRequest, 
@RequestHeader HttpHeaders headers,
             final HttpServletRequest request) throws Exception {
         host = decodeHost(host);
+        AddressUtil.validateHost(host);
         if (StringUtils.isEmpty(host) || 
KylinConfig.getInstanceFromEnv().getMicroServiceMode() != null) {
             String uuid = 
systemService.dumpLocalQueryDiagPackage(queryDiagPackageRequest.getQueryId(),
                     queryDiagPackageRequest.getProject(), headers);
@@ -188,6 +191,7 @@ public class OpsController extends NBasicController {
             @RequestParam(value = "project", required = false) String project, 
final HttpServletRequest request)
             throws Exception {
         host = decodeHost(host);
+        AddressUtil.validateHost(host);
         if (StringUtils.isEmpty(host) || 
KylinConfig.getInstanceFromEnv().getMicroServiceMode() != null) {
             return systemService.getExtractorStatus(id, project);
         } else {
@@ -206,6 +210,7 @@ public class OpsController extends NBasicController {
             @RequestParam(value = "id") String id, @RequestParam(value = 
"project", required = false) String project,
             final HttpServletRequest request, final HttpServletResponse 
response) throws IOException {
         host = decodeHost(host);
+        AddressUtil.validateHost(host);
         if (StringUtils.isEmpty(host) || 
KylinConfig.getInstanceFromEnv().getMicroServiceMode() != null) {
             setDownloadResponse(systemService.getDiagPackagePath(id, project), 
MediaType.APPLICATION_OCTET_STREAM_VALUE,
                     response);
@@ -224,6 +229,7 @@ public class OpsController extends NBasicController {
     public EnvelopeResponse<String> remoteStopPackage(@RequestParam(value = 
"host", required = false) String host,
             @RequestParam(value = "id") String id, final HttpServletRequest 
request) throws Exception {
         host = decodeHost(host);
+        AddressUtil.validateHost(host);
         if (StringUtils.isEmpty(host) || 
KylinConfig.getInstanceFromEnv().getMicroServiceMode() != null) {
             systemService.stopDiagTask(id);
             return new EnvelopeResponse<>(CODE_SUCCESS, "", "");
diff --git 
a/src/query-service/src/main/java/org/apache/kylin/rest/service/SparderUIService.java
 
b/src/query-service/src/main/java/org/apache/kylin/rest/service/SparderUIService.java
index 7c3c675447..bb7f22d696 100644
--- 
a/src/query-service/src/main/java/org/apache/kylin/rest/service/SparderUIService.java
+++ 
b/src/query-service/src/main/java/org/apache/kylin/rest/service/SparderUIService.java
@@ -37,6 +37,7 @@ import org.apache.commons.lang3.StringUtils;
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.exception.ErrorCode;
 import org.apache.kylin.common.msg.MsgPicker;
+import org.apache.kylin.common.util.AddressUtil;
 import org.apache.kylin.rest.util.AclEvaluate;
 import org.apache.kylin.rest.util.SparderUIUtil;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -83,6 +84,7 @@ public class SparderUIService extends BasicService {
         val server = getServer(servletRequest);
         if (StringUtils.isNotBlank(server) && 
!TRUE.equalsIgnoreCase(servletRequest.getHeader(ROUTED))
                 && routeService.needRoute()) {
+            AddressUtil.validateHost(server);
             log.info("proxy sparder UI to server : [{}]", server);
             val queryString = servletRequest.getQueryString();
             proxyToServer(server, queryString, restTemplate, servletRequest, 
servletResponse);
@@ -111,6 +113,7 @@ public class SparderUIService extends BasicService {
         }
         if (StringUtils.isNotBlank(realServer) && 
!TRUE.equalsIgnoreCase(servletRequest.getHeader(ROUTED))
                 && routeService.needRoute()) {
+            AddressUtil.validateHost(realServer);
             log.info("proxy sparder UI to server : [{}] queryId : [{}] Id : 
[{}]", realServer, queryId, id);
             val queryString = "id=" + id;
             proxyToServer(realServer, queryString, restTemplate, 
servletRequest, servletResponse);
diff --git 
a/src/query-service/src/test/java/org/apache/kylin/rest/service/SparderUIServiceTest.java
 
b/src/query-service/src/test/java/org/apache/kylin/rest/service/SparderUIServiceTest.java
index 39e67bf6cb..ea7409d02d 100644
--- 
a/src/query-service/src/test/java/org/apache/kylin/rest/service/SparderUIServiceTest.java
+++ 
b/src/query-service/src/test/java/org/apache/kylin/rest/service/SparderUIServiceTest.java
@@ -20,7 +20,9 @@ package org.apache.kylin.rest.service;
 
 import static org.apache.commons.net.util.Base64.encodeBase64;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import java.nio.charset.Charset;
 import java.util.stream.Collectors;
@@ -101,6 +103,38 @@ public class SparderUIServiceTest {
 
     }
 
+    @Test
+    public void proxyFail() {
+        {
+            try {
+                Mockito.when(routeService.needRoute()).thenReturn(true);
+                val req = new MockHttpServletRequest();
+                req.addHeader("routed", false);
+                setCookie(true, req, "127.0.0.1:7979");
+                val res = new MockHttpServletResponse();
+                sparderUIService.proxy("123", "321", "127.0.0.1:7979>", req, 
res);
+                fail();
+            } catch (Exception e) {
+                assertInstanceOf(IllegalArgumentException.class, e);
+                assertTrue(e.getMessage().contains("Url contains disallowed 
chars, host: "));
+            }
+        }
+        {
+            try {
+                Mockito.when(routeService.needRoute()).thenReturn(true);
+                val req = new MockHttpServletRequest();
+                setCookie(true, req, "127.0.0.1:7979>");
+                req.addHeader("routed", false);
+                val res = new MockHttpServletResponse();
+                sparderUIService.proxy(req, res);
+                fail();
+            } catch (Exception e) {
+                assertInstanceOf(IllegalArgumentException.class, e);
+                assertTrue(e.getMessage().contains("Url contains disallowed 
chars, host: "));
+            }
+        }
+    }
+
     @Test
     public void proxy() throws Exception {
         proxyInner();
@@ -132,14 +166,7 @@ public class SparderUIServiceTest {
             ((Logger) LogManager.getRootLogger()).addAppender(appender);
             Mockito.when(routeService.needRoute()).thenReturn(needRoute);
             val req = new MockHttpServletRequest();
-            if (needCookie) {
-                val server = "127.0.0.1:7070";
-                val serverBytes = server.getBytes(Charset.defaultCharset());
-                val serverCookie = new String(encodeBase64(serverBytes), 
Charset.defaultCharset());
-                Cookie cookie = new Cookie("server", serverCookie);
-                cookie.setPath(SparderUIUtil.KYLIN_UI_BASE);
-                req.setCookies(cookie);
-            }
+            setCookie(needCookie, req, "127.0.0.1:7070");
             req.addHeader("routed", isRouted);
             val res = new MockHttpServletResponse();
             sparderUIService.proxy(req, res);
@@ -169,13 +196,7 @@ public class SparderUIServiceTest {
             ((Logger) LogManager.getRootLogger()).addAppender(appender);
             Mockito.when(routeService.needRoute()).thenReturn(needRoute);
             val req = new MockHttpServletRequest();
-            if (needCookie) {
-                val serverBytes = 
"127.0.0.1:7979".getBytes(Charset.defaultCharset());
-                val serverCookie = new String(encodeBase64(serverBytes), 
Charset.defaultCharset());
-                Cookie cookie = new Cookie("server", serverCookie);
-                cookie.setPath(SparderUIUtil.KYLIN_UI_BASE);
-                req.setCookies(cookie);
-            }
+            setCookie(needCookie, req, "127.0.0.1:7979");
             req.addHeader("routed", isRouted);
             val res = new MockHttpServletResponse();
             sparderUIService.proxy("123", "321", server, req, res);
@@ -197,4 +218,14 @@ public class SparderUIServiceTest {
             ((Logger) LogManager.getRootLogger()).removeAppender(appender);
         }
     }
+
+    private void setCookie(boolean needCookie, MockHttpServletRequest req, 
String server) {
+        if (needCookie) {
+            val serverBytes = server.getBytes(Charset.defaultCharset());
+            val serverCookie = new String(encodeBase64(serverBytes), 
Charset.defaultCharset());
+            Cookie cookie = new Cookie("server", serverCookie);
+            cookie.setPath(SparderUIUtil.KYLIN_UI_BASE);
+            req.setCookies(cookie);
+        }
+    }
 }


Reply via email to