From 503b6ee4da794ee23ccced74b0f92e793665ecbb Mon Sep 17 00:00:00 2001 From: Zhengguo Yang Date: Tue, 10 Jan 2023 17:44:18 +0800 Subject: [PATCH] [chore](vulnerability) fix fe high risk vulnerability scanned by bug scanner (#15649) --- .../apache/doris/common/io/BitmapValue.java | 16 ++++--- .../doris/common/jmockit/FieldReflection.java | 2 +- .../common/jmockit/MethodReflection.java | 2 +- .../doris/analysis/BinaryPredicate.java | 5 +-- .../doris/analysis/ColumnPartitionDesc.java | 3 ++ .../doris/analysis/CreateTableStmt.java | 4 +- .../java/org/apache/doris/catalog/Type.java | 2 +- .../clone/DynamicPartitionScheduler.java | 4 +- .../common/util/DynamicPartitionUtil.java | 5 +-- .../deploy/impl/LocalFileDeployManager.java | 2 +- .../IcebergTableCreationRecordMgr.java | 2 +- .../doris/httpv2/rest/GetLogFileAction.java | 2 +- .../java/org/apache/doris/load/DppConfig.java | 43 +++++++++++++++++++ .../org/apache/doris/load/EtlJobInfo.java | 12 +++++- .../persist/TableAddOrDropColumnsInfo.java | 2 +- .../doris/persist/TableRenameColumnInfo.java | 2 +- .../doris/planner/SingleNodePlanner.java | 2 +- .../apache/doris/catalog/CatalogTestUtil.java | 2 +- .../doris/planner/TableFunctionPlanTest.java | 2 +- 19 files changed, 86 insertions(+), 28 deletions(-) diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/io/BitmapValue.java b/fe/fe-common/src/main/java/org/apache/doris/common/io/BitmapValue.java index cd6de7b3de..0ed342cf6f 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/common/io/BitmapValue.java +++ b/fe/fe-common/src/main/java/org/apache/doris/common/io/BitmapValue.java @@ -320,20 +320,24 @@ public class BitmapValue { } } - public boolean equals(BitmapValue other) { - boolean ret = false; - if (this.bitmapType != other.bitmapType) { + @Override + public boolean equals(Object other) { + if (other == null || !(other instanceof BitmapValue)) { return false; } - switch (other.bitmapType) { // CHECKSTYLE IGNORE THIS LINE: missing switch default + boolean ret = false; + if (this.bitmapType != ((BitmapValue) other).bitmapType) { + return false; + } + switch (((BitmapValue) other).bitmapType) { // CHECKSTYLE IGNORE THIS LINE: missing switch default case EMPTY: ret = true; break; case SINGLE_VALUE: - ret = this.singleValue == other.singleValue; + ret = this.singleValue == ((BitmapValue) other).singleValue; break; case BITMAP_VALUE: - ret = bitmap.equals(other.bitmap); + ret = bitmap.equals(((BitmapValue) other).bitmap); } return ret; } diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/jmockit/FieldReflection.java b/fe/fe-common/src/main/java/org/apache/doris/common/jmockit/FieldReflection.java index 1974d4f53b..084b5bec1a 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/common/jmockit/FieldReflection.java +++ b/fe/fe-common/src/main/java/org/apache/doris/common/jmockit/FieldReflection.java @@ -281,7 +281,7 @@ public final class FieldReflection { private static boolean isSameType(Class firstType, Class secondType) { return firstType == secondType || firstType.isPrimitive() && firstType == AutoType.getPrimitiveType(secondType) - || secondType.isPrimitive() && firstType == AutoType.getPrimitiveType(secondType); + || secondType.isPrimitive() && secondType == AutoType.getPrimitiveType(firstType); } } diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/jmockit/MethodReflection.java b/fe/fe-common/src/main/java/org/apache/doris/common/jmockit/MethodReflection.java index 47bd5e5f2a..749e2e7cca 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/common/jmockit/MethodReflection.java +++ b/fe/fe-common/src/main/java/org/apache/doris/common/jmockit/MethodReflection.java @@ -158,6 +158,6 @@ public final class MethodReflection { private static boolean isSameType(Class firstType, Class secondType) { return firstType == secondType || firstType.isPrimitive() && firstType == AutoType.getPrimitiveType(secondType) - || secondType.isPrimitive() && firstType == AutoType.getPrimitiveType(secondType); + || secondType.isPrimitive() && secondType == AutoType.getPrimitiveType(firstType); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java index cb709eee86..932bc7f2bb 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java @@ -357,9 +357,8 @@ public class BinaryPredicate extends Predicate implements Writable { if (t1 == PrimitiveType.VARCHAR && t2 == PrimitiveType.VARCHAR) { return Type.VARCHAR; } - if (t1 == PrimitiveType.STRING && t2 == PrimitiveType.STRING - || t1 == PrimitiveType.STRING && t2 == PrimitiveType.VARCHAR - || t1 == PrimitiveType.VARCHAR && t2 == PrimitiveType.STRING) { + if ((t1 == PrimitiveType.STRING && (t2 == PrimitiveType.VARCHAR || t2 == PrimitiveType.STRING)) || ( + t2 == PrimitiveType.STRING && (t1 == PrimitiveType.VARCHAR || t1 == PrimitiveType.STRING))) { return Type.STRING; } if (t1 == PrimitiveType.BIGINT && t2 == PrimitiveType.BIGINT) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnPartitionDesc.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnPartitionDesc.java index 0ad8aa1be4..f8f4c07e2b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnPartitionDesc.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnPartitionDesc.java @@ -66,6 +66,9 @@ public class ColumnPartitionDesc extends PartitionDesc { } } } + if (matched == null) { + throw new AnalysisException("The partition columns doesn't match the ones in base table."); + } PartitionInfo partitionInfo = matched.getPartitionInfo(); List partitionColumns = partitionInfo.getPartitionColumns(); if (!columns.stream().map(SlotRef::getColumn).collect(Collectors.toList()).equals(partitionColumns)) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java index 471dc451c4..37d73d7bb0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java @@ -571,9 +571,7 @@ public class CreateTableStmt extends DdlStmt { } } sb.append("\n)"); - if (engineName != null) { - sb.append(" ENGINE = ").append(engineName); - } + sb.append(" ENGINE = ").append(engineName); if (keysDesc != null) { sb.append("\n").append(keysDesc.toSql()); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java index 16eef75265..d80aa4fc22 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java @@ -782,7 +782,7 @@ public abstract class Type { || scalarType.getType() == TPrimitiveType.DECIMAL64 || scalarType.getType() == TPrimitiveType.DECIMAL128I) { Preconditions.checkState(scalarType.isSetPrecision() - && scalarType.isSetPrecision()); + && scalarType.isSetScale()); type = ScalarType.createDecimalV3Type(scalarType.getPrecision(), scalarType.getScale()); } else if (scalarType.getType() == TPrimitiveType.DATETIMEV2) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java b/fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java index 2d8408cc06..754126caaa 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java +++ b/fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java @@ -347,6 +347,8 @@ public class DynamicPartitionScheduler extends MasterDaemon { reservedHistoryPartitionKeyRangeList.add(reservedHistoryPartitionKeyRange); } catch (IllegalArgumentException e) { return dropPartitionClauses; + } catch (AnalysisException e) { + throw new DdlException(e.getMessage()); } } } @@ -390,7 +392,7 @@ public class DynamicPartitionScheduler extends MasterDaemon { } ArrayList addPartitionClauses = new ArrayList<>(); - ArrayList dropPartitionClauses = null; + ArrayList dropPartitionClauses = new ArrayList<>(); String tableName = null; boolean skipAddPartition = false; OlapTable olapTable; diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java index c0df533032..60247b4ec7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java @@ -729,7 +729,7 @@ public class DynamicPartitionUtil { } public static String getHistoryPartitionRangeString(DynamicPartitionProperty dynamicPartitionProperty, - String time, String format) { + String time, String format) throws AnalysisException { ZoneId zoneId = dynamicPartitionProperty.getTimeZone().toZoneId(); Date date = null; Timestamp timestamp = null; @@ -740,8 +740,7 @@ public class DynamicPartitionUtil { date = simpleDateFormat.parse(time); } catch (ParseException e) { LOG.warn("Parse dynamic partition periods error. Error={}", e.getMessage()); - return getFormattedTimeWithoutMinuteSecond( - ZonedDateTime.parse(timestamp.toString(), dateTimeFormatter), format); + throw new AnalysisException("Parse dynamic partition periods error. Error=" + e.getMessage()); } timestamp = new Timestamp(date.getTime()); return getFormattedTimeWithoutMinuteSecond( diff --git a/fe/fe-core/src/main/java/org/apache/doris/deploy/impl/LocalFileDeployManager.java b/fe/fe-core/src/main/java/org/apache/doris/deploy/impl/LocalFileDeployManager.java index 4ff49901cb..e5023a8347 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/deploy/impl/LocalFileDeployManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/deploy/impl/LocalFileDeployManager.java @@ -133,7 +133,7 @@ public class LocalFileDeployManager extends DeployManager { LOG.warn("failed to close buffered reader after reading file: {}", clusterInfoFile, e); } } - if (lock != null && channel.isOpen()) { + if (lock != null) { try { lock.release(); } catch (IOException e) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/external/iceberg/IcebergTableCreationRecordMgr.java b/fe/fe-core/src/main/java/org/apache/doris/external/iceberg/IcebergTableCreationRecordMgr.java index 475215c327..a98dd7f2ab 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/external/iceberg/IcebergTableCreationRecordMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/external/iceberg/IcebergTableCreationRecordMgr.java @@ -152,7 +152,7 @@ public class IcebergTableCreationRecordMgr extends MasterDaemon { try { icebergTables = icebergCatalog.listTables(icebergProperty.getDatabase()); - } catch (DorisIcebergException e) { + } catch (Exception e) { addTableCreationRecord(db.getId(), -1, db.getFullName(), "", FAIL, prop.writeTimeFormat(new Date(System.currentTimeMillis())), e.getMessage()); LOG.warn("Failed list remote Iceberg database, hive.metastore.uris[{}], database[{}], error: {}", diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/GetLogFileAction.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/GetLogFileAction.java index 9c97f2ee90..9535cc650d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/GetLogFileAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/GetLogFileAction.java @@ -88,7 +88,7 @@ public class GetLogFileAction extends RestBaseController { return ResponseEntityBuilder.internalError(e.getMessage()); } } else { - return ResponseEntityBuilder.okWithCommonError("Log file not exist: " + log.getName()); + return ResponseEntityBuilder.okWithCommonError("Log file not exist: " + logFile); } } return ResponseEntityBuilder.ok(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/DppConfig.java b/fe/fe-core/src/main/java/org/apache/doris/load/DppConfig.java index e113ce539c..5b15ced9fb 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/DppConfig.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/DppConfig.java @@ -408,6 +408,49 @@ public class DppConfig implements Writable { @Override public boolean equals(Object obj) { + if (obj == null) { + return false; + } + + if (!(obj instanceof DppConfig)) { + return false; + } + + DppConfig other = (DppConfig) obj; + if (paloPath == null) { + if (other.paloPath != null) { + return false; + } + } else { + if (!paloPath.equals(other.paloPath)) { + return false; + } + } + + if (httpPort != other.httpPort) { + return false; + } + + if (hadoopConfigs == null) { + if (other.hadoopConfigs != null) { + return false; + } + } else { + if (!hadoopConfigs.equals(other.hadoopConfigs)) { + return false; + } + } + + if (priority == null) { + if (other.priority != null) { + return false; + } + } else { + if (!priority.equals(other.priority)) { + return false; + } + } + return true; } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/EtlJobInfo.java b/fe/fe-core/src/main/java/org/apache/doris/load/EtlJobInfo.java index c799d6b337..f0360cb547 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/EtlJobInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/EtlJobInfo.java @@ -54,6 +54,16 @@ public class EtlJobInfo implements Writable { @Override public boolean equals(Object obj) { - return true; + if (obj == null) { + return false; + } + if (this == obj) { + return true; + } + if (obj instanceof EtlJobInfo) { + EtlJobInfo other = (EtlJobInfo) obj; + return jobStatus.equals(other.jobStatus); + } + return false; } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/TableAddOrDropColumnsInfo.java b/fe/fe-core/src/main/java/org/apache/doris/persist/TableAddOrDropColumnsInfo.java index ad4be3ad9d..c6cdd2ce9e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/TableAddOrDropColumnsInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/TableAddOrDropColumnsInfo.java @@ -97,7 +97,7 @@ public class TableAddOrDropColumnsInfo implements Writable { TableAddOrDropColumnsInfo info = (TableAddOrDropColumnsInfo) obj; - return (dbId == info.dbId && tableId == tableId + return (dbId == info.dbId && tableId == info.tableId && indexSchemaMap.equals(info.indexSchemaMap) && indexes.equals(info.indexes) && jobId == info.jobId); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/TableRenameColumnInfo.java b/fe/fe-core/src/main/java/org/apache/doris/persist/TableRenameColumnInfo.java index f7a388756d..aec3a56a4f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/TableRenameColumnInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/TableRenameColumnInfo.java @@ -85,7 +85,7 @@ public class TableRenameColumnInfo implements Writable { TableRenameColumnInfo info = (TableRenameColumnInfo) obj; - return (dbId == info.dbId && tableId == tableId + return (dbId == info.dbId && tableId == info.tableId && colName.equals(info.colName) && newColName.equals(info.newColName)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java index a824a1d099..718cafd980 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java @@ -1712,7 +1712,7 @@ public class SingleNodePlanner { return; } preds.removeAll(pushDownFailedPredicates); - unassignedConjuncts.remove(preds); + unassignedConjuncts.removeAll(preds); unassignedConjuncts.addAll(pushDownFailedPredicates); // Remove unregistered predicates that reference the same slot on diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/CatalogTestUtil.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/CatalogTestUtil.java index 61f5f0d967..0c180e68aa 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/CatalogTestUtil.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/CatalogTestUtil.java @@ -163,7 +163,7 @@ public class CatalogTestUtil { if (slaveReplica.getBackendId() != masterReplica.getBackendId() || slaveReplica.getVersion() != masterReplica.getVersion() || slaveReplica.getLastFailedVersion() != masterReplica.getLastFailedVersion() - || slaveReplica.getLastSuccessVersion() != slaveReplica.getLastSuccessVersion()) { + || slaveReplica.getLastSuccessVersion() != masterReplica.getLastSuccessVersion()) { return false; } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/TableFunctionPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/TableFunctionPlanTest.java index 580bf4cc0a..a621994b34 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/TableFunctionPlanTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/TableFunctionPlanTest.java @@ -459,7 +459,7 @@ public class TableFunctionPlanTest { String sql = "desc verbose select min(c1) from (select k1 as c1, min(k2) as c2 from db1.tbl1 group by c1) a " + "lateral view explode_split(c2, \",\") tmp1 as e1 order by min(c1)"; String errorMsg = UtFrameUtils.getSQLPlanOrErrorMsg(ctx, sql, true); - errorMsg.equalsIgnoreCase("lateral view as a inline view"); + Assert.assertTrue(errorMsg.toLowerCase().contains("lateral view as a inline view")); } /*