janhoy commented on code in PR #2787:
URL: https://github.com/apache/solr/pull/2787#discussion_r1811227283


##########
solr/core/src/java/org/apache/solr/cli/PostTool.java:
##########
@@ -330,15 +331,24 @@ public void runImpl(CommandLine cli) throws Exception {
       }
 
       solrUrl = SolrCLI.normalizeSolrUrl(solrUrl, true, hostContext) + 
hostContext;
-      String url = solrUrl + "/" + cli.getOptionValue("name") + "/update";
-      solrUpdateUrl = new URI(url);
+
+      solrUpdateUrl =
+          UriBuilder.fromUri(SolrCLI.normalizeSolrUrl(solrUrl, true, 
hostContext))
+              .path(hostContext)
+              .path(cli.getOptionValue("name"))
+              .path("update")
+              .build();
 
     } else if (cli.hasOption("solr-update-url")) {
       String url = cli.getOptionValue("solr-update-url");
       solrUpdateUrl = new URI(url);
     } else if (cli.hasOption("name")) {
-      String url = SolrCLI.getDefaultSolrUrl() + "/solr/" + 
cli.getOptionValue("name") + "/update";
-      solrUpdateUrl = new URI(url);
+      solrUpdateUrl =
+          UriBuilder.fromUri(SolrCLI.getDefaultSolrUrl())
+              .path("solr")

Review Comment:
   Here, when `--solr-url` or `--solr-update-url` is not given, meaning it will 
be localhost, we always use `/solr`. But what if the user is at an installation 
with customized `contextPath` (I doubt that happens very often)?



##########
solr/core/src/java/org/apache/solr/cli/PostTool.java:
##########
@@ -330,15 +331,24 @@ public void runImpl(CommandLine cli) throws Exception {
       }
 
       solrUrl = SolrCLI.normalizeSolrUrl(solrUrl, true, hostContext) + 
hostContext;
-      String url = solrUrl + "/" + cli.getOptionValue("name") + "/update";
-      solrUpdateUrl = new URI(url);
+
+      solrUpdateUrl =
+          UriBuilder.fromUri(SolrCLI.normalizeSolrUrl(solrUrl, true, 
hostContext))
+              .path(hostContext)

Review Comment:
   What if the `--solr-url` is a remote URL? Should we not then require the 
baseUrl to include the correct `hostContext`? Or have we documented a way for 
the user to pass in the `-DhostContext=/foo` to `bin/solr post`? I have not 
seen such doc, meaning hostContext will always be `/solr`, unless `bin/solr` is 
executed from within an installation that has overridden this in 
`bin/solr.in.sh`?



-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to