Skip to content

Commit 366726f

Browse files
CaideyipiCaideyipi
authored
Fixed the path check semantic for object (apache#16983)
* create-check * init-windows * windows * windows * partial * IT-change * create-table * po * fix * bypass-sit * finally
1 parent 3ca20a3 commit 366726f

File tree

6 files changed

+177
-45
lines changed

6 files changed

+177
-45
lines changed

‎integration-test/src/test/java/org/apache/iotdb/relational/it/schema/IoTDBTableIT.java‎

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.apache.iotdb.relational.it.schema;
2121

22+
import org.apache.iotdb.commons.utils.WindowsOSUtils;
2223
import org.apache.iotdb.db.it.utils.TestUtils;
2324
import org.apache.iotdb.isession.ITableSession;
2425
import org.apache.iotdb.it.env.EnvFactory;
@@ -31,6 +32,7 @@
3132

3233
import org.apache.tsfile.enums.ColumnCategory;
3334
import org.apache.tsfile.enums.TSDataType;
35+
import org.apache.tsfile.external.commons.lang3.SystemUtils;
3436
import org.apache.tsfile.write.record.Tablet;
3537
import org.apache.tsfile.write.schema.IMeasurementSchema;
3638
import org.apache.tsfile.write.schema.MeasurementSchema;
@@ -784,6 +786,11 @@ public void testConcurrentAutoCreateAndDropColumn() throws Exception {
784786
@Test
785787
public void testTableObjectCheck() throws Exception {
786788
final Set<String> illegal = new HashSet<>(Arrays.asList("./", ".", "..", ".\\", "../hack"));
789+
if (SystemUtils.IS_OS_WINDOWS) {
790+
illegal.add("C.");
791+
illegal.add("a:b<|");
792+
illegal.add("COM1");
793+
}
787794
for (final String single : illegal) {
788795
testObject4SingleIllegalPath(single);
789796
}
@@ -796,21 +803,9 @@ private void testObject4SingleIllegalPath(final String illegal) throws Exception
796803
final ITableSession session = EnvFactory.getEnv().getTableSessionConnection()) {
797804
statement.execute("create database if not exists db2");
798805
statement.execute("use db2");
799-
statement.execute(String.format("create table \"%s\" ()", illegal));
800-
801-
try {
802-
statement.execute(String.format("alter table \"%s\" add column a object", illegal));
803-
fail();
804-
} catch (final SQLException e) {
805-
Assert.assertEquals(
806-
String.format(
807-
"701: When there are object fields, the tableName %s shall not be '.', '..' or contain './', '.\\'",
808-
illegal),
809-
e.getMessage());
810-
}
811806

812-
// Test auto-create
813-
String testObject =
807+
// Test auto-create table
808+
final String testObject =
814809
System.getProperty("user.dir")
815810
+ File.separator
816811
+ "target"
@@ -819,7 +814,7 @@ private void testObject4SingleIllegalPath(final String illegal) throws Exception
819814
+ File.separator
820815
+ "object-example.pt";
821816

822-
List<IMeasurementSchema> schemaList = new ArrayList<>();
817+
final List<IMeasurementSchema> schemaList = new ArrayList<>();
823818
schemaList.add(new MeasurementSchema("a", TSDataType.STRING));
824819
schemaList.add(new MeasurementSchema("b", TSDataType.STRING));
825820
schemaList.add(new MeasurementSchema("c", TSDataType.INT32));
@@ -843,26 +838,46 @@ private void testObject4SingleIllegalPath(final String illegal) throws Exception
843838
tablet.addValue(schemaList.get(2).getMeasurementName(), 0, 0);
844839
tablet.addValue(0, 3, true, 0, Files.readAllBytes(Paths.get(testObject)));
845840

841+
final String expectedTableError =
842+
String.format(
843+
"701: When there are object fields, the tableName %s shall not be '.', '..' or contain './', '.\\'."
844+
+ (SystemUtils.IS_OS_WINDOWS ? " " + WindowsOSUtils.OS_SEGMENT_ERROR : ""),
845+
illegal.toLowerCase());
846+
final String expectedObjectError =
847+
String.format(
848+
"701: When there are object fields, the objectName %s shall not be '.', '..' or contain './', '.\\'."
849+
+ (SystemUtils.IS_OS_WINDOWS ? " " + WindowsOSUtils.OS_SEGMENT_ERROR : ""),
850+
illegal.toLowerCase());
851+
846852
try {
847853
session.executeNonQueryStatement("use db2");
848854
session.insert(tablet);
849855
} catch (final Exception e) {
850-
Assert.assertEquals(
851-
String.format(
852-
"701: When there are object fields, the tableName %s shall not be '.', '..' or contain './', '.\\'",
853-
illegal),
854-
e.getMessage());
856+
Assert.assertEquals(expectedTableError, e.getMessage());
857+
}
858+
859+
statement.execute(String.format("create table \"%s\" ()", illegal));
860+
861+
try {
862+
statement.execute(String.format("alter table \"%s\" add column a object", illegal));
863+
fail();
864+
} catch (final SQLException e) {
865+
Assert.assertEquals(expectedTableError, e.getMessage());
866+
}
867+
868+
// Test auto-create column
869+
try {
870+
session.executeNonQueryStatement("use db2");
871+
session.insert(tablet);
872+
} catch (final Exception e) {
873+
Assert.assertEquals(expectedTableError, e.getMessage());
855874
}
856875

857876
try {
858877
statement.execute(String.format("create table test (\"%s\" object)", illegal));
859878
fail();
860879
} catch (final SQLException e) {
861-
Assert.assertEquals(
862-
String.format(
863-
"701: When there are object fields, the objectName %s shall not be '.', '..' or contain './', '.\\'",
864-
illegal),
865-
e.getMessage());
880+
Assert.assertEquals(expectedObjectError, e.getMessage());
866881
}
867882

868883
statement.execute("create table test (a tag, b attribute, c int32, d object)");
@@ -873,11 +888,7 @@ private void testObject4SingleIllegalPath(final String illegal) throws Exception
873888
session.executeNonQueryStatement("use db2");
874889
session.insert(tablet);
875890
} catch (final Exception e) {
876-
Assert.assertEquals(
877-
String.format(
878-
"701: When there are object fields, the objectName %s shall not be '.', '..' or contain './', '.\\'",
879-
illegal),
880-
e.getMessage());
891+
Assert.assertEquals(expectedObjectError, e.getMessage());
881892
}
882893

883894
// It's OK if you don't write object
@@ -891,7 +902,8 @@ private void testObject4SingleIllegalPath(final String illegal) throws Exception
891902
} catch (final SQLException e) {
892903
Assert.assertEquals(
893904
String.format(
894-
"507: When there are object fields, the deviceId [test, %s] shall not be '.', '..' or contain './', '.\\'",
905+
"507: When there are object fields, the deviceId [test, %s] shall not be '.', '..' or contain './', '.\\'."
906+
+ (SystemUtils.IS_OS_WINDOWS ? " " + WindowsOSUtils.OS_SEGMENT_ERROR : ""),
895907
illegal),
896908
e.getMessage());
897909
}
@@ -900,11 +912,7 @@ private void testObject4SingleIllegalPath(final String illegal) throws Exception
900912
statement.execute(String.format("alter table test add column \"%s\" object", illegal));
901913
fail();
902914
} catch (final SQLException e) {
903-
Assert.assertEquals(
904-
String.format(
905-
"701: When there are object fields, the objectName %s shall not be '.', '..' or contain './', '.\\'",
906-
illegal),
907-
e.getMessage());
915+
Assert.assertEquals(expectedObjectError, e.getMessage());
908916
}
909917

