KYLIN-3893 Add validation for the encoding length and the type of enc…

…oding (apache#622) * KYLIN-3893 Add validation for the encoding length and the type of encoding * KYLIN-3893 Code review
TimGong · May 30, 2019 · 475df34 · 475df34
1 parent b089352
commit 475df34
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 2 deletions.
diff --git a/core-cube/src/main/java/org/apache/kylin/cube/kv/CubeDimEncMap.java b/core-cube/src/main/java/org/apache/kylin/cube/kv/CubeDimEncMap.java
@@ -18,7 +18,8 @@
 
 package org.apache.kylin.cube.kv;
 
-import com.google.common.collect.Maps;
+import java.util.Map;
+
 import org.apache.kylin.common.util.Dictionary;
 import org.apache.kylin.cube.CubeSegment;
 import org.apache.kylin.cube.model.CubeDesc;
@@ -32,7 +33,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Map;
+import com.google.common.collect.Maps;
 
 public class CubeDimEncMap implements IDimensionEncodingMap, java.io.Serializable {
 

diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/BooleanDimEnc.java b/core-metadata/src/main/java/org/apache/kylin/dimension/BooleanDimEnc.java
@@ -60,6 +60,11 @@ public String getSupportedEncodingName() {
         public DimensionEncoding createDimensionEncoding(String encodingName, String[] args) {
             return new BooleanDimEnc();
         }
+
+        @Override
+        public boolean isValidDimensionEncoding(String encodingName, String[] args) {
+            return true;
+        }
     };
 
     // ============================================================================

diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/DateDimEnc.java b/core-metadata/src/main/java/org/apache/kylin/dimension/DateDimEnc.java
@@ -44,6 +44,22 @@ public String getSupportedEncodingName() {
         public DimensionEncoding createDimensionEncoding(String encodingName, String[] args) {
             return new DateDimEnc(args);
         }
+
+        @Override
+        public boolean isValidDimensionEncoding(String encodingName, String[] args) {
+            try {
+                if (args != null && args.length == 1) {
+                    DataType datatype = DataType.getType(args[0]);
+                    if (datatype == null) {
+                        return false;
+                    }
+                }
+            } catch (Exception e) {
+                return false;
+            }
+
+            return true;
+        }
     };
 
     private static class DateDimValueCodec implements IValueCodec {

diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/DimensionEncodingFactory.java b/core-metadata/src/main/java/org/apache/kylin/dimension/DimensionEncodingFactory.java
@@ -97,6 +97,18 @@ public boolean apply(@Nullable Pair<String, Integer> input) {
                 });
     }
 
+    public static boolean isValidEncoding(final String encodingName, String[] args, int version) {
+        if (factoryMap == null)
+            initFactoryMap();
+
+        DimensionEncodingFactory factory = factoryMap.get(Pair.newPair(encodingName, version));
+        if (factory == null) {
+            return true;
+        }
+
+        return factory.isValidDimensionEncoding(encodingName, args);
+    }
+
     private static synchronized void initFactoryMap() {
         if (factoryMap == null) {
             Map<Pair<String, Integer>, DimensionEncodingFactory> map = Maps.newConcurrentMap();
@@ -155,4 +167,9 @@ private static synchronized void initFactoryMap() {
      * Create a DimensionEncoding instance, with inputs corresponding to RowKeyColDesc.encodingName and RowKeyColDesc.encodingArgs
      */
     abstract public DimensionEncoding createDimensionEncoding(String encodingName, String[] args);
+
+    /**
+     * Validate the inputs of RowKeyColDesc.encodingName and RowKeyColDesc.encodingArgs for a DimensionEncoding instance
+     */
+    abstract public boolean isValidDimensionEncoding(String encodingName, String[] args);
 }
diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/FixedLenDimEnc.java b/core-metadata/src/main/java/org/apache/kylin/dimension/FixedLenDimEnc.java
@@ -50,6 +50,16 @@ public String getSupportedEncodingName() {
         public DimensionEncoding createDimensionEncoding(String encodingName, String[] args) {
             return new FixedLenDimEnc(Integer.parseInt(args[0]));
         }
+
+        @Override
+        public boolean isValidDimensionEncoding(String encodingName, String[] args) {
+            try {
+                int length = Integer.parseInt(args[0]);
+                return length >= 1;
+            } catch (Exception e) {
+                return false;
+            }
+        }
     };
 
     // ============================================================================

diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/FixedLenHexDimEnc.java b/core-metadata/src/main/java/org/apache/kylin/dimension/FixedLenHexDimEnc.java
@@ -112,6 +112,16 @@ public String getSupportedEncodingName() {
         public DimensionEncoding createDimensionEncoding(String encodingName, String[] args) {
             return new FixedLenHexDimEnc(Integer.parseInt(args[0]));
         }
+
+        @Override
+        public boolean isValidDimensionEncoding(String encodingName, String[] args) {
+            try {
+                int length = Integer.parseInt(args[0]);
+                return length >= 1;
+            } catch (Exception e) {
+                return false;
+            }
+        }
     }
 
     // ============================================================================

diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/IntDimEnc.java b/core-metadata/src/main/java/org/apache/kylin/dimension/IntDimEnc.java
@@ -54,6 +54,16 @@ public String getSupportedEncodingName() {
         public DimensionEncoding createDimensionEncoding(String encodingName, String[] args) {
             return new IntDimEnc(Integer.parseInt(args[0]));
         }
+
+        @Override
+        public boolean isValidDimensionEncoding(String encodingName, String[] args) {
+            try {
+                int length = Integer.parseInt(args[0]);
+                return length >= 1 && length < CAP.length;
+            } catch (Exception e) {
+                return false;
+            }
+        }
     };
 
     // ============================================================================

diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/IntegerDimEnc.java b/core-metadata/src/main/java/org/apache/kylin/dimension/IntegerDimEnc.java
@@ -64,6 +64,16 @@ public String getSupportedEncodingName() {
         public DimensionEncoding createDimensionEncoding(String encodingName, String[] args) {
             return new IntegerDimEnc(Integer.parseInt(args[0]));
         }
+
+        @Override
+        public boolean isValidDimensionEncoding(String encodingName, String[] args) {
+            try {
+                int length = Integer.parseInt(args[0]);
+                return length >= 1 && length < CAP.length;
+            } catch (Exception e) {
+                return false;
+            }
+        }
     };
 
     // ============================================================================

diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/OneMoreByteVLongDimEnc.java b/core-metadata/src/main/java/org/apache/kylin/dimension/OneMoreByteVLongDimEnc.java
@@ -59,6 +59,16 @@ public String getSupportedEncodingName() {
         public DimensionEncoding createDimensionEncoding(String encodingName, String[] args) {
             return new OneMoreByteVLongDimEnc(Integer.parseInt(args[0]));
         }
+
+        @Override
+        public boolean isValidDimensionEncoding(String encodingName, String[] args) {
+            try {
+                int length = Integer.parseInt(args[0]);
+                return length >= 1 && length < CAP.length;
+            } catch (Exception e) {
+                return false;
+            }
+        }
     };
 
     // ============================================================================

diff --git a/core-metadata/src/main/java/org/apache/kylin/dimension/TimeDimEnc.java b/core-metadata/src/main/java/org/apache/kylin/dimension/TimeDimEnc.java
@@ -40,6 +40,11 @@ public String getSupportedEncodingName() {
         public DimensionEncoding createDimensionEncoding(String encodingName, String[] args) {
             return new TimeDimEnc();
         }
+
+        @Override
+        public boolean isValidDimensionEncoding(String encodingName, String[] args) {
+            return true;
+        }
     };
 
     public TimeDimEnc() {

diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/CubeController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/CubeController.java
@@ -45,6 +45,7 @@
 import org.apache.kylin.cube.model.HBaseColumnDesc;
 import org.apache.kylin.cube.model.HBaseColumnFamilyDesc;
 import org.apache.kylin.cube.model.RowKeyColDesc;
+import org.apache.kylin.dimension.DimensionEncoding;
 import org.apache.kylin.dimension.DimensionEncodingFactory;
 import org.apache.kylin.engine.mr.common.CuboidStatsReaderUtil;
 import org.apache.kylin.job.JobInstance;
@@ -625,6 +626,16 @@ private void validateColumnFamily(CubeDesc cubeDesc) {
             throw new BadRequestException(
                     "the number of input measure and the number of measure defined in cubedesc are not consistent");
         }
+
+        for (RowKeyColDesc rowKeyColDesc : cubeDesc.getRowkey().getRowKeyColumns()) {
+            Object[] encodingConf = DimensionEncoding.parseEncodingConf(rowKeyColDesc.getEncoding());
+            String encodingName = (String) encodingConf[0];
+            String[] encodingArgs = (String[]) encodingConf[1];
+
+            if (!DimensionEncodingFactory.isValidEncoding(encodingName, encodingArgs, rowKeyColDesc.getEncodingVersion())) {
+                throw new BadRequestException("Illegal row key column desc: " + rowKeyColDesc);
+            }
+        }
     }
 
     /**