You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2022/12/13 07:50:19 UTC
[GitHub] [zookeeper] sonatype-lift[bot] commented on a diff in pull request #1961: ZOOKEEPER-4571: Admin server API for restore database from a snapshot
sonatype-lift[bot] commented on code in PR #1961:
URL: https://github.com/apache/zookeeper/pull/1961#discussion_r1046760931
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/JettyAdminServer.java:
##########
@@ -259,15 +261,81 @@ protected void doGet(
}
// Run the command
- CommandResponse cmdResponse = Commands.runCommand(cmd, zkServer, kwargs);
+ final CommandResponse cmdResponse = Commands.runCommand(cmd, zkServer, kwargs, null);
+ response.setStatus(cmdResponse.getStatusCode());
- // Format and print the output of the command
- CommandOutputter outputter = new JsonOutputter();
- response.setStatus(HttpServletResponse.SC_OK);
+ final Map<String, String> headers = cmdResponse.getHeaders();
+ for (final Map.Entry<String, String> header : headers.entrySet()) {
+ response.addHeader(header.getKey(), header.getValue());
+ }
+ final String clientIP = IPAuthenticationProvider.getClientIPAddress(request);
+ if (cmdResponse.getInputStream() == null) {
+ // Format and print the output of the command
+ CommandOutputter outputter = new JsonOutputter(clientIP);
+ response.setContentType(outputter.getContentType());
+ outputter.output(cmdResponse, response.getWriter());
+ } else {
+ // Stream out the output of the command
+ CommandOutputter outputter = new StreamOutputter(clientIP);
+ response.setContentType(outputter.getContentType());
+ outputter.output(cmdResponse, response.getOutputStream());
+ }
+ }
+
+ /**
+ * Serves HTTP POST requests. It reads request payload as raw data.
+ * It's up to each command to process the payload accordingly.
+ * For example, RestoreCommand uses the payload InputStream directly
+ * to read snapshot data.
+ */
+ @Override
+ protected void doPost(final HttpServletRequest request,
+ final HttpServletResponse response) throws ServletException, IOException {
+ final String cmdName = extractCommandNameFromURL(request, response);
+ if (cmdName != null) {
+ // Run the command
+ final CommandResponse cmdResponse = Commands.runCommand(cmdName, zkServer, null, request.getInputStream());
+ final String clientIP = IPAuthenticationProvider.getClientIPAddress(request);
+ sendJSONResponse(response, cmdResponse, clientIP);
+ }
+ }
+
+ /**
+ * Extracts the command name from URL if it exists otherwise null
+ */
+ private String extractCommandNameFromURL(final HttpServletRequest request,
+ final HttpServletResponse response) throws IOException {
+ String cmd = request.getPathInfo();
+ if (cmd == null || cmd.equals("/")) {
+ printCommandLinks(response);
+ return null;
+ }
+ // Strip leading "/"
+ return cmd.substring(1);
+ }
+
+ /**
+ * Prints the list of URLs to each registered command as response.
+ */
+ private void printCommandLinks(final HttpServletResponse response) throws IOException {
+ for (final String link : commandLinks()) {
+ response.getWriter().println(link);
Review Comment:
*[XSS_SERVLET](https://find-sec-bugs.github.io/bugs.htm#XSS_SERVLET):* This use of java/io/PrintWriter.println(Ljava/lang/String;)V could be vulnerable to XSS in the Servlet
---
<details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary>
You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
| **Command** | **Usage** |
| ------------- | ------------- |
| `@sonatype-lift ignore` | Leave out the above finding from this PR |
| `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
| `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
**Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
<sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
---
Was this a good recommendation?
[ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=360445073&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=360445073&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=360445073&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=360445073&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=360445073&lift_comment_rating=5) ]
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZKDatabase.java:
##########
@@ -620,6 +623,48 @@ public void deserializeSnapshot(InputArchive ia) throws IOException {
initialized = true;
}
+ /**
+ * Deserialize a snapshot that contains FileHeader from an input archive. It is used by
+ * the admin restore command.
+ *
+ * @param ia the input archive to deserialize from
+ * @param is the CheckInputStream to check integrity
+ *
+ * @throws IOException
+ */
+ public void deserializeSnapshot(final InputArchive ia, final CheckedInputStream is) throws IOException {
+ // clear the zkDatabase
+ clear();
Review Comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `ZKDatabase.deserializeSnapshot(...)` indirectly writes to field `this.dataTree` outside of synchronization.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
---
<details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary>
You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
| **Command** | **Usage** |
| ------------- | ------------- |
| `@sonatype-lift ignore` | Leave out the above finding from this PR |
| `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
| `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
**Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
<sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
---
Was this a good recommendation?
[ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=360445341&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=360445341&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=360445341&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=360445341&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=360445341&lift_comment_rating=5) ]
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org