From 6f8f5c109c2b3bfdbb39f638387695160fb9f23d Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 27 Oct 2021 18:50:08 -0400 Subject: [PATCH 01/31] write archive uploads into the existing archives directory mount a testing archives directory on disk rather than using tmpfs --- .gitignore | 1 + run.sh | 6 +++++- .../web/http/api/v1/RecordingsPostBodyHandler.java | 12 ++++++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 75c2911c95..3b76c3fa1e 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ build/ target/ bin/ +archive/ conf/ truststore/ clientlib/ diff --git a/run.sh b/run.sh index 124505991b..9435c3a9c6 100755 --- a/run.sh +++ b/run.sh @@ -48,6 +48,10 @@ if [ -z "$KEYSTORE_PATH" ] && [ -f "$(dirname $0)/certs/cryostat-keystore.p12" ] KEYSTORE_PASS="$(cat $(dirname $0)/certs/keystore.pass)" fi +if [ ! -d "$(dirname $0)/archive" ]; then + mkdir "$(dirname $0)/archive" +fi + if [ ! -d "$(dirname $0)/conf" ]; then mkdir "$(dirname $0)/conf" fi @@ -79,8 +83,8 @@ fi podman run \ --pod cryostat \ --memory 512M \ - --mount type=tmpfs,target=/opt/cryostat.d/recordings.d \ --mount type=tmpfs,target=/opt/cryostat.d/templates.d \ + --mount type=bind,source="$(dirname $0)/archive",destination=/opt/cryostat.d/recordings.d,relabel=shared,bind-propagation=shared \ --mount type=bind,source="$(dirname $0)/conf",destination=/opt/cryostat.d/conf.d,relabel=shared,bind-propagation=shared \ --mount type=bind,source="$(dirname $0)/truststore",destination=/truststore,relabel=shared,bind-propagation=shared \ --mount type=bind,source="$(dirname $0)/certs",destination=/certs,relabel=shared,bind-propagation=shared \ diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java index 899b316f69..8b71c2a697 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java @@ -37,10 +37,13 @@ */ package io.cryostat.net.web.http.api.v1; +import java.nio.file.Path; import java.util.Set; import javax.inject.Inject; +import javax.inject.Named; +import io.cryostat.MainModule; import io.cryostat.net.AuthManager; import io.cryostat.net.security.ResourceAction; import io.cryostat.net.web.http.AbstractAuthenticatedRequestHandler; @@ -55,9 +58,14 @@ class RecordingsPostBodyHandler extends AbstractAuthenticatedRequestHandler { private final BodyHandler bodyHandler; @Inject - RecordingsPostBodyHandler(AuthManager auth) { + RecordingsPostBodyHandler( + AuthManager auth, @Named(MainModule.RECORDINGS_PATH) Path recordingsPath) { super(auth); - this.bodyHandler = BodyHandler.create(true); + this.bodyHandler = + BodyHandler.create( + recordingsPath.resolve("file-uploads").toAbsolutePath().toString()) + .setHandleFileUploads(true) + .setPreallocateBodyBuffer(true); } @Override From 854dcb54122bc0fa987fcdc1d60ac9a4c4792fbc Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 27 Oct 2021 19:00:23 -0400 Subject: [PATCH 02/31] perform faster and lighter validation of uploaded binaries --- .../http/api/v1/RecordingsPostHandler.java | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java index a651370c74..3885770496 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java @@ -37,13 +37,16 @@ */ package io.cryostat.net.web.http.api.v1; -import java.io.File; +import java.io.BufferedInputStream; +import java.io.FileInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Instant; import java.util.EnumSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -51,7 +54,8 @@ import javax.inject.Named; import org.openjdk.jmc.flightrecorder.CouldNotLoadRecordingException; -import org.openjdk.jmc.flightrecorder.JfrLoaderToolkit; +import org.openjdk.jmc.flightrecorder.internal.FlightRecordingLoader; +import org.openjdk.jmc.flightrecorder.internal.InvalidJfrFileException; import io.cryostat.MainModule; import io.cryostat.core.log.Logger; @@ -145,6 +149,8 @@ public boolean isOrdered() { @Override public void handleAuthenticated(RoutingContext ctx) throws Exception { + long start = System.currentTimeMillis(); + logger.info("RecordingsPostHandler started at {}", Instant.ofEpochMilli(start)); if (!fs.isDirectory(savedRecordingsPath)) { throw new HttpStatusException(503, "Recording saving not available"); } @@ -209,6 +215,10 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { logger.info("Recording saved as {}", res2.result()); + logger.info( + "RecordingsPostHandler completed successfully in {}ms", + System.currentTimeMillis() - start); + notificationFactory .createBuilder() .metaCategory(NOTIFICATION_CATEGORY) @@ -222,12 +232,25 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { private void validateRecording(String recordingFile, Handler> handler) { vertx.executeBlocking( event -> { + // FIXME remove logging and timing. Add a custom JFR event for the Cryostat + // event profile? + long start = System.nanoTime(); + logger.info("Starting JFR validation for {}", recordingFile); try { - JfrLoaderToolkit.loadEvents( - new File(recordingFile)); // try loading events to see if - // it's a valid file + // try loading chunk info to see if it's a valid file + try (var is = new BufferedInputStream(new FileInputStream(recordingFile))) { + var supplier = FlightRecordingLoader.createChunkSupplier(is); + var chunks = FlightRecordingLoader.readChunkInfo(supplier); + if (chunks.size() < 1) { + throw new InvalidJfrFileException(); + } + } + logger.info( + "JFR validation succeeded in {}ms", + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)); event.complete(); } catch (CouldNotLoadRecordingException | IOException e) { + logger.info("JFR validation FAILED"); event.fail(e); } }, From 3225edd9e1e5b3ed62343ed25157f4a374262fc1 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 28 Oct 2021 10:31:04 -0400 Subject: [PATCH 03/31] implement binary file uploading beta recordings handler --- .../web/http/api/beta/HttpApiBetaModule.java | 4 + .../http/api/beta/RecordingsPostHandler.java | 397 ++++++++++++++++++ .../api/v1/RecordingsPostBodyHandler.java | 4 +- 3 files changed, 402 insertions(+), 3 deletions(-) create mode 100644 src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java b/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java index 0d39ccc928..0a521a3c12 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java @@ -48,4 +48,8 @@ public abstract class HttpApiBetaModule { @Binds @IntoSet abstract RequestHandler bindDiscoveryGetHandler(DiscoveryGetHandler handler); + + @Binds + @IntoSet + abstract RequestHandler bindRecordingsPostHandler(RecordingsPostHandler handler); } diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java new file mode 100644 index 0000000000..1c4e2446b9 --- /dev/null +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -0,0 +1,397 @@ +/* + * Copyright The Cryostat Authors + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or data + * (collectively the "Software"), free of charge and under any and all copyright + * rights in the Software, and any and all patent rights owned or freely + * licensable by each licensor hereunder covering either (i) the unmodified + * Software as contributed to or provided by such licensor, or (ii) the Larger + * Works (as defined below), to deal in both + * + * (a) the Software, and + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software (each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * The above copyright notice and either this complete permission notice or at + * a minimum a reference to the UPL must be included in all copies or + * substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package io.cryostat.net.web.http.api.beta; + +import java.io.BufferedInputStream; +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Instant; +import java.util.EnumSet; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import javax.inject.Inject; +import javax.inject.Named; + +import org.openjdk.jmc.flightrecorder.CouldNotLoadRecordingException; +import org.openjdk.jmc.flightrecorder.internal.FlightRecordingLoader; +import org.openjdk.jmc.flightrecorder.internal.InvalidJfrFileException; + +import io.cryostat.MainModule; +import io.cryostat.core.log.Logger; +import io.cryostat.core.sys.FileSystem; +import io.cryostat.messaging.notifications.NotificationFactory; +import io.cryostat.net.AuthManager; +import io.cryostat.net.HttpServer; +import io.cryostat.net.security.ResourceAction; +import io.cryostat.net.web.http.HttpMimeType; +import io.cryostat.net.web.http.RequestHandler; +import io.cryostat.net.web.http.api.ApiVersion; + +import com.google.gson.Gson; +import io.vertx.core.AsyncResult; +import io.vertx.core.Handler; +import io.vertx.core.Vertx; +import io.vertx.core.file.AsyncFile; +import io.vertx.core.file.OpenOptions; +import io.vertx.core.http.HttpHeaders; +import io.vertx.core.http.HttpMethod; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.ext.web.RoutingContext; +import io.vertx.ext.web.handler.impl.HttpStatusException; +import org.apache.commons.lang3.StringUtils; + +class RecordingsPostHandler implements RequestHandler { + + private static final Pattern RECORDING_FILENAME_PATTERN = + Pattern.compile("([A-Za-z\\d-]*)_([A-Za-z\\d-_]*)_([\\d]*T[\\d]*Z)(\\.[\\d]+)?"); + + private final AuthManager auth; + private final Vertx vertx; + private final FileSystem fs; + private final Path savedRecordingsPath; + private final Gson gson; + private final Logger logger; + private final NotificationFactory notificationFactory; + + private static final String NOTIFICATION_CATEGORY = "RecordingSaved"; + + @Inject + RecordingsPostHandler( + AuthManager auth, + HttpServer httpServer, + FileSystem fs, + @Named(MainModule.RECORDINGS_PATH) Path savedRecordingsPath, + Gson gson, + Logger logger, + NotificationFactory notificationFactory) { + this.auth = auth; + this.vertx = httpServer.getVertx(); + this.fs = fs; + this.savedRecordingsPath = savedRecordingsPath; + this.gson = gson; + this.logger = logger; + this.notificationFactory = notificationFactory; + } + + @Override + public ApiVersion apiVersion() { + return ApiVersion.BETA; + } + + @Override + public HttpMethod httpMethod() { + return HttpMethod.POST; + } + + @Override + public Set resourceActions() { + return EnumSet.of(ResourceAction.CREATE_RECORDING); + } + + @Override + public String path() { + return basePath() + "recordings/:recordingName"; + } + + @Override + public boolean isAsync() { + return false; + } + + @Override + public boolean isOrdered() { + return true; + } + + private Future validateRequestAuthorization(HttpServerRequest req) throws Exception { + return auth.validateHttpHeader( + () -> req.getHeader(HttpHeaders.AUTHORIZATION), resourceActions()); + } + + @Override + public void handle(RoutingContext ctx) { + String desiredSaveName = ctx.pathParam("recordingName"); + if (StringUtils.isBlank(desiredSaveName)) { + throw new HttpStatusException(400, "Recording name must not be empty"); + } + Path dest = + savedRecordingsPath + .toAbsolutePath() + .resolve("file-uploads") + .resolve(UUID.randomUUID().toString()); + ctx.vertx().fileSystem().createFileBlocking(dest.toString()); + AsyncFile as = + ctx.vertx() + .fileSystem() + .openBlocking(dest.toString(), new OpenOptions().setAppend(true)); + CompletableFuture fileUploadPath = new CompletableFuture<>(); + ctx.request() + .handler( + buffer -> { + as.write(buffer); + }); + ctx.request() + .endHandler( + ar -> { + as.close(); + fileUploadPath.complete(dest); + }); + + try { + boolean permissionGranted = validateRequestAuthorization(ctx.request()).get(); + if (!permissionGranted) { + throw new HttpStatusException(401, "HTTP Authorization Failure"); + } + } catch (Exception e) { + throw new HttpStatusException(500, e.getMessage(), e); + } + + if (!fs.isDirectory(savedRecordingsPath)) { + throw new HttpStatusException(503, "Recording saving not available"); + } + + Path upload; + try { + upload = fileUploadPath.get(); + } catch (Exception e) { + throw new HttpStatusException(500, e); + } + + if (desiredSaveName.endsWith(".jfr")) { + desiredSaveName = desiredSaveName.substring(0, desiredSaveName.length() - 4); + } + + Matcher m = RECORDING_FILENAME_PATTERN.matcher(desiredSaveName); + if (!m.matches()) { + throw new HttpStatusException(400, "Incorrect recording file name pattern"); + } + + String targetName = m.group(1); + String recordingName = m.group(2); + String timestamp = m.group(3); + int count = + m.group(4) == null || m.group(4).isEmpty() + ? 0 + : Integer.parseInt(m.group(4).substring(1)); + + final String subdirectoryName = "unlabelled"; + final String basename = String.format("%s_%s_%s", targetName, recordingName, timestamp); + final String uploadedFileName = upload.toString(); + validateRecording( + uploadedFileName, + (res) -> + saveRecording( + subdirectoryName, + basename, + uploadedFileName, + count, + (res2) -> { + if (res2.failed()) { + ctx.fail(res2.cause()); + return; + } + + ctx.response() + .putHeader( + HttpHeaders.CONTENT_TYPE, + HttpMimeType.JSON.mime()) + .end(gson.toJson(Map.of("name", res2.result()))); + + notificationFactory + .createBuilder() + .metaCategory(NOTIFICATION_CATEGORY) + .metaType(HttpMimeType.JSON) + .message(Map.of("recording", res2.result())) + .build() + .send(); + })); + } + + private void validateRecording(String recordingFile, Handler> handler) { + vertx.executeBlocking( + event -> { + try { + // try loading chunk info to see if it's a valid file + try (var is = new BufferedInputStream(new FileInputStream(recordingFile))) { + var supplier = FlightRecordingLoader.createChunkSupplier(is); + var chunks = FlightRecordingLoader.readChunkInfo(supplier); + if (chunks.size() < 1) { + throw new InvalidJfrFileException(); + } + } + event.complete(); + } catch (CouldNotLoadRecordingException | IOException e) { + event.fail(e); + } + }, + res -> { + if (res.failed()) { + Throwable t; + if (res.cause() instanceof CouldNotLoadRecordingException) { + t = + new HttpStatusException( + 400, "Not a valid JFR recording file", res.cause()); + } else { + t = res.cause(); + } + + handler.handle(makeFailedAsyncResult(t)); + return; + } + + handler.handle(makeAsyncResult(null)); + }); + } + + private void saveRecording( + String subdirectoryName, + String basename, + String tmpFile, + int counter, + Handler> handler) { + // TODO byte-sized rename limit is arbitrary. Probably plenty since recordings + // are also differentiated by second-resolution timestamp + if (counter >= Byte.MAX_VALUE) { + handler.handle( + makeFailedAsyncResult( + new IOException( + "Recording could not be saved. File already exists and rename attempts were exhausted."))); + return; + } + + String filename = counter > 1 ? basename + "." + counter + ".jfr" : basename + ".jfr"; + Path specificRecordingsPath = savedRecordingsPath.resolve(subdirectoryName); + + if (!fs.exists(specificRecordingsPath)) { + try { + Files.createDirectory(specificRecordingsPath); + } catch (IOException e) { + handler.handle(makeFailedAsyncResult(e)); + return; + } + } + + vertx.fileSystem() + .exists( + specificRecordingsPath.resolve(filename).toString(), + (res) -> { + if (res.failed()) { + handler.handle(makeFailedAsyncResult(res.cause())); + return; + } + + if (res.result()) { + saveRecording( + subdirectoryName, basename, tmpFile, counter + 1, handler); + return; + } + + // verified no name clash at this time + vertx.fileSystem() + .move( + tmpFile, + specificRecordingsPath.resolve(filename).toString(), + (res2) -> { + if (res2.failed()) { + handler.handle( + makeFailedAsyncResult(res2.cause())); + return; + } + + handler.handle(makeAsyncResult(filename)); + }); + }); + } + + private AsyncResult makeAsyncResult(T result) { + return new AsyncResult<>() { + @Override + public T result() { + return result; + } + + @Override + public Throwable cause() { + return null; + } + + @Override + public boolean succeeded() { + return true; + } + + @Override + public boolean failed() { + return false; + } + }; + } + + private AsyncResult makeFailedAsyncResult(Throwable cause) { + return new AsyncResult<>() { + @Override + public T result() { + return null; + } + + @Override + public Throwable cause() { + return cause; + } + + @Override + public boolean succeeded() { + return false; + } + + @Override + public boolean failed() { + return true; + } + }; + } +} diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java index 8b71c2a697..4a6572de27 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java @@ -63,9 +63,7 @@ class RecordingsPostBodyHandler extends AbstractAuthenticatedRequestHandler { super(auth); this.bodyHandler = BodyHandler.create( - recordingsPath.resolve("file-uploads").toAbsolutePath().toString()) - .setHandleFileUploads(true) - .setPreallocateBodyBuffer(true); + recordingsPath.resolve("file-uploads").toAbsolutePath().toString()); } @Override From 43cf9dfba2956f068b6a107b4b73529b380d483e Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 28 Oct 2021 14:50:36 -0400 Subject: [PATCH 04/31] document beta api handlers --- HTTP_API.md | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/HTTP_API.md b/HTTP_API.md index 31fb5c96e6..21e9a47a3e 100644 --- a/HTTP_API.md +++ b/HTTP_API.md @@ -2,6 +2,7 @@ * [V1](#V1-API) * [V2](#V2-API) +* [Beta](#Beta-API) ## V1 API @@ -1696,3 +1697,101 @@ The handler-specific descriptions below describe how each handler populates the $ curl -F cert=@vertx-fib-demo.cer localhost:8181/api/v2/certificates {"meta":{"type":"text/plain","status":"OK"},"data":{"result":"/truststore/vertx-fib-demo.cer"}} ``` + +## Beta API + +### Quick Reference + +| What you want to do | Which handler you should use | +| ------------------------------------------------------------------------- | --------------------------------------------------------------------------------| +| **Miscellaneous** | | +| View targets in overall deployment environment | [`DiscoveryGetHandler`](#DiscoveryGetHandler) | +| **Recordings in archive** | | +| Upload a recording to archive | [`RecordingsPostHandler`](#RecordingsPostHandler-1) | + +### Miscellaneous + +* #### `DiscoveryGetHandler` + + ###### synopsis + Queries the platform client(s) for the discoverable targets and constructs a + hierarchical tree view of the full deployment environment with targets + belonging to ex. Pods, belonging to Deployments, etc. + + ###### request + `GET /api/beta/discovery` + + ###### response + `200` - The result is the path of the saved file in the server's storage. + + `401` - The user does not have sufficient permissions. + +### Recordings in archive + +* #### `RecordingsPostHandler-1` + + ###### synopsis + Uploads a recording to the archives as a binary file upload rather than a + multipart-form-data upload. This is faster than the API V1 multipart form + upload and should be preferred, particularly for larger JFR files. + + ###### request + `POST /api/beta/recordings/:recordingName` + + The `recordingName` is the name of the JFR file that will be written in the + archives and must be in the following format, which is the same format that + Cryostat uses for recordings it saves itself (all fields in brackets are + optional): + ``` + [$TARGET_NAME]_[$RECORDING_NAME]_[$DATE]T[$TIME]Z[.$COUNTER][.jfr] + ``` + + `TARGET_NAME` - The name of the target JVM (alphanumerics and hyphens allowed) + + `RECORDING_NAME` - The name of the recording + (alphanumerics, hyphens, and underscores allowed) + + `DATE` - The date of the recording (numbers allowed) + + `TIME` - The time of the recording (numbers allowed) + + `COUNTER` - An additional number used to avoid name collisions + (numbers allowed) + + Formally, the required format is: + ``` + ([A-Za-z\d-]*)_([A-Za-z\d-_]*)_([\d]*T[\d]*Z)(\.[\d]+)?(\.jfr)? + ``` + + ###### response + `200` - The body is `{"name":"$NAME"}`, where `$NAME` is the name of the + recording that is now saved in archive. + This name may be different from the filename of the uploaded file + for two reasons. + + First, if there is a name collision, a counter will be added to the + original filename, or the existing counter will be modified, + and the new counter will be set to the next available number, + starting from `2`. A counter of `1` will be removed if there is no + name conflict, and modified to the next available number if there is. + + And second, if the filename of the uploaded file does not include a `.jfr` + ending, one will be added. + + `400` - The recording submission is invalid. The body is an error + message. + + `401` - User authentication failed. The body is an error message. + There will be an `X-WWW-Authenticate: $SCHEME` header that indicates + the authentication scheme that is used. + + `500` - There was an unexpected error. The body is an error message. + + `503` - `CRYOSTAT_ARCHIVE_PATH` is an invalid directory. + The body is an error message. + + ###### example + ``` + $ curl -kv --progress-bar --data-binary @vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr localhost:8181/api/beta/recordings/vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr | cat + {"name":"vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr"} + ``` From bba32761d36172f966cf2566790c8b24910a5b19 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 28 Oct 2021 14:50:56 -0400 Subject: [PATCH 05/31] pause/resume request to ensure all buffer data is correctly handled --- .../net/web/http/api/beta/RecordingsPostHandler.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 1c4e2446b9..2a91d820cb 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -42,14 +42,12 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.time.Instant; import java.util.EnumSet; import java.util.Map; import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -154,6 +152,7 @@ private Future validateRequestAuthorization(HttpServerRequest req) thro @Override public void handle(RoutingContext ctx) { + ctx.request().pause(); String desiredSaveName = ctx.pathParam("recordingName"); if (StringUtils.isBlank(desiredSaveName)) { throw new HttpStatusException(400, "Recording name must not be empty"); @@ -196,6 +195,7 @@ public void handle(RoutingContext ctx) { Path upload; try { + ctx.request().resume(); upload = fileUploadPath.get(); } catch (Exception e) { throw new HttpStatusException(500, e); @@ -265,6 +265,7 @@ private void validateRecording(String recordingFile, Handler> } event.complete(); } catch (CouldNotLoadRecordingException | IOException e) { + // FIXME need to reject the request and clean up the file here event.fail(e); } }, From 1532b29c0bdedde2b941956d805bec078fac3b10 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 28 Oct 2021 16:27:32 -0400 Subject: [PATCH 06/31] Set content-type in example --- HTTP_API.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HTTP_API.md b/HTTP_API.md index 21e9a47a3e..7829b554e1 100644 --- a/HTTP_API.md +++ b/HTTP_API.md @@ -1792,6 +1792,6 @@ The handler-specific descriptions below describe how each handler populates the ###### example ``` - $ curl -kv --progress-bar --data-binary @vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr localhost:8181/api/beta/recordings/vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr | cat + $ curl -kv --progress-bar -H Content-Type:'application/octet-stream' --data-binary @vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr localhost:8181/api/beta/recordings/vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr | cat {"name":"vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr"} ``` From 74f22892fbef0bacb097f213298fb16359a08dd8 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 28 Oct 2021 16:27:46 -0400 Subject: [PATCH 07/31] Allow Content-Type headers in CORS --- .../io/cryostat/net/web/http/generic/CorsEnablingHandler.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/io/cryostat/net/web/http/generic/CorsEnablingHandler.java b/src/main/java/io/cryostat/net/web/http/generic/CorsEnablingHandler.java index bc7ff58dd4..626c56c1bc 100644 --- a/src/main/java/io/cryostat/net/web/http/generic/CorsEnablingHandler.java +++ b/src/main/java/io/cryostat/net/web/http/generic/CorsEnablingHandler.java @@ -48,6 +48,7 @@ import io.cryostat.net.web.http.RequestHandler; import io.cryostat.net.web.http.api.ApiVersion; +import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; import io.vertx.ext.web.RoutingContext; import io.vertx.ext.web.handler.CorsHandler; @@ -65,6 +66,7 @@ class CorsEnablingHandler implements RequestHandler { CorsHandler.create(getOrigin()) .allowedHeader("Authorization") .allowedHeader(AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER) + .allowedHeader(HttpHeaders.CONTENT_TYPE.toString()) .allowedMethod(HttpMethod.GET) .allowedMethod(HttpMethod.POST) .allowedMethod(HttpMethod.PATCH) From ffd88ec6215bac956badb9ad8bd76c21e3409a4f Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 28 Oct 2021 16:33:31 -0400 Subject: [PATCH 08/31] refactor to use more vertx async io --- .../http/api/beta/RecordingsPostHandler.java | 222 +++++++++++------- 1 file changed, 141 insertions(+), 81 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 2a91d820cb..5b9f9f09a5 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -44,9 +44,11 @@ import java.nio.file.Path; import java.util.EnumSet; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -73,7 +75,6 @@ import io.vertx.core.AsyncResult; import io.vertx.core.Handler; import io.vertx.core.Vertx; -import io.vertx.core.file.AsyncFile; import io.vertx.core.file.OpenOptions; import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; @@ -88,7 +89,6 @@ class RecordingsPostHandler implements RequestHandler { Pattern.compile("([A-Za-z\\d-]*)_([A-Za-z\\d-_]*)_([\\d]*T[\\d]*Z)(\\.[\\d]+)?"); private final AuthManager auth; - private final Vertx vertx; private final FileSystem fs; private final Path savedRecordingsPath; private final Gson gson; @@ -107,7 +107,6 @@ class RecordingsPostHandler implements RequestHandler { Logger logger, NotificationFactory notificationFactory) { this.auth = auth; - this.vertx = httpServer.getVertx(); this.fs = fs; this.savedRecordingsPath = savedRecordingsPath; this.gson = gson; @@ -137,12 +136,12 @@ public String path() { @Override public boolean isAsync() { - return false; + return true; } @Override public boolean isOrdered() { - return true; + return false; } private Future validateRequestAuthorization(HttpServerRequest req) throws Exception { @@ -155,103 +154,157 @@ public void handle(RoutingContext ctx) { ctx.request().pause(); String desiredSaveName = ctx.pathParam("recordingName"); if (StringUtils.isBlank(desiredSaveName)) { - throw new HttpStatusException(400, "Recording name must not be empty"); + ctx.fail(400, new HttpStatusException(400, "Recording name must not be empty")); + return; + } + + if (desiredSaveName.endsWith(".jfr")) { + desiredSaveName = desiredSaveName.substring(0, desiredSaveName.length() - 4); + } + Matcher m = RECORDING_FILENAME_PATTERN.matcher(desiredSaveName); + if (!m.matches()) { + ctx.fail(400, new HttpStatusException(400, "Incorrect recording file name pattern")); + return; } + Path dest = savedRecordingsPath .toAbsolutePath() .resolve("file-uploads") .resolve(UUID.randomUUID().toString()); - ctx.vertx().fileSystem().createFileBlocking(dest.toString()); - AsyncFile as = - ctx.vertx() - .fileSystem() - .openBlocking(dest.toString(), new OpenOptions().setAppend(true)); CompletableFuture fileUploadPath = new CompletableFuture<>(); - ctx.request() - .handler( - buffer -> { - as.write(buffer); - }); - ctx.request() - .endHandler( - ar -> { - as.close(); - fileUploadPath.complete(dest); + ctx.vertx() + .fileSystem() + .createFile( + dest.toString(), + createFile -> { + if (createFile.failed()) { + ctx.fail(500, createFile.cause()); + return; + } + ctx.vertx() + .fileSystem() + .open( + dest.toString(), + new OpenOptions().setAppend(true), + openFile -> { + if (openFile.failed()) { + ctx.fail(500, openFile.cause()); + return; + } + ctx.request() + .handler( + buffer -> + openFile.result() + .write(buffer)) + .endHandler( + v -> { + openFile.result().close(); + fileUploadPath.complete(dest); + }); + ctx.request().resume(); + }); }); try { boolean permissionGranted = validateRequestAuthorization(ctx.request()).get(); if (!permissionGranted) { - throw new HttpStatusException(401, "HTTP Authorization Failure"); + ctx.fail(401, new HttpStatusException(401, "HTTP Authorization Failure")); + return; } } catch (Exception e) { - throw new HttpStatusException(500, e.getMessage(), e); + ctx.fail(500, e); + return; } if (!fs.isDirectory(savedRecordingsPath)) { - throw new HttpStatusException(503, "Recording saving not available"); - } - - Path upload; - try { - ctx.request().resume(); - upload = fileUploadPath.get(); - } catch (Exception e) { - throw new HttpStatusException(500, e); - } - - if (desiredSaveName.endsWith(".jfr")) { - desiredSaveName = desiredSaveName.substring(0, desiredSaveName.length() - 4); + ctx.fail(503, new HttpStatusException(503, "Recording saving not available")); + return; } - Matcher m = RECORDING_FILENAME_PATTERN.matcher(desiredSaveName); - if (!m.matches()) { - throw new HttpStatusException(400, "Incorrect recording file name pattern"); + if (!Objects.equals( + HttpMimeType.OCTET_STREAM.mime(), + ctx.request().getHeader(HttpHeaders.CONTENT_TYPE))) { + ctx.fail(400); + return; } - String targetName = m.group(1); - String recordingName = m.group(2); - String timestamp = m.group(3); - int count = - m.group(4) == null || m.group(4).isEmpty() - ? 0 - : Integer.parseInt(m.group(4).substring(1)); - - final String subdirectoryName = "unlabelled"; - final String basename = String.format("%s_%s_%s", targetName, recordingName, timestamp); - final String uploadedFileName = upload.toString(); - validateRecording( - uploadedFileName, - (res) -> - saveRecording( - subdirectoryName, - basename, - uploadedFileName, - count, - (res2) -> { - if (res2.failed()) { - ctx.fail(res2.cause()); - return; - } - - ctx.response() - .putHeader( - HttpHeaders.CONTENT_TYPE, - HttpMimeType.JSON.mime()) - .end(gson.toJson(Map.of("name", res2.result()))); - - notificationFactory - .createBuilder() - .metaCategory(NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message(Map.of("recording", res2.result())) - .build() - .send(); - })); + ctx.vertx() + .executeBlocking( + event -> { + try { + event.complete(fileUploadPath.get()); + } catch (ExecutionException | InterruptedException e) { + event.fail(e); + } + }, + ar -> { + if (ar.failed()) { + ctx.fail(500, ar.cause()); + return; + } + Path upload = (Path) ar.result(); + + String targetName = m.group(1); + String recordingName = m.group(2); + String timestamp = m.group(3); + int count = + m.group(4) == null || m.group(4).isEmpty() + ? 0 + : Integer.parseInt(m.group(4).substring(1)); + + final String subdirectoryName = "unlabelled"; + final String basename = + String.format("%s_%s_%s", targetName, recordingName, timestamp); + final String uploadedFileName = upload.toString(); + validateRecording( + ctx.vertx(), + uploadedFileName, + res -> { + if (res.failed()) { + ctx.fail(400, res.cause()); + return; + } + saveRecording( + ctx.vertx(), + subdirectoryName, + basename, + uploadedFileName, + count, + res2 -> { + if (res2.failed()) { + ctx.fail(res2.cause()); + return; + } + + ctx.response() + .putHeader( + HttpHeaders.CONTENT_TYPE, + HttpMimeType.JSON.mime()) + .end( + gson.toJson( + Map.of( + "name", + res2 + .result()))); + + notificationFactory + .createBuilder() + .metaCategory(NOTIFICATION_CATEGORY) + .metaType(HttpMimeType.JSON) + .message( + Map.of( + "recording", + res2.result())) + .build() + .send(); + }); + }); + }); } - private void validateRecording(String recordingFile, Handler> handler) { + private void validateRecording( + Vertx vertx, String recordingFile, Handler> handler) { vertx.executeBlocking( event -> { try { @@ -260,7 +313,7 @@ private void validateRecording(String recordingFile, Handler> var supplier = FlightRecordingLoader.createChunkSupplier(is); var chunks = FlightRecordingLoader.readChunkInfo(supplier); if (chunks.size() < 1) { - throw new InvalidJfrFileException(); + event.fail(new InvalidJfrFileException()); } } event.complete(); @@ -279,6 +332,7 @@ private void validateRecording(String recordingFile, Handler> } else { t = res.cause(); } + vertx.fileSystem().deleteBlocking(recordingFile); handler.handle(makeFailedAsyncResult(t)); return; @@ -289,6 +343,7 @@ private void validateRecording(String recordingFile, Handler> } private void saveRecording( + Vertx vertx, String subdirectoryName, String basename, String tmpFile, @@ -327,7 +382,12 @@ private void saveRecording( if (res.result()) { saveRecording( - subdirectoryName, basename, tmpFile, counter + 1, handler); + vertx, + subdirectoryName, + basename, + tmpFile, + counter + 1, + handler); return; } From fffab1359800257dde28b613e3b6ab520aa83e73 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 28 Oct 2021 18:22:47 -0400 Subject: [PATCH 09/31] handle read timeouts --- .../http/api/beta/RecordingsPostHandler.java | 102 +++++++++++------- 1 file changed, 66 insertions(+), 36 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 5b9f9f09a5..757a5841a7 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -42,6 +42,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; import java.util.EnumSet; import java.util.Map; import java.util.Objects; @@ -50,6 +51,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -87,6 +90,7 @@ class RecordingsPostHandler implements RequestHandler { private static final Pattern RECORDING_FILENAME_PATTERN = Pattern.compile("([A-Za-z\\d-]*)_([A-Za-z\\d-_]*)_([\\d]*T[\\d]*Z)(\\.[\\d]+)?"); + private static final Duration READ_TIMEOUT = Duration.ofSeconds(10); private final AuthManager auth; private final FileSystem fs; @@ -94,6 +98,7 @@ class RecordingsPostHandler implements RequestHandler { private final Gson gson; private final Logger logger; private final NotificationFactory notificationFactory; + private final AtomicLong lastReadTimestamp = new AtomicLong(0); private static final String NOTIFICATION_CATEGORY = "RecordingSaved"; @@ -136,7 +141,7 @@ public String path() { @Override public boolean isAsync() { - return true; + return false; } @Override @@ -154,8 +159,7 @@ public void handle(RoutingContext ctx) { ctx.request().pause(); String desiredSaveName = ctx.pathParam("recordingName"); if (StringUtils.isBlank(desiredSaveName)) { - ctx.fail(400, new HttpStatusException(400, "Recording name must not be empty")); - return; + throw new HttpStatusException(400, "Recording name must not be empty"); } if (desiredSaveName.endsWith(".jfr")) { @@ -163,45 +167,77 @@ public void handle(RoutingContext ctx) { } Matcher m = RECORDING_FILENAME_PATTERN.matcher(desiredSaveName); if (!m.matches()) { - ctx.fail(400, new HttpStatusException(400, "Incorrect recording file name pattern")); - return; + throw new HttpStatusException(400, "Incorrect recording file name pattern"); } - Path dest = + String destinationFile = savedRecordingsPath .toAbsolutePath() .resolve("file-uploads") - .resolve(UUID.randomUUID().toString()); - CompletableFuture fileUploadPath = new CompletableFuture<>(); + .resolve(UUID.randomUUID().toString()) + .toString(); + CompletableFuture fileUploadPath = new CompletableFuture<>(); + long timerId = + ctx.vertx() + .setPeriodic( + READ_TIMEOUT.toMillis(), + id -> { + if (System.nanoTime() - lastReadTimestamp.get() + > READ_TIMEOUT.toNanos()) { + fileUploadPath.completeExceptionally( + new TimeoutException()); + } + }); ctx.vertx() .fileSystem() .createFile( - dest.toString(), + destinationFile, createFile -> { if (createFile.failed()) { - ctx.fail(500, createFile.cause()); - return; + throw new HttpStatusException(500, createFile.cause()); } ctx.vertx() .fileSystem() .open( - dest.toString(), + destinationFile, new OpenOptions().setAppend(true), openFile -> { if (openFile.failed()) { - ctx.fail(500, openFile.cause()); - return; + throw new HttpStatusException( + 500, openFile.cause()); } ctx.request() .handler( - buffer -> - openFile.result() - .write(buffer)) + buffer -> { + lastReadTimestamp.set( + System.nanoTime()); + openFile.result().write(buffer); + }) + .exceptionHandler( + t -> { + openFile.result().close(); + ctx.vertx() + .fileSystem() + .deleteBlocking( + destinationFile); + ctx.vertx() + .cancelTimer(timerId); + fileUploadPath + .completeExceptionally( + t); + }) .endHandler( v -> { + ctx.vertx() + .cancelTimer(timerId); openFile.result().close(); - fileUploadPath.complete(dest); + fileUploadPath.complete( + destinationFile); }); + ctx.addEndHandler( + ar -> { + ctx.vertx().cancelTimer(timerId); + }); ctx.request().resume(); }); }); @@ -209,24 +245,20 @@ public void handle(RoutingContext ctx) { try { boolean permissionGranted = validateRequestAuthorization(ctx.request()).get(); if (!permissionGranted) { - ctx.fail(401, new HttpStatusException(401, "HTTP Authorization Failure")); - return; + throw new HttpStatusException(401, "HTTP Authorization Failure"); } } catch (Exception e) { - ctx.fail(500, e); - return; + throw new HttpStatusException(500, e); } if (!fs.isDirectory(savedRecordingsPath)) { - ctx.fail(503, new HttpStatusException(503, "Recording saving not available")); - return; + throw new HttpStatusException(503, "Recording saving not available"); } if (!Objects.equals( HttpMimeType.OCTET_STREAM.mime(), ctx.request().getHeader(HttpHeaders.CONTENT_TYPE))) { - ctx.fail(400); - return; + throw new HttpStatusException(400); } ctx.vertx() @@ -240,10 +272,10 @@ public void handle(RoutingContext ctx) { }, ar -> { if (ar.failed()) { - ctx.fail(500, ar.cause()); - return; + ctx.vertx().fileSystem().deleteBlocking(destinationFile); + throw new HttpStatusException(500, ar.cause()); } - Path upload = (Path) ar.result(); + String upload = (String) ar.result(); String targetName = m.group(1); String recordingName = m.group(2); @@ -256,25 +288,23 @@ public void handle(RoutingContext ctx) { final String subdirectoryName = "unlabelled"; final String basename = String.format("%s_%s_%s", targetName, recordingName, timestamp); - final String uploadedFileName = upload.toString(); validateRecording( ctx.vertx(), - uploadedFileName, + upload, res -> { if (res.failed()) { - ctx.fail(400, res.cause()); - return; + throw new HttpStatusException(400, res.cause()); } saveRecording( ctx.vertx(), subdirectoryName, basename, - uploadedFileName, + upload, count, res2 -> { if (res2.failed()) { - ctx.fail(res2.cause()); - return; + throw new HttpStatusException( + 500, res2.cause()); } ctx.response() From a2c2d0e221595d7fd220b1a02866872a4c2e03a8 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 29 Oct 2021 09:35:59 -0400 Subject: [PATCH 10/31] use ctx.fail rather than throwing, to ensure cleanup endhandler is invoked --- .../http/api/beta/RecordingsPostHandler.java | 51 ++++++++++--------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 757a5841a7..9c239aea8a 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -159,7 +159,8 @@ public void handle(RoutingContext ctx) { ctx.request().pause(); String desiredSaveName = ctx.pathParam("recordingName"); if (StringUtils.isBlank(desiredSaveName)) { - throw new HttpStatusException(400, "Recording name must not be empty"); + ctx.fail(400, new HttpStatusException(400, "Recording name must not be empty")); + return; } if (desiredSaveName.endsWith(".jfr")) { @@ -167,7 +168,8 @@ public void handle(RoutingContext ctx) { } Matcher m = RECORDING_FILENAME_PATTERN.matcher(desiredSaveName); if (!m.matches()) { - throw new HttpStatusException(400, "Incorrect recording file name pattern"); + ctx.fail(400, new HttpStatusException(400, "Incorrect recording file name pattern")); + return; } String destinationFile = @@ -194,7 +196,8 @@ public void handle(RoutingContext ctx) { destinationFile, createFile -> { if (createFile.failed()) { - throw new HttpStatusException(500, createFile.cause()); + ctx.fail(new HttpStatusException(500, createFile.cause())); + return; } ctx.vertx() .fileSystem() @@ -203,8 +206,10 @@ public void handle(RoutingContext ctx) { new OpenOptions().setAppend(true), openFile -> { if (openFile.failed()) { - throw new HttpStatusException( - 500, openFile.cause()); + ctx.fail( + new HttpStatusException( + 500, openFile.cause())); + return; } ctx.request() .handler( @@ -214,18 +219,8 @@ public void handle(RoutingContext ctx) { openFile.result().write(buffer); }) .exceptionHandler( - t -> { - openFile.result().close(); - ctx.vertx() - .fileSystem() - .deleteBlocking( - destinationFile); - ctx.vertx() - .cancelTimer(timerId); - fileUploadPath - .completeExceptionally( - t); - }) + fileUploadPath + ::completeExceptionally) .endHandler( v -> { ctx.vertx() @@ -245,20 +240,24 @@ public void handle(RoutingContext ctx) { try { boolean permissionGranted = validateRequestAuthorization(ctx.request()).get(); if (!permissionGranted) { - throw new HttpStatusException(401, "HTTP Authorization Failure"); + ctx.fail(401, new HttpStatusException(401, "HTTP Authorization Failure")); + return; } } catch (Exception e) { - throw new HttpStatusException(500, e); + ctx.fail(e); + return; } if (!fs.isDirectory(savedRecordingsPath)) { - throw new HttpStatusException(503, "Recording saving not available"); + ctx.fail(503, new HttpStatusException(503, "Recording saving not available")); + return; } if (!Objects.equals( HttpMimeType.OCTET_STREAM.mime(), ctx.request().getHeader(HttpHeaders.CONTENT_TYPE))) { - throw new HttpStatusException(400); + ctx.fail(400); + return; } ctx.vertx() @@ -273,7 +272,8 @@ public void handle(RoutingContext ctx) { ar -> { if (ar.failed()) { ctx.vertx().fileSystem().deleteBlocking(destinationFile); - throw new HttpStatusException(500, ar.cause()); + ctx.fail(ar.cause()); + return; } String upload = (String) ar.result(); @@ -293,7 +293,8 @@ public void handle(RoutingContext ctx) { upload, res -> { if (res.failed()) { - throw new HttpStatusException(400, res.cause()); + ctx.fail(400, res.cause()); + return; } saveRecording( ctx.vertx(), @@ -303,8 +304,8 @@ public void handle(RoutingContext ctx) { count, res2 -> { if (res2.failed()) { - throw new HttpStatusException( - 500, res2.cause()); + ctx.fail(res2.cause()); + return; } ctx.response() From 00623cb521f69589e8885a2c82c0c4b330641610 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 29 Oct 2021 12:59:24 -0400 Subject: [PATCH 11/31] fix(recordingupload): delete badly named uploaded temp files --- .../cryostat/net/web/http/api/v1/RecordingsPostHandler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java index 3885770496..e0682ed62b 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java @@ -160,7 +160,9 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { // ignore unrecognized form fields if ("recording".equals(fu.name())) { upload = fu; - break; + } else { + Path p = savedRecordingsPath.resolve("file-uploads").resolve(fu.uploadedFileName()); + vertx.fileSystem().deleteBlocking(p.toString()); } } From 40c0f619862f11a0abf074b7741c4bd986472319 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 29 Oct 2021 13:04:05 -0400 Subject: [PATCH 12/31] fix(templatesupload): delete badly named uploaded files --- .../net/web/http/api/v1/TemplatesPostHandler.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandler.java index 1e8bcb9a7f..8dc4b56a94 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandler.java @@ -113,16 +113,16 @@ public boolean isAsync() { @Override public void handleAuthenticated(RoutingContext ctx) throws Exception { + boolean handledUpload = false; try { for (FileUpload u : ctx.fileUploads()) { Path path = fs.pathOf(u.uploadedFileName()); + if (!"template".equals(u.name())) { + fs.deleteIfExists(path); + continue; + } + handledUpload = true; try (InputStream is = fs.newInputStream(path)) { - if (!"template".equals(u.name())) { - throw new HttpStatusException( - 400, - String.format( - "Received unexpected file upload named {}", u.name())); - } notificationFactory .createBuilder() .metaCategory(NOTIFICATION_CATEGORY) @@ -138,6 +138,9 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { } catch (InvalidXmlException | InvalidEventTemplateException e) { throw new HttpStatusException(400, e.getMessage(), e); } + if (!handledUpload) { + throw new HttpStatusException(400, "No template submission"); + } ctx.response().end(); } } From 321d4229f6dedc3b487c05940633f8ea4bf6751d Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 29 Oct 2021 13:06:51 -0400 Subject: [PATCH 13/31] fix(bodyhandlers): do not handle file uploads where not expected --- .../web/http/api/v1/TargetRecordingOptionsPatchBodyHandler.java | 2 +- .../net/web/http/api/v1/TargetRecordingPatchBodyHandler.java | 2 +- .../net/web/http/api/v1/TargetRecordingsPostBodyHandler.java | 2 +- .../io/cryostat/net/web/http/api/v2/RulesPostBodyHandler.java | 2 +- .../net/web/http/api/v2/TargetCredentialsPostBodyHandler.java | 2 +- .../io/cryostat/net/web/http/api/v2/TargetsPostBodyHandler.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchBodyHandler.java index a5e168f957..f6948e3dcd 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchBodyHandler.java @@ -52,7 +52,7 @@ class TargetRecordingOptionsPatchBodyHandler extends AbstractAuthenticatedRequestHandler { - static final BodyHandler BODY_HANDLER = BodyHandler.create(true); + static final BodyHandler BODY_HANDLER = BodyHandler.create(true).setHandleFileUploads(false); @Inject TargetRecordingOptionsPatchBodyHandler(AuthManager auth) { diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingPatchBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingPatchBodyHandler.java index b98995ef78..2c44c43bae 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingPatchBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingPatchBodyHandler.java @@ -52,7 +52,7 @@ class TargetRecordingPatchBodyHandler extends AbstractAuthenticatedRequestHandler { - static final BodyHandler BODY_HANDLER = BodyHandler.create(true); + static final BodyHandler BODY_HANDLER = BodyHandler.create(true).setHandleFileUploads(false); @Inject TargetRecordingPatchBodyHandler(AuthManager auth) { diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostBodyHandler.java index b2693594b0..df6f5cd60a 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostBodyHandler.java @@ -57,7 +57,7 @@ class TargetRecordingsPostBodyHandler extends AbstractAuthenticatedRequestHandle @Inject TargetRecordingsPostBodyHandler(AuthManager auth) { super(auth); - this.bodyHandler = BodyHandler.create(true); + this.bodyHandler = BodyHandler.create(true).setHandleFileUploads(false); } @Override diff --git a/src/main/java/io/cryostat/net/web/http/api/v2/RulesPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v2/RulesPostBodyHandler.java index a72b593b4d..411eb8cc78 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v2/RulesPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v2/RulesPostBodyHandler.java @@ -52,7 +52,7 @@ class RulesPostBodyHandler extends AbstractAuthenticatedRequestHandler { - static final BodyHandler BODY_HANDLER = BodyHandler.create(true); + static final BodyHandler BODY_HANDLER = BodyHandler.create(true).setHandleFileUploads(false); @Inject RulesPostBodyHandler(AuthManager auth) { diff --git a/src/main/java/io/cryostat/net/web/http/api/v2/TargetCredentialsPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v2/TargetCredentialsPostBodyHandler.java index 6f88b83fe6..2518f49a3f 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v2/TargetCredentialsPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v2/TargetCredentialsPostBodyHandler.java @@ -52,7 +52,7 @@ class TargetCredentialsPostBodyHandler extends AbstractAuthenticatedRequestHandler { - static final BodyHandler BODY_HANDLER = BodyHandler.create(true); + static final BodyHandler BODY_HANDLER = BodyHandler.create(true).setHandleFileUploads(false); @Inject TargetCredentialsPostBodyHandler(AuthManager auth) { diff --git a/src/main/java/io/cryostat/net/web/http/api/v2/TargetsPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v2/TargetsPostBodyHandler.java index b9254889da..8044edaa6a 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v2/TargetsPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v2/TargetsPostBodyHandler.java @@ -52,7 +52,7 @@ class TargetsPostBodyHandler extends AbstractAuthenticatedRequestHandler { - static final BodyHandler BODY_HANDLER = BodyHandler.create(true); + static final BodyHandler BODY_HANDLER = BodyHandler.create(true).setHandleFileUploads(false); @Inject TargetsPostBodyHandler(AuthManager auth) { From 3cd1f372a95f19b5281673744642e7652a94cf7a Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 29 Oct 2021 14:50:38 -0400 Subject: [PATCH 14/31] create and open file in one step rather than two --- .../http/api/beta/RecordingsPostHandler.java | 61 +++++++------------ 1 file changed, 22 insertions(+), 39 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 9c239aea8a..6ba797cb83 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -192,49 +192,32 @@ public void handle(RoutingContext ctx) { }); ctx.vertx() .fileSystem() - .createFile( + .open( destinationFile, - createFile -> { - if (createFile.failed()) { - ctx.fail(new HttpStatusException(500, createFile.cause())); + new OpenOptions().setAppend(true).setCreateNew(true), + openFile -> { + if (openFile.failed()) { + ctx.fail(new HttpStatusException(500, openFile.cause())); return; } - ctx.vertx() - .fileSystem() - .open( - destinationFile, - new OpenOptions().setAppend(true), - openFile -> { - if (openFile.failed()) { - ctx.fail( - new HttpStatusException( - 500, openFile.cause())); - return; - } - ctx.request() - .handler( - buffer -> { - lastReadTimestamp.set( - System.nanoTime()); - openFile.result().write(buffer); - }) - .exceptionHandler( - fileUploadPath - ::completeExceptionally) - .endHandler( - v -> { - ctx.vertx() - .cancelTimer(timerId); - openFile.result().close(); - fileUploadPath.complete( - destinationFile); - }); - ctx.addEndHandler( - ar -> { - ctx.vertx().cancelTimer(timerId); - }); - ctx.request().resume(); + ctx.request() + .handler( + buffer -> { + lastReadTimestamp.set(System.nanoTime()); + openFile.result().write(buffer); + }) + .exceptionHandler(fileUploadPath::completeExceptionally) + .endHandler( + v -> { + ctx.vertx().cancelTimer(timerId); + openFile.result().close(); + fileUploadPath.complete(destinationFile); }); + ctx.addEndHandler( + ar -> { + ctx.vertx().cancelTimer(timerId); + }); + ctx.request().resume(); }); try { From 71e18e2dee8244ea74bb406d503e1c2485a783b6 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 29 Oct 2021 15:37:59 -0400 Subject: [PATCH 15/31] improve stream-to-file handling --- .../http/api/beta/RecordingsPostHandler.java | 60 +++++++++---------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 6ba797cb83..669557d266 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -42,7 +42,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.time.Duration; import java.util.EnumSet; import java.util.Map; import java.util.Objects; @@ -51,8 +50,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; -import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -78,6 +75,7 @@ import io.vertx.core.AsyncResult; import io.vertx.core.Handler; import io.vertx.core.Vertx; +import io.vertx.core.file.AsyncFile; import io.vertx.core.file.OpenOptions; import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; @@ -90,7 +88,6 @@ class RecordingsPostHandler implements RequestHandler { private static final Pattern RECORDING_FILENAME_PATTERN = Pattern.compile("([A-Za-z\\d-]*)_([A-Za-z\\d-_]*)_([\\d]*T[\\d]*Z)(\\.[\\d]+)?"); - private static final Duration READ_TIMEOUT = Duration.ofSeconds(10); private final AuthManager auth; private final FileSystem fs; @@ -98,7 +95,6 @@ class RecordingsPostHandler implements RequestHandler { private final Gson gson; private final Logger logger; private final NotificationFactory notificationFactory; - private final AtomicLong lastReadTimestamp = new AtomicLong(0); private static final String NOTIFICATION_CATEGORY = "RecordingSaved"; @@ -179,44 +175,47 @@ public void handle(RoutingContext ctx) { .resolve(UUID.randomUUID().toString()) .toString(); CompletableFuture fileUploadPath = new CompletableFuture<>(); - long timerId = - ctx.vertx() - .setPeriodic( - READ_TIMEOUT.toMillis(), - id -> { - if (System.nanoTime() - lastReadTimestamp.get() - > READ_TIMEOUT.toNanos()) { - fileUploadPath.completeExceptionally( - new TimeoutException()); - } - }); ctx.vertx() .fileSystem() .open( destinationFile, - new OpenOptions().setAppend(true).setCreateNew(true), + new OpenOptions().setCreateNew(true).setWrite(true).setAppend(true), openFile -> { if (openFile.failed()) { - ctx.fail(new HttpStatusException(500, openFile.cause())); + ctx.fail(openFile.cause()); return; } + AsyncFile as = openFile.result(); + ctx.request() + .pipeTo( + as, + pipe -> { + if (pipe.failed()) { + ctx.fail(pipe.cause()); + return; + } + }); ctx.request() - .handler( - buffer -> { - lastReadTimestamp.set(System.nanoTime()); - openFile.result().write(buffer); - }) .exceptionHandler(fileUploadPath::completeExceptionally) .endHandler( v -> { - ctx.vertx().cancelTimer(timerId); - openFile.result().close(); - fileUploadPath.complete(destinationFile); + as.flush( + flush -> { + if (flush.failed()) { + ctx.fail(flush.cause()); + return; + } + as.close( + close -> { + if (close.failed()) { + ctx.fail(close.cause()); + return; + } + fileUploadPath.complete( + destinationFile); + }); + }); }); - ctx.addEndHandler( - ar -> { - ctx.vertx().cancelTimer(timerId); - }); ctx.request().resume(); }); @@ -332,7 +331,6 @@ private void validateRecording( } event.complete(); } catch (CouldNotLoadRecordingException | IOException e) { - // FIXME need to reject the request and clean up the file here event.fail(e); } }, From 31a491432c37d73a695149936ab2cd64f33702e0 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Sat, 30 Oct 2021 00:09:01 -0400 Subject: [PATCH 16/31] ensure file-uploads dir is created --- .../web/http/api/v1/RecordingsPostBodyHandler.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java index 4a6572de27..1f5c17a38e 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java @@ -37,6 +37,8 @@ */ package io.cryostat.net.web.http.api.v1; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.Set; @@ -48,7 +50,6 @@ import io.cryostat.net.security.ResourceAction; import io.cryostat.net.web.http.AbstractAuthenticatedRequestHandler; import io.cryostat.net.web.http.api.ApiVersion; - import io.vertx.core.http.HttpMethod; import io.vertx.ext.web.RoutingContext; import io.vertx.ext.web.handler.BodyHandler; @@ -61,9 +62,14 @@ class RecordingsPostBodyHandler extends AbstractAuthenticatedRequestHandler { RecordingsPostBodyHandler( AuthManager auth, @Named(MainModule.RECORDINGS_PATH) Path recordingsPath) { super(auth); - this.bodyHandler = - BodyHandler.create( - recordingsPath.resolve("file-uploads").toAbsolutePath().toString()); + Path fileUploads = recordingsPath.resolve("file-uploads"); + this.bodyHandler = BodyHandler.create(fileUploads.toAbsolutePath().toString()); + try { + // FIXME put this somewhere more appropriate + Files.createDirectory(fileUploads); + } catch (IOException ioe) { + throw new RuntimeException(ioe); + } } @Override From 2184af915ec7bb895edaa4a31ea80013202ada9d Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 10:30:02 -0400 Subject: [PATCH 17/31] Revert "improve stream-to-file handling" This reverts commit 71e18e2dee8244ea74bb406d503e1c2485a783b6. --- .../http/api/beta/RecordingsPostHandler.java | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 669557d266..6ba797cb83 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -42,6 +42,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; import java.util.EnumSet; import java.util.Map; import java.util.Objects; @@ -50,6 +51,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -75,7 +78,6 @@ import io.vertx.core.AsyncResult; import io.vertx.core.Handler; import io.vertx.core.Vertx; -import io.vertx.core.file.AsyncFile; import io.vertx.core.file.OpenOptions; import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; @@ -88,6 +90,7 @@ class RecordingsPostHandler implements RequestHandler { private static final Pattern RECORDING_FILENAME_PATTERN = Pattern.compile("([A-Za-z\\d-]*)_([A-Za-z\\d-_]*)_([\\d]*T[\\d]*Z)(\\.[\\d]+)?"); + private static final Duration READ_TIMEOUT = Duration.ofSeconds(10); private final AuthManager auth; private final FileSystem fs; @@ -95,6 +98,7 @@ class RecordingsPostHandler implements RequestHandler { private final Gson gson; private final Logger logger; private final NotificationFactory notificationFactory; + private final AtomicLong lastReadTimestamp = new AtomicLong(0); private static final String NOTIFICATION_CATEGORY = "RecordingSaved"; @@ -175,47 +179,44 @@ public void handle(RoutingContext ctx) { .resolve(UUID.randomUUID().toString()) .toString(); CompletableFuture fileUploadPath = new CompletableFuture<>(); + long timerId = + ctx.vertx() + .setPeriodic( + READ_TIMEOUT.toMillis(), + id -> { + if (System.nanoTime() - lastReadTimestamp.get() + > READ_TIMEOUT.toNanos()) { + fileUploadPath.completeExceptionally( + new TimeoutException()); + } + }); ctx.vertx() .fileSystem() .open( destinationFile, - new OpenOptions().setCreateNew(true).setWrite(true).setAppend(true), + new OpenOptions().setAppend(true).setCreateNew(true), openFile -> { if (openFile.failed()) { - ctx.fail(openFile.cause()); + ctx.fail(new HttpStatusException(500, openFile.cause())); return; } - AsyncFile as = openFile.result(); - ctx.request() - .pipeTo( - as, - pipe -> { - if (pipe.failed()) { - ctx.fail(pipe.cause()); - return; - } - }); ctx.request() + .handler( + buffer -> { + lastReadTimestamp.set(System.nanoTime()); + openFile.result().write(buffer); + }) .exceptionHandler(fileUploadPath::completeExceptionally) .endHandler( v -> { - as.flush( - flush -> { - if (flush.failed()) { - ctx.fail(flush.cause()); - return; - } - as.close( - close -> { - if (close.failed()) { - ctx.fail(close.cause()); - return; - } - fileUploadPath.complete( - destinationFile); - }); - }); + ctx.vertx().cancelTimer(timerId); + openFile.result().close(); + fileUploadPath.complete(destinationFile); }); + ctx.addEndHandler( + ar -> { + ctx.vertx().cancelTimer(timerId); + }); ctx.request().resume(); }); @@ -331,6 +332,7 @@ private void validateRecording( } event.complete(); } catch (CouldNotLoadRecordingException | IOException e) { + // FIXME need to reject the request and clean up the file here event.fail(e); } }, From 63ef3819ce0d13720c6c0e0a72e00d8c677dd971 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 10:33:02 -0400 Subject: [PATCH 18/31] replace System.nanoTime with Clock.getMonotonicTime --- .../http/api/beta/RecordingsPostHandler.java | 33 ++++++++++--------- .../api/v1/RecordingsPostBodyHandler.java | 1 + 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 6ba797cb83..6904e6c862 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -65,10 +65,10 @@ import io.cryostat.MainModule; import io.cryostat.core.log.Logger; +import io.cryostat.core.sys.Clock; import io.cryostat.core.sys.FileSystem; import io.cryostat.messaging.notifications.NotificationFactory; import io.cryostat.net.AuthManager; -import io.cryostat.net.HttpServer; import io.cryostat.net.security.ResourceAction; import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.net.web.http.RequestHandler; @@ -90,33 +90,34 @@ class RecordingsPostHandler implements RequestHandler { private static final Pattern RECORDING_FILENAME_PATTERN = Pattern.compile("([A-Za-z\\d-]*)_([A-Za-z\\d-_]*)_([\\d]*T[\\d]*Z)(\\.[\\d]+)?"); + private static final String NOTIFICATION_CATEGORY = "RecordingSaved"; private static final Duration READ_TIMEOUT = Duration.ofSeconds(10); private final AuthManager auth; - private final FileSystem fs; - private final Path savedRecordingsPath; private final Gson gson; - private final Logger logger; + private final Path savedRecordingsPath; + private final FileSystem fs; + private final Clock clock; private final NotificationFactory notificationFactory; + private final Logger logger; private final AtomicLong lastReadTimestamp = new AtomicLong(0); - private static final String NOTIFICATION_CATEGORY = "RecordingSaved"; - @Inject RecordingsPostHandler( AuthManager auth, - HttpServer httpServer, - FileSystem fs, - @Named(MainModule.RECORDINGS_PATH) Path savedRecordingsPath, Gson gson, - Logger logger, - NotificationFactory notificationFactory) { + @Named(MainModule.RECORDINGS_PATH) Path savedRecordingsPath, + FileSystem fs, + Clock clock, + NotificationFactory notificationFactory, + Logger logger) { this.auth = auth; - this.fs = fs; - this.savedRecordingsPath = savedRecordingsPath; this.gson = gson; - this.logger = logger; + this.savedRecordingsPath = savedRecordingsPath; + this.fs = fs; + this.clock = clock; this.notificationFactory = notificationFactory; + this.logger = logger; } @Override @@ -184,7 +185,7 @@ public void handle(RoutingContext ctx) { .setPeriodic( READ_TIMEOUT.toMillis(), id -> { - if (System.nanoTime() - lastReadTimestamp.get() + if (clock.getMonotonicTime() - lastReadTimestamp.get() > READ_TIMEOUT.toNanos()) { fileUploadPath.completeExceptionally( new TimeoutException()); @@ -203,8 +204,8 @@ public void handle(RoutingContext ctx) { ctx.request() .handler( buffer -> { - lastReadTimestamp.set(System.nanoTime()); openFile.result().write(buffer); + lastReadTimestamp.set(clock.getMonotonicTime()); }) .exceptionHandler(fileUploadPath::completeExceptionally) .endHandler( diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java index 1f5c17a38e..23b67d1b60 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java @@ -50,6 +50,7 @@ import io.cryostat.net.security.ResourceAction; import io.cryostat.net.web.http.AbstractAuthenticatedRequestHandler; import io.cryostat.net.web.http.api.ApiVersion; + import io.vertx.core.http.HttpMethod; import io.vertx.ext.web.RoutingContext; import io.vertx.ext.web.handler.BodyHandler; From 83a3decd532e7cea8dc16126c5e9d958b48739db Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 10:42:05 -0400 Subject: [PATCH 19/31] replace makeAsyncResult methods with FutureFactory --- .../web/http/api/beta/HttpApiBetaModule.java | 11 +++ .../http/api/beta/RecordingsPostHandler.java | 68 ++++--------------- 2 files changed, 24 insertions(+), 55 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java b/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java index 0a521a3c12..b009dbc35e 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java @@ -37,11 +37,16 @@ */ package io.cryostat.net.web.http.api.beta; +import javax.inject.Singleton; + import io.cryostat.net.web.http.RequestHandler; import dagger.Binds; import dagger.Module; +import dagger.Provides; import dagger.multibindings.IntoSet; +import io.vertx.core.impl.FutureFactoryImpl; +import io.vertx.core.spi.FutureFactory; @Module public abstract class HttpApiBetaModule { @@ -52,4 +57,10 @@ public abstract class HttpApiBetaModule { @Binds @IntoSet abstract RequestHandler bindRecordingsPostHandler(RecordingsPostHandler handler); + + @Provides + @Singleton + static FutureFactory provideFutureFactory() { + return new FutureFactoryImpl(); + } } diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 6904e6c862..4f36b74d1c 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -82,6 +82,7 @@ import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.spi.FutureFactory; import io.vertx.ext.web.RoutingContext; import io.vertx.ext.web.handler.impl.HttpStatusException; import org.apache.commons.lang3.StringUtils; @@ -98,6 +99,7 @@ class RecordingsPostHandler implements RequestHandler { private final Path savedRecordingsPath; private final FileSystem fs; private final Clock clock; + private final FutureFactory futureFactory; private final NotificationFactory notificationFactory; private final Logger logger; private final AtomicLong lastReadTimestamp = new AtomicLong(0); @@ -109,6 +111,7 @@ class RecordingsPostHandler implements RequestHandler { @Named(MainModule.RECORDINGS_PATH) Path savedRecordingsPath, FileSystem fs, Clock clock, + FutureFactory futureFactory, NotificationFactory notificationFactory, Logger logger) { this.auth = auth; @@ -116,6 +119,7 @@ class RecordingsPostHandler implements RequestHandler { this.savedRecordingsPath = savedRecordingsPath; this.fs = fs; this.clock = clock; + this.futureFactory = futureFactory; this.notificationFactory = notificationFactory; this.logger = logger; } @@ -349,11 +353,11 @@ private void validateRecording( } vertx.fileSystem().deleteBlocking(recordingFile); - handler.handle(makeFailedAsyncResult(t)); + handler.handle(futureFactory.failedFuture(t)); return; } - handler.handle(makeAsyncResult(null)); + handler.handle(futureFactory.succeededFuture()); }); } @@ -368,7 +372,7 @@ private void saveRecording( // are also differentiated by second-resolution timestamp if (counter >= Byte.MAX_VALUE) { handler.handle( - makeFailedAsyncResult( + futureFactory.failedFuture( new IOException( "Recording could not be saved. File already exists and rename attempts were exhausted."))); return; @@ -381,7 +385,7 @@ private void saveRecording( try { Files.createDirectory(specificRecordingsPath); } catch (IOException e) { - handler.handle(makeFailedAsyncResult(e)); + handler.handle(futureFactory.failedFuture(e)); return; } } @@ -391,7 +395,7 @@ private void saveRecording( specificRecordingsPath.resolve(filename).toString(), (res) -> { if (res.failed()) { - handler.handle(makeFailedAsyncResult(res.cause())); + handler.handle(futureFactory.failedFuture(res.cause())); return; } @@ -414,60 +418,14 @@ private void saveRecording( (res2) -> { if (res2.failed()) { handler.handle( - makeFailedAsyncResult(res2.cause())); + futureFactory.failedFuture( + res2.cause())); return; } - handler.handle(makeAsyncResult(filename)); + handler.handle( + futureFactory.succeededFuture(filename)); }); }); } - - private AsyncResult makeAsyncResult(T result) { - return new AsyncResult<>() { - @Override - public T result() { - return result; - } - - @Override - public Throwable cause() { - return null; - } - - @Override - public boolean succeeded() { - return true; - } - - @Override - public boolean failed() { - return false; - } - }; - } - - private AsyncResult makeFailedAsyncResult(Throwable cause) { - return new AsyncResult<>() { - @Override - public T result() { - return null; - } - - @Override - public Throwable cause() { - return cause; - } - - @Override - public boolean succeeded() { - return false; - } - - @Override - public boolean failed() { - return true; - } - }; - } } From 5370ce9508d86ba42fef61ab26b4910b348ec0e5 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 10:45:52 -0400 Subject: [PATCH 20/31] fixup! ensure file-uploads dir is created --- .../net/web/http/api/v1/RecordingsPostBodyHandler.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java index 23b67d1b60..09ca256512 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostBodyHandler.java @@ -46,6 +46,7 @@ import javax.inject.Named; import io.cryostat.MainModule; +import io.cryostat.core.sys.FileSystem; import io.cryostat.net.AuthManager; import io.cryostat.net.security.ResourceAction; import io.cryostat.net.web.http.AbstractAuthenticatedRequestHandler; @@ -61,13 +62,17 @@ class RecordingsPostBodyHandler extends AbstractAuthenticatedRequestHandler { @Inject RecordingsPostBodyHandler( - AuthManager auth, @Named(MainModule.RECORDINGS_PATH) Path recordingsPath) { + AuthManager auth, + @Named(MainModule.RECORDINGS_PATH) Path recordingsPath, + FileSystem fs) { super(auth); Path fileUploads = recordingsPath.resolve("file-uploads"); this.bodyHandler = BodyHandler.create(fileUploads.toAbsolutePath().toString()); try { // FIXME put this somewhere more appropriate - Files.createDirectory(fileUploads); + if (!fs.isDirectory(fileUploads)) { + Files.createDirectories(fileUploads); + } } catch (IOException ioe) { throw new RuntimeException(ioe); } From bf0bd0aee8ad22841b3469da491b58d15d30942c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 11:20:51 -0400 Subject: [PATCH 21/31] reorder request header and param validations --- .../http/api/beta/RecordingsPostHandler.java | 66 +++++++++++-------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 4f36b74d1c..8f164a77c0 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -45,7 +45,6 @@ import java.time.Duration; import java.util.EnumSet; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; @@ -59,6 +58,9 @@ import javax.inject.Inject; import javax.inject.Named; +import com.google.gson.Gson; + +import org.apache.commons.lang3.StringUtils; import org.openjdk.jmc.flightrecorder.CouldNotLoadRecordingException; import org.openjdk.jmc.flightrecorder.internal.FlightRecordingLoader; import org.openjdk.jmc.flightrecorder.internal.InvalidJfrFileException; @@ -73,8 +75,6 @@ import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.net.web.http.RequestHandler; import io.cryostat.net.web.http.api.ApiVersion; - -import com.google.gson.Gson; import io.vertx.core.AsyncResult; import io.vertx.core.Handler; import io.vertx.core.Vertx; @@ -85,7 +85,6 @@ import io.vertx.core.spi.FutureFactory; import io.vertx.ext.web.RoutingContext; import io.vertx.ext.web.handler.impl.HttpStatusException; -import org.apache.commons.lang3.StringUtils; class RecordingsPostHandler implements RequestHandler { @@ -162,6 +161,42 @@ private Future validateRequestAuthorization(HttpServerRequest req) thro @Override public void handle(RoutingContext ctx) { ctx.request().pause(); + + try { + boolean permissionGranted = validateRequestAuthorization(ctx.request()).get(); + if (!permissionGranted) { + ctx.fail(401, new HttpStatusException(401, "HTTP Authorization Failure")); + return; + } + } catch (Exception e) { + ctx.fail(e); + return; + } + + if (!fs.isDirectory(savedRecordingsPath)) { + ctx.fail(503, new HttpStatusException(503, "Recording saving not available")); + return; + } + + if (!"100-continue".equals(ctx.request().getHeader(HttpHeaders.EXPECT))) { + ctx.fail(400, new HttpStatusException(400, "Expect:100-continue header is required")); + return; + } + + if (!HttpMimeType.OCTET_STREAM.mime().equals( + ctx.request().getHeader(HttpHeaders.CONTENT_TYPE))) { + ctx.fail(400); + return; + } + + if (ctx.request().getHeader(HttpHeaders.CONTENT_LENGTH) == null + || Long.parseLong(ctx.request().getHeader(HttpHeaders.CONTENT_LENGTH)) < 1) { + ctx.fail( + 400, + new HttpStatusException(400, "Content-Length header must be a positive value")); + return; + } + String desiredSaveName = ctx.pathParam("recordingName"); if (StringUtils.isBlank(desiredSaveName)) { ctx.fail(400, new HttpStatusException(400, "Recording name must not be empty")); @@ -225,29 +260,6 @@ public void handle(RoutingContext ctx) { ctx.request().resume(); }); - try { - boolean permissionGranted = validateRequestAuthorization(ctx.request()).get(); - if (!permissionGranted) { - ctx.fail(401, new HttpStatusException(401, "HTTP Authorization Failure")); - return; - } - } catch (Exception e) { - ctx.fail(e); - return; - } - - if (!fs.isDirectory(savedRecordingsPath)) { - ctx.fail(503, new HttpStatusException(503, "Recording saving not available")); - return; - } - - if (!Objects.equals( - HttpMimeType.OCTET_STREAM.mime(), - ctx.request().getHeader(HttpHeaders.CONTENT_TYPE))) { - ctx.fail(400); - return; - } - ctx.vertx() .executeBlocking( event -> { From 4282c9677a9fcb03580a0b7b72e16f872686cf6b Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 11:25:32 -0400 Subject: [PATCH 22/31] Update doc with required headers --- HTTP_API.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/HTTP_API.md b/HTTP_API.md index 7829b554e1..be5ebde26b 100644 --- a/HTTP_API.md +++ b/HTTP_API.md @@ -1733,7 +1733,11 @@ The handler-specific descriptions below describe how each handler populates the ###### synopsis Uploads a recording to the archives as a binary file upload rather than a multipart-form-data upload. This is faster than the API V1 multipart form - upload and should be preferred, particularly for larger JFR files. + upload and should be preferred when possible, particularly for larger JFR + files. However, this requires the client to send the `Expect: 100-continue` + header, which common web browser implementations do not do. The + `Content-Length` header must be included and the `Content-Type` must be + `application/octet-stream`. ###### request `POST /api/beta/recordings/:recordingName` From 2ffd56df1d4f5f2e858a661b17b82609e56c6733 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 11:27:54 -0400 Subject: [PATCH 23/31] apply spotless formatting --- .../net/web/http/api/beta/RecordingsPostHandler.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java index 8f164a77c0..b37e28c0dc 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java @@ -58,9 +58,6 @@ import javax.inject.Inject; import javax.inject.Named; -import com.google.gson.Gson; - -import org.apache.commons.lang3.StringUtils; import org.openjdk.jmc.flightrecorder.CouldNotLoadRecordingException; import org.openjdk.jmc.flightrecorder.internal.FlightRecordingLoader; import org.openjdk.jmc.flightrecorder.internal.InvalidJfrFileException; @@ -75,6 +72,8 @@ import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.net.web.http.RequestHandler; import io.cryostat.net.web.http.api.ApiVersion; + +import com.google.gson.Gson; import io.vertx.core.AsyncResult; import io.vertx.core.Handler; import io.vertx.core.Vertx; @@ -85,6 +84,7 @@ import io.vertx.core.spi.FutureFactory; import io.vertx.ext.web.RoutingContext; import io.vertx.ext.web.handler.impl.HttpStatusException; +import org.apache.commons.lang3.StringUtils; class RecordingsPostHandler implements RequestHandler { @@ -183,8 +183,9 @@ public void handle(RoutingContext ctx) { return; } - if (!HttpMimeType.OCTET_STREAM.mime().equals( - ctx.request().getHeader(HttpHeaders.CONTENT_TYPE))) { + if (!HttpMimeType.OCTET_STREAM + .mime() + .equals(ctx.request().getHeader(HttpHeaders.CONTENT_TYPE))) { ctx.fail(400); return; } From 6c52c80b650a9374e3f8ecc57b87b92963cf416e Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 11:37:54 -0400 Subject: [PATCH 24/31] correct broken unit tests --- .../net/web/http/api/v1/TemplatesPostHandlerTest.java | 4 +--- .../net/web/http/generic/CorsEnablingHandlerTest.java | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandlerTest.java index 36bc6f4d96..f2ac7d763c 100644 --- a/src/test/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/v1/TemplatesPostHandlerTest.java @@ -121,6 +121,7 @@ void shouldHaveExpectedRequiredPermissions() { void shouldThrowIfWriteFails() throws Exception { RoutingContext ctx = Mockito.mock(RoutingContext.class); FileUpload upload = Mockito.mock(FileUpload.class); + Mockito.when(upload.name()).thenReturn("template"); Mockito.when(ctx.fileUploads()).thenReturn(Set.of(upload)); Mockito.when(upload.uploadedFileName()).thenReturn("/file-uploads/abcd-1234"); @@ -170,9 +171,6 @@ void shouldThrowIfTemplateUploadNameInvalid() throws Exception { Path uploadPath = Mockito.mock(Path.class); Mockito.when(fs.pathOf("/file-uploads/abcd-1234")).thenReturn(uploadPath); - InputStream stream = Mockito.mock(InputStream.class); - Mockito.when(fs.newInputStream(Mockito.any())).thenReturn(stream); - HttpStatusException ex = Assertions.assertThrows( HttpStatusException.class, () -> handler.handleAuthenticated(ctx)); diff --git a/src/test/java/io/cryostat/net/web/http/generic/CorsEnablingHandlerTest.java b/src/test/java/io/cryostat/net/web/http/generic/CorsEnablingHandlerTest.java index a825f7014a..4ac4625bb2 100644 --- a/src/test/java/io/cryostat/net/web/http/generic/CorsEnablingHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/generic/CorsEnablingHandlerTest.java @@ -164,7 +164,7 @@ void shouldRespondOKToOPTIONSWithAcceptedMethod(HttpMethod method) { Mockito.verify(res) .putHeader( HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS, - "Authorization,X-JMX-Authorization"); + "Authorization,X-JMX-Authorization,content-type"); Mockito.verify(res).setStatusCode(200); Mockito.verify(res).end(); Mockito.verifyNoMoreInteractions(res); From ace65d6569a51be263861f89680d6055c874bfb3 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 11:58:47 -0400 Subject: [PATCH 25/31] remove debug logging --- .../web/http/api/v1/RecordingsPostHandler.java | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java index e0682ed62b..4eec6619cc 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java @@ -42,11 +42,9 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.time.Instant; import java.util.EnumSet; import java.util.Map; import java.util.Set; -import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -149,8 +147,6 @@ public boolean isOrdered() { @Override public void handleAuthenticated(RoutingContext ctx) throws Exception { - long start = System.currentTimeMillis(); - logger.info("RecordingsPostHandler started at {}", Instant.ofEpochMilli(start)); if (!fs.isDirectory(savedRecordingsPath)) { throw new HttpStatusException(503, "Recording saving not available"); } @@ -215,12 +211,6 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { HttpMimeType.JSON.mime()) .end(gson.toJson(Map.of("name", res2.result()))); - logger.info("Recording saved as {}", res2.result()); - - logger.info( - "RecordingsPostHandler completed successfully in {}ms", - System.currentTimeMillis() - start); - notificationFactory .createBuilder() .metaCategory(NOTIFICATION_CATEGORY) @@ -234,10 +224,6 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { private void validateRecording(String recordingFile, Handler> handler) { vertx.executeBlocking( event -> { - // FIXME remove logging and timing. Add a custom JFR event for the Cryostat - // event profile? - long start = System.nanoTime(); - logger.info("Starting JFR validation for {}", recordingFile); try { // try loading chunk info to see if it's a valid file try (var is = new BufferedInputStream(new FileInputStream(recordingFile))) { @@ -247,12 +233,8 @@ private void validateRecording(String recordingFile, Handler> throw new InvalidJfrFileException(); } } - logger.info( - "JFR validation succeeded in {}ms", - TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)); event.complete(); } catch (CouldNotLoadRecordingException | IOException e) { - logger.info("JFR validation FAILED"); event.fail(e); } }, From d5fa677e996d9c3b4a6b7ee84061f1182ffd289a Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 11:59:16 -0400 Subject: [PATCH 26/31] delete invalid uploaded files --- .../io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java index 4eec6619cc..0ee0eed1e0 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java @@ -248,6 +248,7 @@ private void validateRecording(String recordingFile, Handler> } else { t = res.cause(); } + vertx.fileSystem().deleteBlocking(recordingFile); handler.handle(makeFailedAsyncResult(t)); return; From ddb6ffa83429f9d649bbb5aaefaa39f2a8f7979a Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 1 Nov 2021 15:23:42 -0400 Subject: [PATCH 27/31] Upgrade vertx to 3.9.9 Includes a Netty version bump which fixes a performance regression in multipart form-data decoding --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 3372406eb8..711460dbc7 100644 --- a/pom.xml +++ b/pom.xml @@ -68,7 +68,7 @@ 2.8.0 4.5.13 5.4.1 - 3.9.7 + 3.9.9 1.7.30 2.8.6 3.0.1 From 2b23c17fa600b25dd5382b415b1f2d1e4c888b2c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 2 Nov 2021 17:52:48 -0400 Subject: [PATCH 28/31] Remove POST /api/beta/recordings handler --- HTTP_API.md | 74 --- .../web/http/api/beta/HttpApiBetaModule.java | 10 - .../http/api/beta/RecordingsPostHandler.java | 444 ------------------ 3 files changed, 528 deletions(-) delete mode 100644 src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java diff --git a/HTTP_API.md b/HTTP_API.md index be5ebde26b..ce95e30d91 100644 --- a/HTTP_API.md +++ b/HTTP_API.md @@ -1725,77 +1725,3 @@ The handler-specific descriptions below describe how each handler populates the `200` - The result is the path of the saved file in the server's storage. `401` - The user does not have sufficient permissions. - -### Recordings in archive - -* #### `RecordingsPostHandler-1` - - ###### synopsis - Uploads a recording to the archives as a binary file upload rather than a - multipart-form-data upload. This is faster than the API V1 multipart form - upload and should be preferred when possible, particularly for larger JFR - files. However, this requires the client to send the `Expect: 100-continue` - header, which common web browser implementations do not do. The - `Content-Length` header must be included and the `Content-Type` must be - `application/octet-stream`. - - ###### request - `POST /api/beta/recordings/:recordingName` - - The `recordingName` is the name of the JFR file that will be written in the - archives and must be in the following format, which is the same format that - Cryostat uses for recordings it saves itself (all fields in brackets are - optional): - ``` - [$TARGET_NAME]_[$RECORDING_NAME]_[$DATE]T[$TIME]Z[.$COUNTER][.jfr] - ``` - - `TARGET_NAME` - The name of the target JVM (alphanumerics and hyphens allowed) - - `RECORDING_NAME` - The name of the recording - (alphanumerics, hyphens, and underscores allowed) - - `DATE` - The date of the recording (numbers allowed) - - `TIME` - The time of the recording (numbers allowed) - - `COUNTER` - An additional number used to avoid name collisions - (numbers allowed) - - Formally, the required format is: - ``` - ([A-Za-z\d-]*)_([A-Za-z\d-_]*)_([\d]*T[\d]*Z)(\.[\d]+)?(\.jfr)? - ``` - - ###### response - `200` - The body is `{"name":"$NAME"}`, where `$NAME` is the name of the - recording that is now saved in archive. - This name may be different from the filename of the uploaded file - for two reasons. - - First, if there is a name collision, a counter will be added to the - original filename, or the existing counter will be modified, - and the new counter will be set to the next available number, - starting from `2`. A counter of `1` will be removed if there is no - name conflict, and modified to the next available number if there is. - - And second, if the filename of the uploaded file does not include a `.jfr` - ending, one will be added. - - `400` - The recording submission is invalid. The body is an error - message. - - `401` - User authentication failed. The body is an error message. - There will be an `X-WWW-Authenticate: $SCHEME` header that indicates - the authentication scheme that is used. - - `500` - There was an unexpected error. The body is an error message. - - `503` - `CRYOSTAT_ARCHIVE_PATH` is an invalid directory. - The body is an error message. - - ###### example - ``` - $ curl -kv --progress-bar -H Content-Type:'application/octet-stream' --data-binary @vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr localhost:8181/api/beta/recordings/vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr | cat - {"name":"vertx-fib-demo-6f4775cdbf-82dvl_150mb_20211006T152006Z.jfr"} - ``` diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java b/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java index b009dbc35e..8cfb3f73a3 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java @@ -53,14 +53,4 @@ public abstract class HttpApiBetaModule { @Binds @IntoSet abstract RequestHandler bindDiscoveryGetHandler(DiscoveryGetHandler handler); - - @Binds - @IntoSet - abstract RequestHandler bindRecordingsPostHandler(RecordingsPostHandler handler); - - @Provides - @Singleton - static FutureFactory provideFutureFactory() { - return new FutureFactoryImpl(); - } } diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java deleted file mode 100644 index b37e28c0dc..0000000000 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsPostHandler.java +++ /dev/null @@ -1,444 +0,0 @@ -/* - * Copyright The Cryostat Authors - * - * The Universal Permissive License (UPL), Version 1.0 - * - * Subject to the condition set forth below, permission is hereby granted to any - * person obtaining a copy of this software, associated documentation and/or data - * (collectively the "Software"), free of charge and under any and all copyright - * rights in the Software, and any and all patent rights owned or freely - * licensable by each licensor hereunder covering either (i) the unmodified - * Software as contributed to or provided by such licensor, or (ii) the Larger - * Works (as defined below), to deal in both - * - * (a) the Software, and - * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if - * one is included with the Software (each a "Larger Work" to which the Software - * is contributed by such licensors), - * - * without restriction, including without limitation the rights to copy, create - * derivative works of, display, perform, and distribute the Software and make, - * use, sell, offer for sale, import, export, have made, and have sold the - * Software and the Larger Work(s), and to sublicense the foregoing rights on - * either these or other terms. - * - * This license is subject to the following condition: - * The above copyright notice and either this complete permission notice or at - * a minimum a reference to the UPL must be included in all copies or - * substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ -package io.cryostat.net.web.http.api.beta; - -import java.io.BufferedInputStream; -import java.io.FileInputStream; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.time.Duration; -import java.util.EnumSet; -import java.util.Map; -import java.util.Set; -import java.util.UUID; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; -import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicLong; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import javax.inject.Inject; -import javax.inject.Named; - -import org.openjdk.jmc.flightrecorder.CouldNotLoadRecordingException; -import org.openjdk.jmc.flightrecorder.internal.FlightRecordingLoader; -import org.openjdk.jmc.flightrecorder.internal.InvalidJfrFileException; - -import io.cryostat.MainModule; -import io.cryostat.core.log.Logger; -import io.cryostat.core.sys.Clock; -import io.cryostat.core.sys.FileSystem; -import io.cryostat.messaging.notifications.NotificationFactory; -import io.cryostat.net.AuthManager; -import io.cryostat.net.security.ResourceAction; -import io.cryostat.net.web.http.HttpMimeType; -import io.cryostat.net.web.http.RequestHandler; -import io.cryostat.net.web.http.api.ApiVersion; - -import com.google.gson.Gson; -import io.vertx.core.AsyncResult; -import io.vertx.core.Handler; -import io.vertx.core.Vertx; -import io.vertx.core.file.OpenOptions; -import io.vertx.core.http.HttpHeaders; -import io.vertx.core.http.HttpMethod; -import io.vertx.core.http.HttpServerRequest; -import io.vertx.core.spi.FutureFactory; -import io.vertx.ext.web.RoutingContext; -import io.vertx.ext.web.handler.impl.HttpStatusException; -import org.apache.commons.lang3.StringUtils; - -class RecordingsPostHandler implements RequestHandler { - - private static final Pattern RECORDING_FILENAME_PATTERN = - Pattern.compile("([A-Za-z\\d-]*)_([A-Za-z\\d-_]*)_([\\d]*T[\\d]*Z)(\\.[\\d]+)?"); - private static final String NOTIFICATION_CATEGORY = "RecordingSaved"; - private static final Duration READ_TIMEOUT = Duration.ofSeconds(10); - - private final AuthManager auth; - private final Gson gson; - private final Path savedRecordingsPath; - private final FileSystem fs; - private final Clock clock; - private final FutureFactory futureFactory; - private final NotificationFactory notificationFactory; - private final Logger logger; - private final AtomicLong lastReadTimestamp = new AtomicLong(0); - - @Inject - RecordingsPostHandler( - AuthManager auth, - Gson gson, - @Named(MainModule.RECORDINGS_PATH) Path savedRecordingsPath, - FileSystem fs, - Clock clock, - FutureFactory futureFactory, - NotificationFactory notificationFactory, - Logger logger) { - this.auth = auth; - this.gson = gson; - this.savedRecordingsPath = savedRecordingsPath; - this.fs = fs; - this.clock = clock; - this.futureFactory = futureFactory; - this.notificationFactory = notificationFactory; - this.logger = logger; - } - - @Override - public ApiVersion apiVersion() { - return ApiVersion.BETA; - } - - @Override - public HttpMethod httpMethod() { - return HttpMethod.POST; - } - - @Override - public Set resourceActions() { - return EnumSet.of(ResourceAction.CREATE_RECORDING); - } - - @Override - public String path() { - return basePath() + "recordings/:recordingName"; - } - - @Override - public boolean isAsync() { - return false; - } - - @Override - public boolean isOrdered() { - return false; - } - - private Future validateRequestAuthorization(HttpServerRequest req) throws Exception { - return auth.validateHttpHeader( - () -> req.getHeader(HttpHeaders.AUTHORIZATION), resourceActions()); - } - - @Override - public void handle(RoutingContext ctx) { - ctx.request().pause(); - - try { - boolean permissionGranted = validateRequestAuthorization(ctx.request()).get(); - if (!permissionGranted) { - ctx.fail(401, new HttpStatusException(401, "HTTP Authorization Failure")); - return; - } - } catch (Exception e) { - ctx.fail(e); - return; - } - - if (!fs.isDirectory(savedRecordingsPath)) { - ctx.fail(503, new HttpStatusException(503, "Recording saving not available")); - return; - } - - if (!"100-continue".equals(ctx.request().getHeader(HttpHeaders.EXPECT))) { - ctx.fail(400, new HttpStatusException(400, "Expect:100-continue header is required")); - return; - } - - if (!HttpMimeType.OCTET_STREAM - .mime() - .equals(ctx.request().getHeader(HttpHeaders.CONTENT_TYPE))) { - ctx.fail(400); - return; - } - - if (ctx.request().getHeader(HttpHeaders.CONTENT_LENGTH) == null - || Long.parseLong(ctx.request().getHeader(HttpHeaders.CONTENT_LENGTH)) < 1) { - ctx.fail( - 400, - new HttpStatusException(400, "Content-Length header must be a positive value")); - return; - } - - String desiredSaveName = ctx.pathParam("recordingName"); - if (StringUtils.isBlank(desiredSaveName)) { - ctx.fail(400, new HttpStatusException(400, "Recording name must not be empty")); - return; - } - - if (desiredSaveName.endsWith(".jfr")) { - desiredSaveName = desiredSaveName.substring(0, desiredSaveName.length() - 4); - } - Matcher m = RECORDING_FILENAME_PATTERN.matcher(desiredSaveName); - if (!m.matches()) { - ctx.fail(400, new HttpStatusException(400, "Incorrect recording file name pattern")); - return; - } - - String destinationFile = - savedRecordingsPath - .toAbsolutePath() - .resolve("file-uploads") - .resolve(UUID.randomUUID().toString()) - .toString(); - CompletableFuture fileUploadPath = new CompletableFuture<>(); - long timerId = - ctx.vertx() - .setPeriodic( - READ_TIMEOUT.toMillis(), - id -> { - if (clock.getMonotonicTime() - lastReadTimestamp.get() - > READ_TIMEOUT.toNanos()) { - fileUploadPath.completeExceptionally( - new TimeoutException()); - } - }); - ctx.vertx() - .fileSystem() - .open( - destinationFile, - new OpenOptions().setAppend(true).setCreateNew(true), - openFile -> { - if (openFile.failed()) { - ctx.fail(new HttpStatusException(500, openFile.cause())); - return; - } - ctx.request() - .handler( - buffer -> { - openFile.result().write(buffer); - lastReadTimestamp.set(clock.getMonotonicTime()); - }) - .exceptionHandler(fileUploadPath::completeExceptionally) - .endHandler( - v -> { - ctx.vertx().cancelTimer(timerId); - openFile.result().close(); - fileUploadPath.complete(destinationFile); - }); - ctx.addEndHandler( - ar -> { - ctx.vertx().cancelTimer(timerId); - }); - ctx.request().resume(); - }); - - ctx.vertx() - .executeBlocking( - event -> { - try { - event.complete(fileUploadPath.get()); - } catch (ExecutionException | InterruptedException e) { - event.fail(e); - } - }, - ar -> { - if (ar.failed()) { - ctx.vertx().fileSystem().deleteBlocking(destinationFile); - ctx.fail(ar.cause()); - return; - } - String upload = (String) ar.result(); - - String targetName = m.group(1); - String recordingName = m.group(2); - String timestamp = m.group(3); - int count = - m.group(4) == null || m.group(4).isEmpty() - ? 0 - : Integer.parseInt(m.group(4).substring(1)); - - final String subdirectoryName = "unlabelled"; - final String basename = - String.format("%s_%s_%s", targetName, recordingName, timestamp); - validateRecording( - ctx.vertx(), - upload, - res -> { - if (res.failed()) { - ctx.fail(400, res.cause()); - return; - } - saveRecording( - ctx.vertx(), - subdirectoryName, - basename, - upload, - count, - res2 -> { - if (res2.failed()) { - ctx.fail(res2.cause()); - return; - } - - ctx.response() - .putHeader( - HttpHeaders.CONTENT_TYPE, - HttpMimeType.JSON.mime()) - .end( - gson.toJson( - Map.of( - "name", - res2 - .result()))); - - notificationFactory - .createBuilder() - .metaCategory(NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message( - Map.of( - "recording", - res2.result())) - .build() - .send(); - }); - }); - }); - } - - private void validateRecording( - Vertx vertx, String recordingFile, Handler> handler) { - vertx.executeBlocking( - event -> { - try { - // try loading chunk info to see if it's a valid file - try (var is = new BufferedInputStream(new FileInputStream(recordingFile))) { - var supplier = FlightRecordingLoader.createChunkSupplier(is); - var chunks = FlightRecordingLoader.readChunkInfo(supplier); - if (chunks.size() < 1) { - event.fail(new InvalidJfrFileException()); - } - } - event.complete(); - } catch (CouldNotLoadRecordingException | IOException e) { - // FIXME need to reject the request and clean up the file here - event.fail(e); - } - }, - res -> { - if (res.failed()) { - Throwable t; - if (res.cause() instanceof CouldNotLoadRecordingException) { - t = - new HttpStatusException( - 400, "Not a valid JFR recording file", res.cause()); - } else { - t = res.cause(); - } - vertx.fileSystem().deleteBlocking(recordingFile); - - handler.handle(futureFactory.failedFuture(t)); - return; - } - - handler.handle(futureFactory.succeededFuture()); - }); - } - - private void saveRecording( - Vertx vertx, - String subdirectoryName, - String basename, - String tmpFile, - int counter, - Handler> handler) { - // TODO byte-sized rename limit is arbitrary. Probably plenty since recordings - // are also differentiated by second-resolution timestamp - if (counter >= Byte.MAX_VALUE) { - handler.handle( - futureFactory.failedFuture( - new IOException( - "Recording could not be saved. File already exists and rename attempts were exhausted."))); - return; - } - - String filename = counter > 1 ? basename + "." + counter + ".jfr" : basename + ".jfr"; - Path specificRecordingsPath = savedRecordingsPath.resolve(subdirectoryName); - - if (!fs.exists(specificRecordingsPath)) { - try { - Files.createDirectory(specificRecordingsPath); - } catch (IOException e) { - handler.handle(futureFactory.failedFuture(e)); - return; - } - } - - vertx.fileSystem() - .exists( - specificRecordingsPath.resolve(filename).toString(), - (res) -> { - if (res.failed()) { - handler.handle(futureFactory.failedFuture(res.cause())); - return; - } - - if (res.result()) { - saveRecording( - vertx, - subdirectoryName, - basename, - tmpFile, - counter + 1, - handler); - return; - } - - // verified no name clash at this time - vertx.fileSystem() - .move( - tmpFile, - specificRecordingsPath.resolve(filename).toString(), - (res2) -> { - if (res2.failed()) { - handler.handle( - futureFactory.failedFuture( - res2.cause())); - return; - } - - handler.handle( - futureFactory.succeededFuture(filename)); - }); - }); - } -} From d96dc474a80e5beac06cb7aa410b263cfb9367c7 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 2 Nov 2021 17:54:36 -0400 Subject: [PATCH 29/31] apply spotless formatting --- .../io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java b/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java index 8cfb3f73a3..0d39ccc928 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/HttpApiBetaModule.java @@ -37,16 +37,11 @@ */ package io.cryostat.net.web.http.api.beta; -import javax.inject.Singleton; - import io.cryostat.net.web.http.RequestHandler; import dagger.Binds; import dagger.Module; -import dagger.Provides; import dagger.multibindings.IntoSet; -import io.vertx.core.impl.FutureFactoryImpl; -import io.vertx.core.spi.FutureFactory; @Module public abstract class HttpApiBetaModule { From fc034f848a39c2793b5237346f8f19fe43db1038 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 2 Nov 2021 18:00:00 -0400 Subject: [PATCH 30/31] remove unneeded CORS change after removal of Beta API handler --- .../io/cryostat/net/web/http/generic/CorsEnablingHandler.java | 2 -- .../cryostat/net/web/http/generic/CorsEnablingHandlerTest.java | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/generic/CorsEnablingHandler.java b/src/main/java/io/cryostat/net/web/http/generic/CorsEnablingHandler.java index 626c56c1bc..bc7ff58dd4 100644 --- a/src/main/java/io/cryostat/net/web/http/generic/CorsEnablingHandler.java +++ b/src/main/java/io/cryostat/net/web/http/generic/CorsEnablingHandler.java @@ -48,7 +48,6 @@ import io.cryostat.net.web.http.RequestHandler; import io.cryostat.net.web.http.api.ApiVersion; -import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; import io.vertx.ext.web.RoutingContext; import io.vertx.ext.web.handler.CorsHandler; @@ -66,7 +65,6 @@ class CorsEnablingHandler implements RequestHandler { CorsHandler.create(getOrigin()) .allowedHeader("Authorization") .allowedHeader(AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER) - .allowedHeader(HttpHeaders.CONTENT_TYPE.toString()) .allowedMethod(HttpMethod.GET) .allowedMethod(HttpMethod.POST) .allowedMethod(HttpMethod.PATCH) diff --git a/src/test/java/io/cryostat/net/web/http/generic/CorsEnablingHandlerTest.java b/src/test/java/io/cryostat/net/web/http/generic/CorsEnablingHandlerTest.java index 4ac4625bb2..a825f7014a 100644 --- a/src/test/java/io/cryostat/net/web/http/generic/CorsEnablingHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/generic/CorsEnablingHandlerTest.java @@ -164,7 +164,7 @@ void shouldRespondOKToOPTIONSWithAcceptedMethod(HttpMethod method) { Mockito.verify(res) .putHeader( HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS, - "Authorization,X-JMX-Authorization,content-type"); + "Authorization,X-JMX-Authorization"); Mockito.verify(res).setStatusCode(200); Mockito.verify(res).end(); Mockito.verifyNoMoreInteractions(res); From 543d51f1df4c02213d383c01100efab0f7c79945 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 3 Nov 2021 13:53:45 -0400 Subject: [PATCH 31/31] doc(httpapi): remove reference to Beta POST /api/beta/recordings/:recordingName handler --- HTTP_API.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/HTTP_API.md b/HTTP_API.md index ce95e30d91..e33c4d7b81 100644 --- a/HTTP_API.md +++ b/HTTP_API.md @@ -1706,8 +1706,6 @@ The handler-specific descriptions below describe how each handler populates the | ------------------------------------------------------------------------- | --------------------------------------------------------------------------------| | **Miscellaneous** | | | View targets in overall deployment environment | [`DiscoveryGetHandler`](#DiscoveryGetHandler) | -| **Recordings in archive** | | -| Upload a recording to archive | [`RecordingsPostHandler`](#RecordingsPostHandler-1) | ### Miscellaneous