You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "LJW21-02 (via GitHub)" <gi...@apache.org> on 2025/11/04 06:53:23 UTC
[PR] Support SSL-related parameters and interactive password input in export/import tools [iotdb]
LJW21-02 opened a new pull request, #16698:
URL: https://github.com/apache/iotdb/pull/16698
# Description
Codes are modified to:
- support new parameters (`-usessl` , `-ts` and `-tpw`) in data export/import tools
- support new parameters (`-usessl` , `-ts` and `-tpw`) in schema export/import tools
- support interactive password input in data export/import tools
- support interactive password input in schema export/import tools
# Preview
- New parameters added:
<img width="1297" height="306" alt="a" src="https://github.com/user-attachments/assets/f9d3fa26-94af-43ed-8ade-5d9a629229f2" />
- Support interactive input for `-pw`, `-ts` and `-tpw` parameters:
<img width="1500" height="116" alt="b" src="https://github.com/user-attachments/assets/8edb3b30-643d-4c3f-bef5-0c7989c983ba" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support SSL-related parameters and interactive password input in export/import tools [iotdb]
Posted by "LJW21-02 (via GitHub)" <gi...@apache.org>.
LJW21-02 commented on PR #16698:
URL: https://github.com/apache/iotdb/pull/16698#issuecomment-3488790955
Changed params processing order: ts > tpw > pw
<img width="1550" height="118" alt="c" src="https://github.com/user-attachments/assets/983d31e1-c2b0-44e6-adc4-b1a9f32e65ae" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support SSL-related parameters and interactive password input in export/import tools [iotdb]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16698:
URL: https://github.com/apache/iotdb/pull/16698#discussion_r2489295639
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/AbstractSchemaTool.java:
##########
@@ -93,9 +101,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password =
- checkRequiredArg(
- Constants.PW_ARGS, Constants.PW_NAME, commandLine, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
+ boolean hasPw = commandLine.hasOption(Constants.PW_ARGS);
+ if (hasPw) {
+ String inputPassword = commandLine.getOptionValue(Constants.PW_ARGS);
+ if (inputPassword != null) {
+ password = inputPassword;
+ } else {
+ password = cliCtx.getLineReader().readLine("please input your password:", '\0');
+ }
+ } else {
+ password = Constants.PW_DEFAULT_VALUE;
+ }
+ String useSslStr = commandLine.getOptionValue(Constants.USE_SSL_ARGS);
+ useSsl = Boolean.parseBoolean(useSslStr);
Review Comment:
Potential NullPointerException when `useSslStr` is null. `Boolean.parseBoolean(null)` returns false, which is correct, but if SSL is not specified, `useSsl` will be false. However, when checking `if (useSsl)` on line 120, it could throw NPE if `useSsl` is null from a previous invocation in static context. Consider using `Boolean.valueOf(useSslStr != null && Boolean.parseBoolean(useSslStr))` or initializing `useSsl = false` explicitly before parsing to ensure it's never null.
```suggestion
useSsl = Boolean.valueOf(useSslStr != null && Boolean.parseBoolean(useSslStr));
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/common/Constants.java:
##########
@@ -56,6 +56,18 @@ public class Constants {
public static final String USERNAME_DESC = "Username (optional)";
public static final String USERNAME_DEFAULT_VALUE = "root";
+ public static final String USE_SSL_ARGS = "usessl";
+ public static final String USE_SSL_NAME = "use_ssl";
+ public static final String USE_SSL_DESC = "Use SSL statement. (optional)";
Review Comment:
The description 'Use SSL statement' is unclear. Consider changing to 'Enable SSL connection' or 'Use SSL for secure connection' to better describe what this option does.
```suggestion
public static final String USE_SSL_DESC = "Enable SSL connection. (optional)";
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/AbstractSchemaTool.java:
##########
@@ -93,9 +101,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password =
- checkRequiredArg(
- Constants.PW_ARGS, Constants.PW_NAME, commandLine, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
+ boolean hasPw = commandLine.hasOption(Constants.PW_ARGS);
+ if (hasPw) {
+ String inputPassword = commandLine.getOptionValue(Constants.PW_ARGS);
+ if (inputPassword != null) {
+ password = inputPassword;
+ } else {
+ password = cliCtx.getLineReader().readLine("please input your password:", '\0');
+ }
+ } else {
+ password = Constants.PW_DEFAULT_VALUE;
+ }
+ String useSslStr = commandLine.getOptionValue(Constants.USE_SSL_ARGS);
+ useSsl = Boolean.parseBoolean(useSslStr);
+ if (useSsl) {
+ String givenTS = commandLine.getOptionValue(Constants.TRUST_STORE_ARGS);
+ if (givenTS != null) {
+ trustStore = givenTS;
+ } else {
+ trustStore = cliCtx.getLineReader().readLine("please input your trust_store:", '\0');
+ }
+ String givenTPW = commandLine.getOptionValue(Constants.TRUST_STORE_PWD_ARGS);
+ if (givenTPW != null) {
+ trustStorePwd = givenTPW;
+ } else {
+ trustStorePwd = cliCtx.getLineReader().readLine("please input your trust_store_pwd:", '\0');
Review Comment:
[nitpick] The prompt message should be capitalized and use more standard terminology. Change 'please input your trust_store_pwd:' to 'Please enter trust store password:'.
```suggestion
trustStorePwd = cliCtx.getLineReader().readLine("Please enter trust store password:", '\0');
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/AbstractDataTool.java:
##########
@@ -155,7 +163,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password = commandLine.getOptionValue(Constants.PW_ARGS, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
+ boolean hasPw = commandLine.hasOption(Constants.PW_ARGS);
+ if (hasPw) {
+ String inputPassword = commandLine.getOptionValue(Constants.PW_ARGS);
+ if (inputPassword != null) {
+ password = inputPassword;
+ } else {
+ password = cliCtx.getLineReader().readLine("please input your password:", '\0');
Review Comment:
[nitpick] The prompt message should be capitalized for consistency with standard CLI conventions. Change 'please input your password:' to 'Please enter your password:'.
```suggestion
password = cliCtx.getLineReader().readLine("Please enter your password:", '\0');
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/ImportSchemaTree.java:
##########
@@ -70,8 +70,12 @@ public void init()
.maxSize(threadNum + 1)
.enableIoTDBRpcCompression(false)
.enableRedirection(false)
- .enableAutoFetch(false)
- .build();
+ .enableAutoFetch(false);
+ if (useSsl) {
Review Comment:
Missing validation for `trustStore` and `trustStorePwd` before passing them to the SSL configuration. If the user doesn't provide these values when SSL is enabled, they could be null, potentially causing issues. Consider validating that these values are not null/empty when `useSsl` is true.
```suggestion
if (useSsl) {
if (StringUtils.isBlank(trustStore) || StringUtils.isBlank(trustStorePwd)) {
ioTPrinter.println("Error: SSL is enabled, but trustStore and/or trustStorePwd are missing or empty.");
System.exit(Constants.CODE_ERROR);
}
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/ExportSchemaTree.java:
##########
@@ -43,7 +43,17 @@ public class ExportSchemaTree extends AbstractExportSchema {
public void init()
throws InterruptedException, IoTDBConnectionException, StatementExecutionException {
- session = new Session(host, Integer.parseInt(port), username, password);
+ Session.Builder sessionBuilder =
+ new Session.Builder()
+ .host(host)
+ .port(Integer.parseInt(port))
+ .username(username)
+ .password(password);
+ if (useSsl) {
Review Comment:
Missing validation for `trustStore` and `trustStorePwd` before passing them to the SSL configuration. If the user doesn't provide these values when SSL is enabled, they could be null, potentially causing issues. Consider validating that these values are not null/empty when `useSsl` is true.
```suggestion
if (useSsl) {
if (trustStore == null || trustStore.isEmpty() || trustStorePwd == null || trustStorePwd.isEmpty()) {
throw new IllegalArgumentException("SSL is enabled, but trustStore and/or trustStorePwd are not provided.");
}
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/ExportDataTree.java:
##########
@@ -67,18 +67,23 @@ public class ExportDataTree extends AbstractExportData {
@Override
public void init() throws IoTDBConnectionException, StatementExecutionException, TException {
- session =
- new Session(
- host,
- Integer.parseInt(port),
- username,
- password,
- SessionConfig.DEFAULT_FETCH_SIZE,
- null,
- SessionConfig.DEFAULT_INITIAL_BUFFER_CAPACITY,
- rpcMaxFrameSize,
- SessionConfig.DEFAULT_REDIRECTION_MODE,
- SessionConfig.DEFAULT_VERSION);
+ Session.Builder sessionBuilder =
+ new Session.Builder()
+ .host(host)
+ .port(Integer.parseInt(port))
+ .username(username)
+ .password(password)
+ .fetchSize(SessionConfig.DEFAULT_FETCH_SIZE)
+ .zoneId(null)
+ .thriftDefaultBufferSize(SessionConfig.DEFAULT_INITIAL_BUFFER_CAPACITY)
+ .thriftMaxFrameSize(rpcMaxFrameSize)
+ .enableRedirection(SessionConfig.DEFAULT_REDIRECTION_MODE)
+ .version(SessionConfig.DEFAULT_VERSION);
+ if (useSsl) {
Review Comment:
Missing validation for `trustStore` and `trustStorePwd` before passing them to the SSL configuration. If the user doesn't provide these values when SSL is enabled, they could be null, potentially causing issues. Consider validating that these values are not null/empty when `useSsl` is true.
```suggestion
if (useSsl) {
if (trustStore == null || trustStore.isEmpty() || trustStorePwd == null || trustStorePwd.isEmpty()) {
throw new IllegalArgumentException("SSL is enabled, but trustStore and/or trustStorePwd are not provided.");
}
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/ExportDataTable.java:
##########
@@ -63,14 +63,18 @@ public class ExportDataTable extends AbstractExportData {
@Override
public void init() throws IoTDBConnectionException, StatementExecutionException {
- tableSession =
+ TableSessionBuilder tableSessionBuilder =
new TableSessionBuilder()
.nodeUrls(Collections.singletonList(host + ":" + port))
.username(username)
.password(password)
.database(database)
- .thriftMaxFrameSize(rpcMaxFrameSize)
- .build();
+ .thriftMaxFrameSize(rpcMaxFrameSize);
+ if (useSsl) {
Review Comment:
Missing validation for `trustStore` and `trustStorePwd` before passing them to the SSL configuration. If the user doesn't provide these values when SSL is enabled, they could be null, potentially causing issues. Consider validating that these values are not null/empty when `useSsl` is true.
```suggestion
if (useSsl) {
if (StringUtils.isEmpty(trustStore) || StringUtils.isEmpty(trustStorePwd)) {
ioTPrinter.println("Error: SSL is enabled, but trustStore and/or trustStorePwd are not provided.");
System.exit(1);
}
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/ExportDataTree.java:
##########
@@ -67,18 +67,23 @@ public class ExportDataTree extends AbstractExportData {
@Override
public void init() throws IoTDBConnectionException, StatementExecutionException, TException {
- session =
- new Session(
- host,
- Integer.parseInt(port),
- username,
- password,
- SessionConfig.DEFAULT_FETCH_SIZE,
- null,
- SessionConfig.DEFAULT_INITIAL_BUFFER_CAPACITY,
- rpcMaxFrameSize,
- SessionConfig.DEFAULT_REDIRECTION_MODE,
- SessionConfig.DEFAULT_VERSION);
+ Session.Builder sessionBuilder =
+ new Session.Builder()
+ .host(host)
+ .port(Integer.parseInt(port))
Review Comment:
Potential uncaught 'java.lang.NumberFormatException'.
```suggestion
int portInt;
try {
portInt = Integer.parseInt(port);
} catch (NumberFormatException e) {
ioTPrinter.print("Error: Invalid port number '" + port + "'. Please provide a valid integer value for port.");
throw new IoTDBConnectionException("Invalid port number: " + port, e);
}
Session.Builder sessionBuilder =
new Session.Builder()
.host(host)
.port(portInt)
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/AbstractDataTool.java:
##########
@@ -155,7 +163,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password = commandLine.getOptionValue(Constants.PW_ARGS, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
+ boolean hasPw = commandLine.hasOption(Constants.PW_ARGS);
+ if (hasPw) {
+ String inputPassword = commandLine.getOptionValue(Constants.PW_ARGS);
+ if (inputPassword != null) {
+ password = inputPassword;
+ } else {
+ password = cliCtx.getLineReader().readLine("please input your password:", '\0');
+ }
+ } else {
+ password = Constants.PW_DEFAULT_VALUE;
+ }
+ String useSslStr = commandLine.getOptionValue(Constants.USE_SSL_ARGS);
+ useSsl = Boolean.parseBoolean(useSslStr);
+ if (useSsl) {
+ String givenTS = commandLine.getOptionValue(Constants.TRUST_STORE_ARGS);
+ if (givenTS != null) {
+ trustStore = givenTS;
+ } else {
+ trustStore = cliCtx.getLineReader().readLine("please input your trust_store:", '\0');
Review Comment:
[nitpick] The prompt message should be capitalized and use more standard terminology. Change 'please input your trust_store:' to 'Please enter trust store path:'.
```suggestion
trustStore = cliCtx.getLineReader().readLine("Please enter trust store path:", '\0');
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/AbstractDataTool.java:
##########
@@ -155,7 +163,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password = commandLine.getOptionValue(Constants.PW_ARGS, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
+ boolean hasPw = commandLine.hasOption(Constants.PW_ARGS);
+ if (hasPw) {
+ String inputPassword = commandLine.getOptionValue(Constants.PW_ARGS);
+ if (inputPassword != null) {
+ password = inputPassword;
+ } else {
+ password = cliCtx.getLineReader().readLine("please input your password:", '\0');
+ }
+ } else {
+ password = Constants.PW_DEFAULT_VALUE;
+ }
+ String useSslStr = commandLine.getOptionValue(Constants.USE_SSL_ARGS);
+ useSsl = Boolean.parseBoolean(useSslStr);
Review Comment:
Potential NullPointerException when `useSslStr` is null. `Boolean.parseBoolean(null)` returns false, which is correct, but if SSL is not specified, `useSsl` will be false. However, when checking `if (useSsl)` on line 182, it could throw NPE if `useSsl` is null from a previous invocation in static context. Consider using `Boolean.valueOf(useSslStr != null && Boolean.parseBoolean(useSslStr))` or initializing `useSsl = false` explicitly before parsing to ensure it's never null.
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/AbstractSchemaTool.java:
##########
@@ -93,9 +101,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password =
- checkRequiredArg(
- Constants.PW_ARGS, Constants.PW_NAME, commandLine, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
+ boolean hasPw = commandLine.hasOption(Constants.PW_ARGS);
+ if (hasPw) {
+ String inputPassword = commandLine.getOptionValue(Constants.PW_ARGS);
+ if (inputPassword != null) {
+ password = inputPassword;
+ } else {
+ password = cliCtx.getLineReader().readLine("please input your password:", '\0');
Review Comment:
[nitpick] The prompt message should be capitalized for consistency with standard CLI conventions. Change 'please input your password:' to 'Please enter your password:'.
```suggestion
password = cliCtx.getLineReader().readLine("Please enter your password:", '\0');
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/AbstractSchemaTool.java:
##########
@@ -93,9 +101,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password =
- checkRequiredArg(
- Constants.PW_ARGS, Constants.PW_NAME, commandLine, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
Review Comment:
The CliContext and LineReader are created every time `parseBasicParams` is called, even when interactive input is not needed. Consider creating these only when necessary (i.e., when password or SSL parameters are missing) to avoid unnecessary overhead.
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/AbstractSchemaTool.java:
##########
@@ -93,9 +101,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password =
- checkRequiredArg(
- Constants.PW_ARGS, Constants.PW_NAME, commandLine, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
+ boolean hasPw = commandLine.hasOption(Constants.PW_ARGS);
+ if (hasPw) {
+ String inputPassword = commandLine.getOptionValue(Constants.PW_ARGS);
+ if (inputPassword != null) {
+ password = inputPassword;
+ } else {
+ password = cliCtx.getLineReader().readLine("please input your password:", '\0');
+ }
+ } else {
+ password = Constants.PW_DEFAULT_VALUE;
+ }
+ String useSslStr = commandLine.getOptionValue(Constants.USE_SSL_ARGS);
+ useSsl = Boolean.parseBoolean(useSslStr);
+ if (useSsl) {
+ String givenTS = commandLine.getOptionValue(Constants.TRUST_STORE_ARGS);
+ if (givenTS != null) {
+ trustStore = givenTS;
+ } else {
+ trustStore = cliCtx.getLineReader().readLine("please input your trust_store:", '\0');
Review Comment:
[nitpick] The prompt message should be capitalized and use more standard terminology. Change 'please input your trust_store:' to 'Please enter trust store path:'.
```suggestion
trustStore = cliCtx.getLineReader().readLine("Please enter trust store path:", '\0');
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/AbstractDataTool.java:
##########
@@ -155,7 +163,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password = commandLine.getOptionValue(Constants.PW_ARGS, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
+ boolean hasPw = commandLine.hasOption(Constants.PW_ARGS);
+ if (hasPw) {
+ String inputPassword = commandLine.getOptionValue(Constants.PW_ARGS);
+ if (inputPassword != null) {
+ password = inputPassword;
+ } else {
+ password = cliCtx.getLineReader().readLine("please input your password:", '\0');
+ }
+ } else {
+ password = Constants.PW_DEFAULT_VALUE;
+ }
+ String useSslStr = commandLine.getOptionValue(Constants.USE_SSL_ARGS);
+ useSsl = Boolean.parseBoolean(useSslStr);
+ if (useSsl) {
+ String givenTS = commandLine.getOptionValue(Constants.TRUST_STORE_ARGS);
+ if (givenTS != null) {
+ trustStore = givenTS;
+ } else {
+ trustStore = cliCtx.getLineReader().readLine("please input your trust_store:", '\0');
+ }
+ String givenTPW = commandLine.getOptionValue(Constants.TRUST_STORE_PWD_ARGS);
+ if (givenTPW != null) {
+ trustStorePwd = givenTPW;
+ } else {
+ trustStorePwd = cliCtx.getLineReader().readLine("please input your trust_store_pwd:", '\0');
Review Comment:
[nitpick] The prompt message should be capitalized and use more standard terminology. Change 'please input your trust_store_pwd:' to 'Please enter trust store password:'.
```suggestion
trustStorePwd = cliCtx.getLineReader().readLine("Please enter trust store password:", '\0');
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/AbstractDataTool.java:
##########
@@ -155,7 +163,36 @@ protected static void parseBasicParams(CommandLine commandLine) throws ArgsError
Constants.USERNAME_NAME,
commandLine,
Constants.USERNAME_DEFAULT_VALUE);
- password = commandLine.getOptionValue(Constants.PW_ARGS, Constants.PW_DEFAULT_VALUE);
+ CliContext cliCtx = new CliContext(System.in, System.out, System.err, ExitType.SYSTEM_EXIT);
+ LineReader lineReader = JlineUtils.getLineReader(cliCtx, username, host, port);
+ cliCtx.setLineReader(lineReader);
Review Comment:
The CliContext and LineReader are created every time `parseBasicParams` is called, even when interactive input is not needed. Consider creating these only when necessary (i.e., when password or SSL parameters are missing) to avoid unnecessary overhead.
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/ImportSchemaTable.java:
##########
@@ -57,8 +57,12 @@ public void init() throws InterruptedException {
.enableThriftCompression(false)
.enableRedirection(false)
.enableAutoFetch(false)
- .database(database)
- .build();
+ .database(database);
+ if (useSsl) {
Review Comment:
Missing validation for `trustStore` and `trustStorePwd` before passing them to the SSL configuration. If the user doesn't provide these values when SSL is enabled, they could be null, potentially causing issues. Consider validating that these values are not null/empty when `useSsl` is true.
```suggestion
if (useSsl) {
if (StringUtils.isEmpty(trustStore) || StringUtils.isEmpty(trustStorePwd)) {
ioTPrinter.println("SSL is enabled, but trustStore and/or trustStorePwd is missing or empty.");
System.exit(Constants.CODE_ERROR);
}
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/ExportSchemaTable.java:
##########
@@ -61,8 +61,12 @@ public void init() throws InterruptedException {
.enableThriftCompression(false)
.enableRedirection(false)
.enableAutoFetch(false)
- .database(database)
- .build();
+ .database(database);
+ if (useSsl) {
Review Comment:
Missing validation for `trustStore` and `trustStorePwd` before passing them to the SSL configuration. If the user doesn't provide these values when SSL is enabled, they could be null, potentially causing issues. Consider validating that these values are not null/empty when `useSsl` is true.
```suggestion
if (useSsl) {
if (StringUtils.isEmpty(trustStore) || StringUtils.isEmpty(trustStorePwd)) {
ioTPrinter.println("Error: SSL is enabled, but trustStore and/or trustStorePwd are not provided.");
System.exit(1);
}
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/ImportDataTable.java:
##########
@@ -75,8 +75,12 @@ public void init() throws InterruptedException {
.enableThriftCompression(false)
.enableRedirection(false)
.enableAutoFetch(false)
- .database(database)
- .build();
+ .database(database);
+ if (useSsl) {
Review Comment:
Missing validation for `trustStore` and `trustStorePwd` before passing them to the SSL configuration. If the user doesn't provide these values when SSL is enabled, they could be null, potentially causing issues. Consider validating that these values are not null/empty when `useSsl` is true.
```suggestion
if (useSsl) {
if (StringUtils.isBlank(trustStore) || StringUtils.isBlank(trustStorePwd)) {
ioTPrinter.println("SSL is enabled, but trustStore and/or trustStorePwd are not provided.");
System.exit(Constants.CODE_ERROR);
}
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/schema/ExportSchemaTree.java:
##########
@@ -43,7 +43,17 @@ public class ExportSchemaTree extends AbstractExportSchema {
public void init()
throws InterruptedException, IoTDBConnectionException, StatementExecutionException {
- session = new Session(host, Integer.parseInt(port), username, password);
+ Session.Builder sessionBuilder =
+ new Session.Builder()
+ .host(host)
+ .port(Integer.parseInt(port))
Review Comment:
Potential uncaught 'java.lang.NumberFormatException'.
```suggestion
Session.Builder sessionBuilder;
int portInt;
try {
portInt = Integer.parseInt(port);
} catch (NumberFormatException e) {
ioTPrinter.println("Invalid port number: " + port);
throw new StatementExecutionException("Invalid port number: " + port, e);
}
sessionBuilder =
new Session.Builder()
.host(host)
.port(portInt)
```
##########
iotdb-client/cli/src/main/java/org/apache/iotdb/tool/data/ImportDataTree.java:
##########
@@ -70,8 +70,12 @@ public void init()
.maxSize(threadNum + 1)
.enableIoTDBRpcCompression(false)
.enableRedirection(false)
- .enableAutoFetch(false)
- .build();
+ .enableAutoFetch(false);
+ if (useSsl) {
Review Comment:
Missing validation for `trustStore` and `trustStorePwd` before passing them to the SSL configuration. If the user doesn't provide these values when SSL is enabled, they could be null, potentially causing issues. Consider validating that these values are not null/empty when `useSsl` is true.
```suggestion
if (useSsl) {
if (trustStore == null || trustStore.isEmpty() || trustStorePwd == null || trustStorePwd.isEmpty()) {
ioTPrinter.println("Error: SSL is enabled, but trustStore and/or trustStorePwd are not provided.");
System.exit(Constants.CODE_ERROR);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support SSL-related parameters and interactive password input in export/import tools [iotdb]
Posted by "CritasWang (via GitHub)" <gi...@apache.org>.
CritasWang merged PR #16698:
URL: https://github.com/apache/iotdb/pull/16698
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org