[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2604: Tune zookeeper request handler permissions

[GitBox <gi...@apache.org>]

dsmiley commented on a change in pull request #2604:
URL: https://github.com/apache/lucene-solr/pull/2604#discussion_r743775899



##########
File path: solr/core/src/java/org/apache/solr/handler/admin/ZookeeperInfoHandler.java
##########
@@ -110,6 +112,18 @@ public Category getCategory() {
     none, name, status
   }
 
+  @Override
+  public PermissionNameProvider.Name getPermissionName(AuthorizationContext request) {
+    SolrParams params = request.getParams();
+    String path = params.get("path", "");
+    String detail = params.get("detail", "false");

Review comment:
       Can we add a field constant for this param (and perhaps others) to more clearly show we depend on this name in the code from two places?  Generally I find string constants to be an over-used coding habit yet ironically I propose it here because I think it would decrease a security risk if the code evolves.  I don't feel strongly; feel free to ignore.  Maybe this could be ameliorated with a simple comment in handleRequestBody saying simply `//note: these params are used in getPermissionName at runtime`

##########
File path: solr/core/src/java/org/apache/solr/handler/admin/ZookeeperReadAPI.java
##########
@@ -130,6 +132,49 @@ public void listNodes(SolrQueryRequest req, SolrQueryResponse rsp) {
     }
   }
 
+  /**
+   * Simple mime type guessing based on first character of the response
+   */
+  private String guessMime(byte firstByte) {
+    switch(firstByte) {
+      case '{':
+        return CommonParams.JSON_MIME;
+      case '<':
+      case '?':
+        return XMLResponseParser.XML_CONTENT_TYPE;
+      default:
+        return BinaryResponseParser.BINARY_CONTENT_TYPE;
+    }
+  }
+
+  /**
+   * Reads a single node from zookeeper and return as byte array
+   */
+  private byte[] readPathFromZookeeper(String path) {
+    byte[] d;
+    try {
+      d = coreContainer.getZkController().getZkClient().getData(path, null, null, false);
+    } catch (KeeperException.NoNodeException e) {
+      throw new SolrException(SolrException.ErrorCode.NOT_FOUND, "No such node: " + path);
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unexpected error", e);
+    }
+    return d;
+  }
+
+  /**
+   * Reads content of a znode and adds it to the response
+   */
+  private void readNodeAndAddToResponse(String zkPath, SolrQueryRequest req, SolrQueryResponse rsp) {

Review comment:
       Please order higher than the methods it calls out to.  It's nicer to read code top-to-bottom.




-- 
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-unsubscribe@lucene.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org