910918
statement.execute("drop database db2");

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TableConfigTaskVisitor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ private Pair<String, TsTable> parseTable4CreateTableOrView(
580580
final TsTableColumnCategory category = columnDefinition.getColumnCategory();
581581
final String columnName = columnDefinition.getName().getValue();
582582
final TSDataType dataType = getDataType(columnDefinition.getType());
583-
hasObject |= dataType.equals(TSDataType.OBJECT);
583+
hasObject |= dataType == TSDataType.OBJECT;
584584
final String comment = columnDefinition.getComment();
585585
if (checkTimeColumnIdempotent(category, columnName, dataType, comment, table)
586586
&& !hasTimeColumn) {

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/metadata/fetcher/TableHeaderSchemaValidator.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
package org.apache.iotdb.db.queryengine.plan.relational.metadata.fetcher;
2121

2222
import org.apache.iotdb.commons.exception.IoTDBException;
23+
import org.apache.iotdb.commons.exception.IoTDBRuntimeException;
24+
import org.apache.iotdb.commons.exception.MetadataException;
2325
import org.apache.iotdb.commons.schema.table.InsertNodeMeasurementInfo;
2426
import org.apache.iotdb.commons.schema.table.TreeViewSchema;
2527
import org.apache.iotdb.commons.schema.table.TsTable;
@@ -629,6 +631,7 @@ public TsTable toTsTable(InsertNodeMeasurementInfo measurementInfo) {
629631
return tsTable;
630632
}
631633

634+
boolean hasObject = false;
632635
for (int i = 0; i < measurements.length; i++) {
633636
if (measurements[i] == null) {
634637
continue;
@@ -657,9 +660,17 @@ public TsTable toTsTable(InsertNodeMeasurementInfo measurementInfo) {
657660
throw new ColumnCreationFailException(
658661
"Cannot create column " + columnName + " datatype is not provided");
659662
}
660-
663+
hasObject |= dataType == TSDataType.OBJECT;
661664
tsTable.addColumnSchema(generateColumnSchema(category, columnName, dataType, null, null));
662665
}
666+
if (hasObject) {
667+
try {
668+
tsTable.checkTableNameAndObjectNames4Object();
669+
} catch (final MetadataException e) {
670+
throw new IoTDBRuntimeException(
671+
e.getMessage(), TSStatusCode.SEMANTIC_ERROR.getStatusCode());
672+
}
673+
}
663674
return tsTable;
664675
}
665676

‎iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/TsTable.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@
3030
import org.apache.iotdb.commons.schema.table.column.TsTableColumnSchema;
3131
import org.apache.iotdb.commons.schema.table.column.TsTableColumnSchemaUtil;
3232
import org.apache.iotdb.commons.utils.CommonDateTimeUtils;
33+
import org.apache.iotdb.commons.utils.WindowsOSUtils;
3334
import org.apache.iotdb.rpc.TSStatusCode;
3435

3536
import com.google.common.collect.ImmutableList;
3637
import org.apache.tsfile.enums.TSDataType;
38+
import org.apache.tsfile.external.commons.lang3.SystemUtils;
3739
import org.apache.tsfile.utils.Pair;
3840
import org.apache.tsfile.utils.ReadWriteIOUtils;
3941

@@ -71,7 +73,7 @@ public class TsTable {
7173
public static final String TTL_PROPERTY = "ttl";
7274
public static final Set<String> TABLE_ALLOWED_PROPERTIES = Collections.singleton(TTL_PROPERTY);
7375
private static final String OBJECT_STRING_ERROR =
74-
"When there are object fields, the %s %s shall not be '.', '..' or contain './', '.\\'";
76+
"When there are object fields, the %s %s shall not be '.', '..' or contain './', '.\\'.";
7577
protected String tableName;
7678

7779
private final Map<String, TsTableColumnSchema> columnSchemaMap = new LinkedHashMap<>();
@@ -435,15 +437,21 @@ && isInvalid4ObjectType(schema.getColumnName())) {
435437
}
436438
}
437439

438-
public static boolean isInvalid4ObjectType(final String column) {
439-
return column.equals(".")
440-
|| column.equals("..")
441-
|| column.contains("./")
442-
|| column.contains(".\\");
440+
public static boolean isInvalid4ObjectType(final String path) {
441+
return path.equals(".")
442+
|| path.equals("..")
443+
|| path.contains("./")
444+
|| path.contains(".\\")
445+
|| !WindowsOSUtils.isLegalPathSegment4Windows(path);
443446
}
444447

445448
public static String getObjectStringError(final String columnType, final String columnName) {
446-
return String.format(OBJECT_STRING_ERROR, columnType, columnName);
449+
return String.format(
450+
SystemUtils.IS_OS_WINDOWS
451+
? OBJECT_STRING_ERROR + " " + WindowsOSUtils.OS_SEGMENT_ERROR
452+
: OBJECT_STRING_ERROR,
453+
columnType,
454+
columnName);
447455
}
448456

449457
@Override

‎iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/WindowsOSUtils.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.iotdb.commons.utils;
21+
22+
import org.apache.tsfile.external.commons.lang3.SystemUtils;
23+
24+
import java.util.Arrays;
25+
import java.util.HashSet;
26+
import java.util.Set;
27+
28+
public class WindowsOSUtils {
29+
private static final String ILLEGAL_WINDOWS_CHARS = "\\/:*?\"<>|";
30+
private static final Set<String> ILLEGAL_WINDOWS_NAMES =
31+
new HashSet<>(Arrays.asList("CON", "PRN", "AUX", "NUL", "COM1-COM9, LPT1-LPT9"));
32+
33+
static {
34+
for (int i = 0; i < 10; ++i) {
35+
ILLEGAL_WINDOWS_NAMES.add("COM" + i);
36+
ILLEGAL_WINDOWS_NAMES.add("LPT" + i);
37+
}
38+
}
39+
40+
public static final String OS_SEGMENT_ERROR =
41+
String.format(
42+
"In Windows System, the path shall not contains %s, equals one of %s, or ends with '.' or ' '.",
43+
ILLEGAL_WINDOWS_CHARS, ILLEGAL_WINDOWS_NAMES);
44+
45+
public static boolean isLegalPathSegment4Windows(final String pathSegment) {
46+
if (!SystemUtils.IS_OS_WINDOWS) {
47+
return true;
48+
}
49+
for (final char illegalChar : ILLEGAL_WINDOWS_CHARS.toCharArray()) {
50+
if (pathSegment.indexOf(illegalChar) != -1) {
51+
return false;
52+
}
53+
}
54+
if (pathSegment.endsWith(".") || pathSegment.endsWith(" ")) {
55+
return false;
56+
}
57+
for (final String illegalName : ILLEGAL_WINDOWS_NAMES) {
58+
if (pathSegment.equalsIgnoreCase(illegalName)) {
59+
return false;
60+
}
61+
}
62+
return true;
63+
}
64+
}

‎iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/utils/WindowsOSUtilsTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.iotdb.commons.utils;
21+
22+
import org.apache.tsfile.external.commons.lang3.SystemUtils;
23+
import org.junit.Assert;
24+
import org.junit.Test;
25+
26+
import static org.apache.iotdb.commons.utils.WindowsOSUtils.isLegalPathSegment4Windows;
27+
28+
public class WindowsOSUtilsTest {
29+
@Test
30+
public void testIllegalDetection() {
31+
if (!SystemUtils.IS_OS_WINDOWS) {
32+
return;
33+
}
34+
Assert.assertTrue(isLegalPathSegment4Windows("abc"));
35+
Assert.assertTrue(isLegalPathSegment4Windows(".A!"));
36+
37+
Assert.assertFalse(isLegalPathSegment4Windows("C."));
38+
Assert.assertFalse(isLegalPathSegment4Windows("a:b<|"));
39+
Assert.assertFalse(isLegalPathSegment4Windows("COM1"));
40+
}
41+
}

0 commit comments

Comments
 (0)