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

penghui pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit c4e491b827c60ada575862eb2a64988ac558020a
Author: Ruguo Yu <[email protected]>
AuthorDate: Mon Nov 8 19:12:37 2021 +0800

    [pulsar-admin] Perfect judgment conditions of pulsar-admin (#12315)
    
    ### Motivation
    Perfect judgment conditions of `bin/pulsar-admin`, included parameter 
`CONF_FILE_PATH` and field `serviceUrl`.
    
    **Details are as follows:**
    **First of all**, the configuration file `CONF_FILE_PATH` is a required 
option rather than an optional option (from the [source 
code](https://github.com/apache/pulsar/blob/master/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java#L288-L299),
 the configuration file is regarded as an optional option) , because some very 
important [default 
values](https://github.com/apache/pulsar/blob/master/conf/client.conf#L22-L30) 
(such as `webServiceUrl=http://localhos [...]
    ```
    ./bin/pulsar-admin topics list-partitioned-topics test/app1
    Exception in thread "main" java.io.FileNotFoundException: topics (No such 
file or directory)
        at java.io.FileInputStream.open0(Native Method)
        at java.io.FileInputStream.open(FileInputStream.java:195)
        at java.io.FileInputStream.<init>(FileInputStream.java:138)
        at java.io.FileInputStream.<init>(FileInputStream.java:93)
        at 
org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:296)
    ```
    so we first need to judge whether the configuration file `CONF_FILE_PATH` 
exists, and give the correct command line format (`Usage: pulsar-admin 
CONF_FILE_PATH [options] [command] [command options]`) if it is illegal.
    
    **Secondly**, `serviceUrl` is a very important field, because pulsarAdmin 
can connect to the pulsar server through this value. If the configuration file 
does not have releated `key`=`value`(such as `webServiceUrl`=, 
`brokerServiceUrl`=) and the user does not specify option `--admin-url`, then 
NPE will appear, as follows:
    ```
    ./bin/pulsar-admin topics list-partitioned-topics test/app1
    java.lang.NullPointerException
        at 
org.apache.pulsar.client.admin.internal.PulsarAdminImpl.<init>(PulsarAdminImpl.java:189)
        at 
org.apache.pulsar.client.admin.internal.PulsarAdminBuilderImpl.build(PulsarAdminBuilderImpl.java:47)
        at 
org.apache.pulsar.admin.cli.PulsarAdminTool.lambda$main$2(PulsarAdminTool.java:320)
        at 
org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:174)
        at 
org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:161)
        at org.apache.pulsar.admin.cli.CmdBase.getAdmin(CmdBase.java:111)
        at org.apache.pulsar.admin.cli.CmdTopics.getTopics(CmdTopics.java:2360)
        at org.apache.pulsar.admin.cli.CmdTopics.access$11000(CmdTopics.java:73)
        at 
org.apache.pulsar.admin.cli.CmdTopics$PartitionedTopicListCmd.run(CmdTopics.java:275)
        at org.apache.pulsar.admin.cli.CmdBase.run(CmdBase.java:86)
        at 
org.apache.pulsar.admin.cli.PulsarAdminTool.run(PulsarAdminTool.java:282)
        at 
org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:330)
    class java.lang.NullPointerException: null
    ```
    so we need to judge whether field `serviceUrl` exists.
    
    (cherry picked from commit f976500029514e5275dc97e1e51748e8ed4fc0b2)
---
 .../org/apache/pulsar/admin/cli/PulsarAdminTool.java    | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git 
a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java
 
b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java
index 5f0cd58..a41f444 100644
--- 
a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java
+++ 
b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java
@@ -37,6 +37,8 @@ import org.apache.pulsar.client.admin.PulsarAdmin;
 import org.apache.pulsar.client.admin.PulsarAdminBuilder;
 import org.apache.pulsar.client.admin.internal.PulsarAdminImpl;
 
+import static org.apache.commons.lang3.StringUtils.isBlank;
+
 public class PulsarAdminTool {
 
     private static boolean allowSystemExit = true;
@@ -249,6 +251,11 @@ public class PulsarAdminTool {
             return false;
         }
 
+        if (isBlank(serviceUrl)) {
+            jcommander.usage();
+            return false;
+        }
+
         if (version) {
             System.out.println("Current version of pulsar admin client is: " + 
PulsarVersion.getVersion());
             return true;
@@ -285,11 +292,12 @@ public class PulsarAdminTool {
 
     public static void main(String[] args) throws Exception {
         lastExitCode = 0;
-        String configFile = null;
-        if (args.length > 0) {
-            configFile = args[0];
-            args = Arrays.copyOfRange(args, 1, args.length);
+        if (args.length == 0) {
+            System.out.println("Usage: pulsar-admin CONF_FILE_PATH [options] 
[command] [command options]");
+            exit(0);
+            return;
         }
+        String configFile = args[0];
         Properties properties = new Properties();
 
         if (configFile != null) {
@@ -301,6 +309,7 @@ public class PulsarAdminTool {
         PulsarAdminTool tool = new PulsarAdminTool(properties);
 
         int cmdPos;
+        args = Arrays.copyOfRange(args, 1, args.length);
         for (cmdPos = 0; cmdPos < args.length; cmdPos++) {
             if (tool.commandMap.containsKey(args[cmdPos])) {
                 break;

Reply via email to