From 3dfc0d1fa0872ebcfd7b19405f39da60abdea098 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Sat, 27 Aug 2022 00:09:59 +0200 Subject: [PATCH 1/3] Move HttpException and its subclasses to common HTTP module --- .../FileStorage.java.multipart.mustache | 4 +-- .../common/http}/BadRequestException.java | 4 +-- .../helidon/common/http}/HttpException.java | 4 +-- .../common/http}/NotFoundException.java | 4 +-- docs-internal/http-exceptions.md | 36 +++++++++++++++++++ .../examples/media/multipart/FileStorage.java | 4 +-- .../backend/TranslatorBackendService.java | 2 +- .../frontend/TranslatorFrontendService.java | 2 +- .../webserver/examples/basics/Main.java | 2 +- .../webserver/examples/comments/Main.java | 2 +- .../webserver/jersey/JerseySupport.java | 2 +- .../ClassPathContentHandler.java | 2 +- .../FileBasedContentHandler.java | 6 ++-- .../staticcontent/StaticContentHandler.java | 12 +++---- .../StaticContentHandlerTest.java | 2 +- .../webserver/testsupport/TestClientTest.java | 4 +-- .../reactive/webserver/ForwardingHandler.java | 1 + .../reactive/webserver/RequestRouting.java | 2 ++ .../helidon/reactive/webserver/Response.java | 1 + .../reactive/webserver/ServerRequest.java | 1 + .../reactive/webserver/BytesReuseTest.java | 1 + .../webserver/BytesReuseV2ApiTest.java | 1 + 22 files changed, 68 insertions(+), 31 deletions(-) rename {reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver => common/http/src/main/java/io/helidon/common/http}/BadRequestException.java (94%) rename {reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver => common/http/src/main/java/io/helidon/common/http}/HttpException.java (96%) rename {reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver => common/http/src/main/java/io/helidon/common/http}/NotFoundException.java (94%) create mode 100644 docs-internal/http-exceptions.md diff --git a/archetypes/helidon/src/main/archetype/se/custom/files/src/main/java/__pkg__/FileStorage.java.multipart.mustache b/archetypes/helidon/src/main/archetype/se/custom/files/src/main/java/__pkg__/FileStorage.java.multipart.mustache index 07398574ff8..1dc22c06e02 100644 --- a/archetypes/helidon/src/main/archetype/se/custom/files/src/main/java/__pkg__/FileStorage.java.multipart.mustache +++ b/archetypes/helidon/src/main/archetype/se/custom/files/src/main/java/__pkg__/FileStorage.java.multipart.mustache @@ -1,8 +1,8 @@ package {{package}}; -import io.helidon.reactive.webserver.BadRequestException; -import io.helidon.reactive.webserver.NotFoundException; +import io.helidon.common.http.BadRequestException; +import io.helidon.common.http.NotFoundException; import java.io.IOException; import java.io.UncheckedIOException; diff --git a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/BadRequestException.java b/common/http/src/main/java/io/helidon/common/http/BadRequestException.java similarity index 94% rename from reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/BadRequestException.java rename to common/http/src/main/java/io/helidon/common/http/BadRequestException.java index a4c6ba6f901..e857612aaff 100644 --- a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/BadRequestException.java +++ b/common/http/src/main/java/io/helidon/common/http/BadRequestException.java @@ -14,9 +14,7 @@ * limitations under the License. */ -package io.helidon.reactive.webserver; - -import io.helidon.common.http.Http; +package io.helidon.common.http; /** * A runtime exception indicating a {@link Http.Status#BAD_REQUEST_400 bad request}. diff --git a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/HttpException.java b/common/http/src/main/java/io/helidon/common/http/HttpException.java similarity index 96% rename from reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/HttpException.java rename to common/http/src/main/java/io/helidon/common/http/HttpException.java index 065e1a5bbce..b8bd32bc363 100644 --- a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/HttpException.java +++ b/common/http/src/main/java/io/helidon/common/http/HttpException.java @@ -14,9 +14,7 @@ * limitations under the License. */ -package io.helidon.reactive.webserver; - -import io.helidon.common.http.Http; +package io.helidon.common.http; /** * Runtime exception for applications. diff --git a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/NotFoundException.java b/common/http/src/main/java/io/helidon/common/http/NotFoundException.java similarity index 94% rename from reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/NotFoundException.java rename to common/http/src/main/java/io/helidon/common/http/NotFoundException.java index aba466ef26b..9767f9dc2ca 100644 --- a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/NotFoundException.java +++ b/common/http/src/main/java/io/helidon/common/http/NotFoundException.java @@ -14,9 +14,7 @@ * limitations under the License. */ -package io.helidon.reactive.webserver; - -import io.helidon.common.http.Http; +package io.helidon.common.http; /** * A runtime exception indicating a {@link Http.Status#NOT_FOUND_404 not found}. diff --git a/docs-internal/http-exceptions.md b/docs-internal/http-exceptions.md new file mode 100644 index 00000000000..2a66dfbfac0 --- /dev/null +++ b/docs-internal/http-exceptions.md @@ -0,0 +1,36 @@ +HTTP Exception Handling +--- + +# WebServer + +The following rules are valid for both Helidon Níma and Helidon Reactive (SE). + +The request may fail in three locations: +1. In Helidon server processing code (internal, unexpected error caused by a bug). Such a case will be logged into the server log in + Severe/Error log level and the connection will be terminated. +2. In request processing code, before ServerRequest and ServerResponse are created. Such errors (such as when request cannot be parsed) + are handled by `DirectHandler`, and the server provides as much information as possible to the handler. +3. Exceptions thrown from user routing code, and most server exceptions thrown after `ServerRequest` and `ServerResponse` are created is handled using error handlers of routing. The exception is `BadRequestException` which is always handled by `DirectHandler` (we may discover lazily that a header does not adhere to specification and thrown this exception) + +The exception class hierarchy that is used (these exceptions should not be caught by user code, unless you want to handle everything on your own): +- `RequestException` - used for case 2 - this is the only exception (final class) used when we need to explicitly throw an exception + to be handled by Helidon server code in a `DirectHandler`. If an exception with "known" status is thrown and `ServerRequest` and + `ServerResponse` are already available, such an exception will be converted to `HttpException` and handled appropriately + specific case is `CloseConnectionException` - this exception can be thrown to terminate current connection +- `HttpException` - used for case 3 - this is the top level exception class for exceptions that provide a status code and message, and + that can be handled by error handlers. If Error handles do not handle them, they will create a response with the expected error + code, and will send the exception message as entity + + +# WebClient + +! we need to discuss this first! + Web client will use `HttpException` and its subclasses (such as `NotFoundException`) in case the entity is read directly +(e.g. the user skips `WebClientResponse`, and uses `as(Class)` to read entity) and the status code returned does not provide +an expected entity (such as any error) + +# HttpException reference +The following HTTP status codes have a specific exception class: + +- 404 - `NotFoundException` +- 400 - `BadRequestException` - ALWAYS handled by `DirectHandler` \ No newline at end of file diff --git a/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileStorage.java b/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileStorage.java index 80964cb7c76..8825395f665 100644 --- a/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileStorage.java +++ b/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileStorage.java @@ -21,8 +21,8 @@ import java.nio.file.Path; import java.util.stream.Stream; -import io.helidon.reactive.webserver.BadRequestException; -import io.helidon.reactive.webserver.NotFoundException; +import io.helidon.common.http.BadRequestException; +import io.helidon.common.http.NotFoundException; /** * Simple bean to managed a directory based storage. diff --git a/examples/translator-app/backend/src/main/java/io/helidon/examples/translator/backend/TranslatorBackendService.java b/examples/translator-app/backend/src/main/java/io/helidon/examples/translator/backend/TranslatorBackendService.java index 035abe75bcb..2786761bb11 100644 --- a/examples/translator-app/backend/src/main/java/io/helidon/examples/translator/backend/TranslatorBackendService.java +++ b/examples/translator-app/backend/src/main/java/io/helidon/examples/translator/backend/TranslatorBackendService.java @@ -18,7 +18,7 @@ import java.util.HashMap; import java.util.Map; -import io.helidon.reactive.webserver.BadRequestException; +import io.helidon.common.http.BadRequestException; import io.helidon.reactive.webserver.Routing; import io.helidon.reactive.webserver.ServerRequest; import io.helidon.reactive.webserver.ServerResponse; diff --git a/examples/translator-app/frontend/src/main/java/io/helidon/examples/translator/frontend/TranslatorFrontendService.java b/examples/translator-app/frontend/src/main/java/io/helidon/examples/translator/frontend/TranslatorFrontendService.java index 9bd7f4ce151..acbc07a40c8 100644 --- a/examples/translator-app/frontend/src/main/java/io/helidon/examples/translator/frontend/TranslatorFrontendService.java +++ b/examples/translator-app/frontend/src/main/java/io/helidon/examples/translator/frontend/TranslatorFrontendService.java @@ -18,7 +18,7 @@ import java.util.logging.Level; import java.util.logging.Logger; -import io.helidon.reactive.webserver.BadRequestException; +import io.helidon.common.http.BadRequestException; import io.helidon.reactive.webserver.Routing; import io.helidon.reactive.webserver.ServerRequest; import io.helidon.reactive.webserver.ServerResponse; diff --git a/examples/webserver/basics/src/main/java/io/helidon/reactive/webserver/examples/basics/Main.java b/examples/webserver/basics/src/main/java/io/helidon/reactive/webserver/examples/basics/Main.java index a8d7558ed36..1a353191763 100644 --- a/examples/webserver/basics/src/main/java/io/helidon/reactive/webserver/examples/basics/Main.java +++ b/examples/webserver/basics/src/main/java/io/helidon/reactive/webserver/examples/basics/Main.java @@ -23,13 +23,13 @@ import io.helidon.common.http.DataChunk; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.http.HttpMediaType; import io.helidon.common.media.type.MediaTypes; import io.helidon.reactive.media.common.MediaContext; import io.helidon.reactive.media.common.MessageBodyReader; import io.helidon.reactive.media.jsonp.JsonpSupport; import io.helidon.reactive.webserver.Handler; -import io.helidon.reactive.webserver.HttpException; import io.helidon.reactive.webserver.RequestPredicate; import io.helidon.reactive.webserver.Routing; import io.helidon.reactive.webserver.WebServer; diff --git a/examples/webserver/comment-aas/src/main/java/io/helidon/reactive/webserver/examples/comments/Main.java b/examples/webserver/comment-aas/src/main/java/io/helidon/reactive/webserver/examples/comments/Main.java index 290607c23ab..7cd9c0bbc0e 100644 --- a/examples/webserver/comment-aas/src/main/java/io/helidon/reactive/webserver/examples/comments/Main.java +++ b/examples/webserver/comment-aas/src/main/java/io/helidon/reactive/webserver/examples/comments/Main.java @@ -20,8 +20,8 @@ import java.util.concurrent.CompletionException; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.config.Config; -import io.helidon.reactive.webserver.HttpException; import io.helidon.reactive.webserver.Routing; import io.helidon.reactive.webserver.WebServer; diff --git a/reactive/webserver/jersey/src/main/java/io/helidon/reactive/webserver/jersey/JerseySupport.java b/reactive/webserver/jersey/src/main/java/io/helidon/reactive/webserver/jersey/JerseySupport.java index 7286bfbc151..c96dbce6506 100644 --- a/reactive/webserver/jersey/src/main/java/io/helidon/reactive/webserver/jersey/JerseySupport.java +++ b/reactive/webserver/jersey/src/main/java/io/helidon/reactive/webserver/jersey/JerseySupport.java @@ -37,10 +37,10 @@ import io.helidon.common.context.Context; import io.helidon.common.context.Contexts; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.config.Config; import io.helidon.config.ConfigValue; import io.helidon.reactive.webserver.Handler; -import io.helidon.reactive.webserver.HttpException; import io.helidon.reactive.webserver.KeyPerformanceIndicatorSupport; import io.helidon.reactive.webserver.Routing; import io.helidon.reactive.webserver.ServerRequest; diff --git a/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/ClassPathContentHandler.java b/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/ClassPathContentHandler.java index af23817934b..71d983eaf80 100644 --- a/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/ClassPathContentHandler.java +++ b/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/ClassPathContentHandler.java @@ -37,8 +37,8 @@ import io.helidon.common.http.DataChunk; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.reactive.IoMulti; -import io.helidon.reactive.webserver.HttpException; import io.helidon.reactive.webserver.ServerRequest; import io.helidon.reactive.webserver.ServerResponse; diff --git a/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/FileBasedContentHandler.java b/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/FileBasedContentHandler.java index 74797b5db76..16a12e26e88 100644 --- a/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/FileBasedContentHandler.java +++ b/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/FileBasedContentHandler.java @@ -27,12 +27,12 @@ import java.util.logging.Logger; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.http.HttpMediaType; import io.helidon.common.media.type.MediaType; import io.helidon.common.media.type.MediaTypes; import io.helidon.reactive.media.common.DefaultMediaSupport; import io.helidon.reactive.media.common.MessageBodyWriter; -import io.helidon.reactive.webserver.HttpException; import io.helidon.reactive.webserver.RequestHeaders; import io.helidon.reactive.webserver.ResponseHeaders; import io.helidon.reactive.webserver.ServerRequest; @@ -61,12 +61,12 @@ static String fileName(Path path) { } /** - * Find welcome file in provided directory or throw not found {@link io.helidon.reactive.webserver.HttpException}. + * Find welcome file in provided directory or throw not found {@link io.helidon.common.http.HttpException}. * * @param directory a directory to find in * @param name welcome file name * @return a path of the welcome file - * @throws io.helidon.reactive.webserver.HttpException if welcome file doesn't exists + * @throws io.helidon.common.http.HttpException if welcome file doesn't exists */ private static Path resolveWelcomeFile(Path directory, String name) { throwNotFoundIf(name == null || name.isEmpty()); diff --git a/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/StaticContentHandler.java b/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/StaticContentHandler.java index 18376b13357..a760198fd14 100644 --- a/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/StaticContentHandler.java +++ b/reactive/webserver/static-content/src/main/java/io/helidon/reactive/webserver/staticcontent/StaticContentHandler.java @@ -27,7 +27,7 @@ import java.util.logging.Logger; import io.helidon.common.http.Http; -import io.helidon.reactive.webserver.HttpException; +import io.helidon.common.http.HttpException; import io.helidon.reactive.webserver.RequestHeaders; import io.helidon.reactive.webserver.ResponseHeaders; import io.helidon.reactive.webserver.Routing; @@ -128,7 +128,7 @@ void handle(Http.Method method, ServerRequest request, ServerResponse response) * @param response an HTTP response * @return {@code true} only if static content was found and processed. * @throws java.io.IOException if resource is not acceptable - * @throws io.helidon.reactive.webserver.HttpException if some known WEB error + * @throws io.helidon.common.http.HttpException if some known WEB error */ abstract boolean doHandle(Http.Method method, String requestedPath, ServerRequest request, ServerResponse response) throws IOException, URISyntaxException; @@ -140,7 +140,7 @@ abstract boolean doHandle(Http.Method method, String requestedPath, ServerReques * @param etag the proposed ETag. If {@code null} then method returns false * @param requestHeaders an HTTP request headers * @param responseHeaders an HTTP response headers - * @throws io.helidon.reactive.webserver.HttpException if ETag is checked + * @throws io.helidon.common.http.HttpException if ETag is checked */ static void processEtag(String etag, RequestHeaders requestHeaders, ResponseHeaders responseHeaders) { if (etag == null || etag.isEmpty()) { @@ -198,7 +198,7 @@ private static String unquoteETag(String etag) { * @param modified the last modification instance. If {@code null} then method just returns {@code false}. * @param requestHeaders an HTTP request headers * @param responseHeaders an HTTP response headers - * @throws io.helidon.reactive.webserver.HttpException if (un)modify since header is checked + * @throws io.helidon.common.http.HttpException if (un)modify since header is checked */ static void processModifyHeaders(Instant modified, RequestHeaders requestHeaders, ResponseHeaders responseHeaders) { if (modified == null) { @@ -223,10 +223,10 @@ static void processModifyHeaders(Instant modified, RequestHeaders requestHeaders } /** - * If provided {@code condition} is {@code true} then throws not found {@link io.helidon.reactive.webserver.HttpException}. + * If provided {@code condition} is {@code true} then throws not found {@link io.helidon.common.http.HttpException}. * * @param condition if condition is true then throws an exception otherwise not - * @throws io.helidon.reactive.webserver.HttpException if {@code condition} parameter is {@code true}. + * @throws io.helidon.common.http.HttpException if {@code condition} parameter is {@code true}. */ static void throwNotFoundIf(boolean condition) { if (condition) { diff --git a/reactive/webserver/static-content/src/test/java/io/helidon/reactive/webserver/staticcontent/StaticContentHandlerTest.java b/reactive/webserver/static-content/src/test/java/io/helidon/reactive/webserver/staticcontent/StaticContentHandlerTest.java index cfdf6585b4a..a70fc11869d 100644 --- a/reactive/webserver/static-content/src/test/java/io/helidon/reactive/webserver/staticcontent/StaticContentHandlerTest.java +++ b/reactive/webserver/static-content/src/test/java/io/helidon/reactive/webserver/staticcontent/StaticContentHandlerTest.java @@ -27,7 +27,7 @@ import java.util.concurrent.atomic.AtomicInteger; import io.helidon.common.http.Http; -import io.helidon.reactive.webserver.HttpException; +import io.helidon.common.http.HttpException; import io.helidon.reactive.webserver.RequestHeaders; import io.helidon.reactive.webserver.ResponseHeaders; import io.helidon.reactive.webserver.ServerRequest; diff --git a/reactive/webserver/test-support/src/test/java/io/helidon/reactive/webserver/testsupport/TestClientTest.java b/reactive/webserver/test-support/src/test/java/io/helidon/reactive/webserver/testsupport/TestClientTest.java index 61670929619..00c9fc67938 100644 --- a/reactive/webserver/test-support/src/test/java/io/helidon/reactive/webserver/testsupport/TestClientTest.java +++ b/reactive/webserver/test-support/src/test/java/io/helidon/reactive/webserver/testsupport/TestClientTest.java @@ -20,8 +20,8 @@ import java.util.concurrent.TimeUnit; import io.helidon.common.http.Http; -import io.helidon.reactive.webserver.HttpException; -import io.helidon.reactive.webserver.NotFoundException; +import io.helidon.common.http.HttpException; +import io.helidon.common.http.NotFoundException; import io.helidon.reactive.webserver.Routing; import org.junit.jupiter.api.Test; diff --git a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/ForwardingHandler.java b/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/ForwardingHandler.java index 0eadab6cf2d..55a379a06e5 100644 --- a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/ForwardingHandler.java +++ b/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/ForwardingHandler.java @@ -32,6 +32,7 @@ import io.helidon.common.context.Context; import io.helidon.common.context.Contexts; +import io.helidon.common.http.BadRequestException; import io.helidon.common.http.DirectHandler; import io.helidon.common.http.DirectHandler.TransportResponse; import io.helidon.common.http.HeadersServerRequest; diff --git a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/RequestRouting.java b/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/RequestRouting.java index 957cff71ee7..f38f0ed8271 100644 --- a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/RequestRouting.java +++ b/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/RequestRouting.java @@ -35,7 +35,9 @@ import io.helidon.common.context.Contexts; import io.helidon.common.http.HtmlEncoder; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.http.HttpMediaType; +import io.helidon.common.http.NotFoundException; import io.helidon.tracing.Span; import io.helidon.tracing.SpanContext; import io.helidon.tracing.Tracer; diff --git a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/Response.java b/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/Response.java index eb7558243d7..6d028d3a4ad 100644 --- a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/Response.java +++ b/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/Response.java @@ -27,6 +27,7 @@ import io.helidon.common.GenericType; import io.helidon.common.http.DataChunk; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.http.HttpMediaType; import io.helidon.common.media.type.MediaType; import io.helidon.common.reactive.Single; diff --git a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/ServerRequest.java b/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/ServerRequest.java index b967e761c42..b4b99c77af2 100644 --- a/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/ServerRequest.java +++ b/reactive/webserver/webserver/src/main/java/io/helidon/reactive/webserver/ServerRequest.java @@ -25,6 +25,7 @@ import io.helidon.common.context.Context; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.reactive.Single; import io.helidon.common.uri.UriQuery; import io.helidon.reactive.media.common.MessageBodyReadableContent; diff --git a/reactive/webserver/webserver/src/test/java/io/helidon/reactive/webserver/BytesReuseTest.java b/reactive/webserver/webserver/src/test/java/io/helidon/reactive/webserver/BytesReuseTest.java index 0c2a8977cba..c880467f3d6 100644 --- a/reactive/webserver/webserver/src/test/java/io/helidon/reactive/webserver/BytesReuseTest.java +++ b/reactive/webserver/webserver/src/test/java/io/helidon/reactive/webserver/BytesReuseTest.java @@ -27,6 +27,7 @@ import java.util.logging.Level; import java.util.logging.Logger; +import io.helidon.common.http.BadRequestException; import io.helidon.common.http.DataChunk; import io.helidon.common.http.Http; import io.helidon.common.reactive.Multi; diff --git a/reactive/webserver/webserver/src/test/java/io/helidon/reactive/webserver/BytesReuseV2ApiTest.java b/reactive/webserver/webserver/src/test/java/io/helidon/reactive/webserver/BytesReuseV2ApiTest.java index ab02c96e70a..1e1807dd46e 100644 --- a/reactive/webserver/webserver/src/test/java/io/helidon/reactive/webserver/BytesReuseV2ApiTest.java +++ b/reactive/webserver/webserver/src/test/java/io/helidon/reactive/webserver/BytesReuseV2ApiTest.java @@ -27,6 +27,7 @@ import java.util.logging.Level; import java.util.logging.Logger; +import io.helidon.common.http.BadRequestException; import io.helidon.common.http.DataChunk; import io.helidon.common.http.Http; import io.helidon.common.reactive.Multi; From 792d307ae4d40d2f1ff253d97108ca24cd10e7b0 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Sat, 27 Aug 2022 02:21:38 +0200 Subject: [PATCH 2/3] Refactor HttpException and RequestException to common HTTP module. Add ErrorHandlers (without actual implementation) to Nima. Use HttpException in Nima where appropriate --- .../io/helidon/common/http/DirectHandler.java | 4 - .../common/http/ForbiddenException.java | 42 +++++++ .../io/helidon/common/http/HttpException.java | 38 ++++++- .../common/http/InternalServerException.java | 43 +++++++ .../common/http/NotFoundException.java | 4 +- .../helidon/common/http/RequestException.java | 61 ++-------- docs-internal/http-exceptions.md | 14 +-- .../nima/http2/webserver/Http2Stream.java | 94 +++++---------- .../nima/observe/config/ConfigService.java | 10 +- .../observe/health/SingleCheckHandler.java | 10 +- .../nima/observe/info/InfoService.java | 10 +- .../helidon/nima/observe/ObserveSupport.java | 11 +- .../test/resources/logging-test.properties | 6 +- .../staticcontent/ByteRangeRequest.java | 18 +-- .../ClassPathContentHandler.java | 21 +--- .../FileBasedContentHandler.java | 22 ++-- .../staticcontent/StaticContentHandler.java | 54 +++------ .../StaticContentHandlerTest.java | 5 +- .../nima/webserver/ConnectionHandler.java | 4 +- .../nima/webserver/http/DirectHandlers.java | 3 +- ...quest.java => DirectTransportRequest.java} | 28 ++--- .../nima/webserver/http/ErrorHandlers.java | 107 ++++++++++++++++++ .../helidon/nima/webserver/http/Filter.java | 9 ++ .../helidon/nima/webserver/http/Filters.java | 49 +++++--- .../nima/webserver/http/HttpRouting.java | 83 ++++++-------- .../webserver/http/ServerResponseBase.java | 9 +- .../nima/webserver/http1/Http1Connection.java | 68 +++++------ .../nima/webserver/http1/Http1Headers.java | 8 +- .../nima/webserver/http1/Http1Prologue.java | 16 +-- .../webserver/WsUpgradeProvider.java | 8 +- 30 files changed, 470 insertions(+), 389 deletions(-) create mode 100644 common/http/src/main/java/io/helidon/common/http/ForbiddenException.java create mode 100644 common/http/src/main/java/io/helidon/common/http/InternalServerException.java rename nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpException.java => common/http/src/main/java/io/helidon/common/http/RequestException.java (76%) rename nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/{HttpSimpleRequest.java => DirectTransportRequest.java} (70%) create mode 100644 nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ErrorHandlers.java diff --git a/common/http/src/main/java/io/helidon/common/http/DirectHandler.java b/common/http/src/main/java/io/helidon/common/http/DirectHandler.java index cc2f97e762d..3db3c45f375 100644 --- a/common/http/src/main/java/io/helidon/common/http/DirectHandler.java +++ b/common/http/src/main/java/io/helidon/common/http/DirectHandler.java @@ -148,10 +148,6 @@ enum EventType { * Internal server error. */ INTERNAL_ERROR(Http.Status.INTERNAL_SERVER_ERROR_500, true), - /** - * No route was found for the request. - */ - NOT_FOUND(Http.Status.NOT_FOUND_404, true), /** * Other type, please specify expected status code. */ diff --git a/common/http/src/main/java/io/helidon/common/http/ForbiddenException.java b/common/http/src/main/java/io/helidon/common/http/ForbiddenException.java new file mode 100644 index 00000000000..2d545fda290 --- /dev/null +++ b/common/http/src/main/java/io/helidon/common/http/ForbiddenException.java @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2017, 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.common.http; + +/** + * A runtime exception indicating a {@link io.helidon.common.http.Http.Status#NOT_FOUND_404 not found}. + */ +public class ForbiddenException extends HttpException { + + /** + * Creates {@link io.helidon.common.http.ForbiddenException}. + * + * @param message the message + */ + public ForbiddenException(String message) { + super(message, Http.Status.FORBIDDEN_403, null, true); + } + + /** + * Creates {@link io.helidon.common.http.ForbiddenException}. + * + * @param message the message + * @param cause the cause of this exception + */ + public ForbiddenException(String message, Throwable cause) { + super(message, Http.Status.FORBIDDEN_403, cause, true); + } +} diff --git a/common/http/src/main/java/io/helidon/common/http/HttpException.java b/common/http/src/main/java/io/helidon/common/http/HttpException.java index b8bd32bc363..f0580bda340 100644 --- a/common/http/src/main/java/io/helidon/common/http/HttpException.java +++ b/common/http/src/main/java/io/helidon/common/http/HttpException.java @@ -26,6 +26,7 @@ public class HttpException extends RuntimeException { private final Http.Status status; + private final boolean keepAlive; /** * Creates {@link HttpException} associated with {@link Http.Status#INTERNAL_SERVER_ERROR_500}. @@ -53,11 +54,21 @@ public HttpException(String message, Throwable cause) { * @param status the http status */ public HttpException(String message, Http.Status status) { - super(message); + this(message, status, null); + } - this.status = status; + /** + * Creates {@link HttpException}. + * + * @param message the message + * @param status the http status + * @param keepAlive whether to keep the connection alive + */ + public HttpException(String message, Http.Status status, boolean keepAlive) { + this(message, status, null, keepAlive); } + /** * Creates {@link HttpException}. * @@ -66,9 +77,22 @@ public HttpException(String message, Http.Status status) { * @param cause the cause of this exception */ public HttpException(String message, Http.Status status, Throwable cause) { + this(message, status, cause, false); + } + + /** + * Creates {@link HttpException}. + * + * @param message the message + * @param status the http status + * @param cause the cause of this exception + * @param keepAlive whether to keep this connection alive + */ + public HttpException(String message, Http.Status status, Throwable cause, boolean keepAlive) { super(message, cause); this.status = status; + this.keepAlive = keepAlive; } /** @@ -79,4 +103,14 @@ public HttpException(String message, Http.Status status, Throwable cause) { public final Http.Status status() { return status; } + + /** + * Whether we should attempt to keep the connection alive (if enabled for it). + * Some exceptions may allow the connection to be further used (such as {@link io.helidon.common.http.NotFoundException}. + * + * @return whether to keep alive + */ + public boolean keepAlive() { + return keepAlive; + } } diff --git a/common/http/src/main/java/io/helidon/common/http/InternalServerException.java b/common/http/src/main/java/io/helidon/common/http/InternalServerException.java new file mode 100644 index 00000000000..e5c25b668ce --- /dev/null +++ b/common/http/src/main/java/io/helidon/common/http/InternalServerException.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2017, 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.common.http; + +/** + * A runtime exception indicating a {@link io.helidon.common.http.Http.Status#INTERNAL_SERVER_ERROR_500 internal server error}. + */ +public class InternalServerException extends HttpException { + /** + * Creates {@link io.helidon.common.http.InternalServerException}. + * + * @param message the message + * @param cause the cause of this exception + */ + public InternalServerException(String message, Throwable cause) { + super(message, Http.Status.INTERNAL_SERVER_ERROR_500, cause); + } + + /** + * Creates {@link io.helidon.common.http.InternalServerException}. + * + * @param message the message + * @param cause the cause of this exception + * @param keepAlive whether to keep the connection alive (if keep alives are enabled) + */ + public InternalServerException(String message, Throwable cause, boolean keepAlive) { + super(message, Http.Status.INTERNAL_SERVER_ERROR_500, cause, keepAlive); + } +} diff --git a/common/http/src/main/java/io/helidon/common/http/NotFoundException.java b/common/http/src/main/java/io/helidon/common/http/NotFoundException.java index 9767f9dc2ca..522ed365757 100644 --- a/common/http/src/main/java/io/helidon/common/http/NotFoundException.java +++ b/common/http/src/main/java/io/helidon/common/http/NotFoundException.java @@ -27,7 +27,7 @@ public class NotFoundException extends HttpException { * @param message the message */ public NotFoundException(String message) { - super(message, Http.Status.NOT_FOUND_404); + super(message, Http.Status.NOT_FOUND_404, null, true); } /** @@ -37,6 +37,6 @@ public NotFoundException(String message) { * @param cause the cause of this exception */ public NotFoundException(String message, Throwable cause) { - super(message, Http.Status.NOT_FOUND_404, cause); + super(message, Http.Status.NOT_FOUND_404, cause, true); } } diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpException.java b/common/http/src/main/java/io/helidon/common/http/RequestException.java similarity index 76% rename from nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpException.java rename to common/http/src/main/java/io/helidon/common/http/RequestException.java index a27e990dd54..9eed8d0ec82 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpException.java +++ b/common/http/src/main/java/io/helidon/common/http/RequestException.java @@ -14,22 +14,17 @@ * limitations under the License. */ -package io.helidon.nima.webserver.http; - -import java.util.Optional; - -import io.helidon.common.http.DirectHandler; -import io.helidon.common.http.HeadersServerResponse; -import io.helidon.common.http.Http; +package io.helidon.common.http; /** - * HTTP exception. This allows custom handlers to be used for different even types. + * Exception that will be handled by {@link io.helidon.common.http.DirectHandler}, unless server request and server response + * are already available, in which case it would be handled by appropriate error handler of routing. + * This exception is not used by clients. */ -public class HttpException extends RuntimeException { +public class RequestException extends RuntimeException { private final DirectHandler.EventType eventType; private final Http.Status status; private final DirectHandler.TransportRequest transportRequest; - private final ServerResponse fullResponse; private final boolean keepAlive; private final HeadersServerResponse responseHeaders; @@ -39,12 +34,11 @@ public class HttpException extends RuntimeException { * * @param builder builder with details to create this instance */ - protected HttpException(Builder builder) { + protected RequestException(Builder builder) { super(builder.message, builder.cause); this.eventType = builder.type; this.status = builder.status; this.transportRequest = builder.request; - this.fullResponse = builder.fullResponse; this.keepAlive = builder.keepAlive; this.responseHeaders = builder.responseHeaders; } @@ -85,16 +79,6 @@ public DirectHandler.TransportRequest request() { return transportRequest; } - /** - * Routing response (if available). This is used to correctly send response through this vehicle to handle - * post-send events. - * - * @return routing response if available - */ - public Optional fullResponse() { - return Optional.ofNullable(fullResponse); - } - /** * Whether to attempt to keep connection alive. * @@ -114,15 +98,14 @@ public HeadersServerResponse responseHeaders() { } /** - * Fluent API builder for {@link io.helidon.nima.webserver.http.HttpException}. + * Fluent API builder for {@link RequestException}. */ - public static class Builder implements io.helidon.common.Builder { + public static class Builder implements io.helidon.common.Builder { private String message; private Throwable cause; private DirectHandler.TransportRequest request; private DirectHandler.EventType type; private Http.Status status; - private ServerResponse fullResponse; private Boolean keepAlive; private final HeadersServerResponse responseHeaders = HeadersServerResponse.create(); @@ -130,7 +113,7 @@ private Builder() { } @Override - public HttpException build() { + public RequestException build() { if (message == null) { message = ""; } @@ -140,7 +123,7 @@ public HttpException build() { if (type == null) { type(DirectHandler.EventType.INTERNAL_ERROR); } - return new HttpException(this); + return new RequestException(this); } /** @@ -165,28 +148,6 @@ public Builder cause(Throwable cause) { return this; } - /** - * Routing request. - * - * @param request request to obtain information from - * @return updated builder - */ - public Builder request(ServerRequest request) { - this.request = HttpSimpleRequest.create(request.prologue(), request.headers()); - return this; - } - - /** - * Routing response to be used to handle response from direct handler. - * - * @param response response to use - * @return updated builder - */ - public Builder response(ServerResponse response) { - this.fullResponse = response; - return this; - } - /** * Transport request with as much information as is available. * @@ -231,7 +192,7 @@ public Builder status(Http.Status status) { * Override default keep alive for this exception. * * @param keepAlive whether to keep connection alive - * @return updated builderw + * @return updated builder */ public Builder setKeepAlive(boolean keepAlive) { this.keepAlive = keepAlive; diff --git a/docs-internal/http-exceptions.md b/docs-internal/http-exceptions.md index 2a66dfbfac0..ccaa292b489 100644 --- a/docs-internal/http-exceptions.md +++ b/docs-internal/http-exceptions.md @@ -3,23 +3,21 @@ HTTP Exception Handling # WebServer -The following rules are valid for both Helidon Níma and Helidon Reactive (SE). - The request may fail in three locations: 1. In Helidon server processing code (internal, unexpected error caused by a bug). Such a case will be logged into the server log in Severe/Error log level and the connection will be terminated. 2. In request processing code, before ServerRequest and ServerResponse are created. Such errors (such as when request cannot be parsed) are handled by `DirectHandler`, and the server provides as much information as possible to the handler. -3. Exceptions thrown from user routing code, and most server exceptions thrown after `ServerRequest` and `ServerResponse` are created is handled using error handlers of routing. The exception is `BadRequestException` which is always handled by `DirectHandler` (we may discover lazily that a header does not adhere to specification and thrown this exception) +3. Exceptions thrown from user routing code, and most server exceptions thrown after `ServerRequest` and `ServerResponse` are created, are handled using error handlers of routing. The exception is `BadRequestException` which is always handled by `DirectHandler` (we may discover lazily that a header does not adhere to specification and thrown this exception) The exception class hierarchy that is used (these exceptions should not be caught by user code, unless you want to handle everything on your own): - `RequestException` - used for case 2 - this is the only exception (final class) used when we need to explicitly throw an exception - to be handled by Helidon server code in a `DirectHandler`. If an exception with "known" status is thrown and `ServerRequest` and - `ServerResponse` are already available, such an exception will be converted to `HttpException` and handled appropriately - specific case is `CloseConnectionException` - this exception can be thrown to terminate current connection + to be handled by Helidon server code in a `DirectHandler`. - `HttpException` - used for case 3 - this is the top level exception class for exceptions that provide a status code and message, and that can be handled by error handlers. If Error handles do not handle them, they will create a response with the expected error code, and will send the exception message as entity +- `CloseConnectionException` - this exception can be thrown to terminate current connection +- `UncheckedIOException` - this exception is NOT handled and will cause the connection to be closed (we expect this to be problems such as socket closed etc.) # WebClient @@ -32,5 +30,7 @@ an expected entity (such as any error) # HttpException reference The following HTTP status codes have a specific exception class: +- 403 - `ForbiddenException` - 404 - `NotFoundException` -- 400 - `BadRequestException` - ALWAYS handled by `DirectHandler` \ No newline at end of file +- 400 - `BadRequestException` - ALWAYS handled by `DirectHandler` +- 500 - `InternalServerException` - the error handler will be first attempted for the cause, then for this exception \ No newline at end of file diff --git a/nima/http2/webserver/src/main/java/io/helidon/nima/http2/webserver/Http2Stream.java b/nima/http2/webserver/src/main/java/io/helidon/nima/http2/webserver/Http2Stream.java index 25506492cae..6595a7a9b67 100644 --- a/nima/http2/webserver/src/main/java/io/helidon/nima/http2/webserver/Http2Stream.java +++ b/nima/http2/webserver/src/main/java/io/helidon/nima/http2/webserver/Http2Stream.java @@ -18,7 +18,6 @@ import java.io.UncheckedIOException; import java.util.List; -import java.util.Optional; import java.util.ServiceLoader; import java.util.concurrent.ArrayBlockingQueue; @@ -30,6 +29,7 @@ import io.helidon.common.http.Http.Header; import io.helidon.common.http.Http.HeaderValue; import io.helidon.common.http.HttpPrologue; +import io.helidon.common.http.RequestException; import io.helidon.common.socket.SocketWriterException; import io.helidon.nima.http.encoding.ContentDecoder; import io.helidon.nima.http2.FlowControl; @@ -48,11 +48,10 @@ import io.helidon.nima.http2.Http2WindowUpdate; import io.helidon.nima.http2.webserver.spi.Http2SubProtocolProvider; import io.helidon.nima.http2.webserver.spi.SubProtocolResult; +import io.helidon.nima.webserver.CloseConnectionException; import io.helidon.nima.webserver.ConnectionContext; import io.helidon.nima.webserver.Router; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http.HttpRouting; -import io.helidon.nima.webserver.http.ServerResponse; /** * Server HTTP/2 stream implementation. @@ -262,21 +261,12 @@ public void run() { .setName("[" + ctx.socketId() + " " + ctx.childSocketId() + " ] - " + streamId); try { - try { - handle(); - } catch (UncheckedIOException | SocketWriterException e) { - // failed to write to socket, this happens (remote connection closed, network issues) - ctx.log(LOGGER, System.Logger.Level.TRACE, "Failed to write to socket", e); - } catch (HttpException e) { - throw e; - } catch (Throwable e) { - throw HttpException.builder() - .message("Internal error") - .type(DirectHandler.EventType.INTERNAL_ERROR) - .cause(e) - .build(); - } - } catch (HttpException e) { + handle(); + } catch (SocketWriterException e) { + throw new CloseConnectionException(e.getMessage(), e); + } catch (CloseConnectionException | UncheckedIOException e) { + throw e; + } catch (RequestException e) { DirectHandler handler = ctx.directHandlers().handler(e.eventType()); DirectHandler.TransportResponse response = handler.handle(e.request(), e.eventType(), @@ -284,37 +274,27 @@ public void run() { e.responseHeaders(), e); - Optional fullResponse = e.fullResponse(); - if (fullResponse.isPresent()) { - fullResponse.ifPresent(res -> { - res.status(response.status()); - response.headers() - .forEach(res::header); - response.entity().ifPresentOrElse(res::send, res::send); - }); + HeadersServerResponse headers = response.headers(); + byte[] message = response.entity().orElse(BufferData.EMPTY_BYTES); + if (message.length != 0) { + headers.set(HeaderValue.create(Header.CONTENT_LENGTH, String.valueOf(message.length))); + } + Http2Headers http2Headers = Http2Headers.create(headers); + if (message.length == 0) { + writer.writeHeaders(http2Headers, + streamId, + Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS | Http2Flag.END_OF_STREAM), + flowControl); } else { - HeadersServerResponse headers = response.headers(); - byte[] message = response.entity().orElse(BufferData.EMPTY_BYTES); - if (message.length != 0) { - headers.set(HeaderValue.create(Header.CONTENT_LENGTH, String.valueOf(message.length))); - } - Http2Headers http2Headers = Http2Headers.create(headers); - if (message.length == 0) { - writer.writeHeaders(http2Headers, - streamId, - Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS | Http2Flag.END_OF_STREAM), - flowControl); - } else { - Http2FrameHeader dataHeader = Http2FrameHeader.create(message.length, - Http2FrameTypes.DATA, - Http2Flag.DataFlags.create(Http2Flag.END_OF_STREAM), - streamId); - writer.writeHeaders(http2Headers, - streamId, - Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS), - new Http2FrameData(dataHeader, BufferData.create(message)), - flowControl); - } + Http2FrameHeader dataHeader = Http2FrameHeader.create(message.length, + Http2FrameTypes.DATA, + Http2Flag.DataFlags.create(Http2Flag.END_OF_STREAM), + streamId); + writer.writeHeaders(http2Headers, + streamId, + Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS), + new Http2FrameData(dataHeader, BufferData.create(message)), + flowControl); } } finally { headers = null; @@ -394,26 +374,8 @@ private void handle() { streamId, this::readEntityFromPipeline); Http2ServerResponse response = new Http2ServerResponse(ctx, request, writer, streamId, flowControl); - try { routing.route(ctx, request, response); - } catch (HttpException e) { - throw HttpException.builder() - .request(request) - .response(response) - .type(e.eventType()) - .status(e.status()) - .message(e.getMessage()) - .cause(e) - .build(); - } catch (Throwable e) { - throw HttpException.builder() - .request(request) - .response(response) - .type(DirectHandler.EventType.INTERNAL_ERROR) - .message(e.getMessage()) - .cause(e) - .build(); } finally { this.state = Http2StreamState.CLOSED; } diff --git a/nima/observe/config/src/main/java/io/helidon/nima/observe/config/ConfigService.java b/nima/observe/config/src/main/java/io/helidon/nima/observe/config/ConfigService.java index a43aea0723d..e0a318865a0 100644 --- a/nima/observe/config/src/main/java/io/helidon/nima/observe/config/ConfigService.java +++ b/nima/observe/config/src/main/java/io/helidon/nima/observe/config/ConfigService.java @@ -23,13 +23,12 @@ import java.util.Set; import java.util.regex.Pattern; -import io.helidon.common.http.DirectHandler; +import io.helidon.common.http.NotFoundException; import io.helidon.config.Config; import io.helidon.config.ConfigValue; import io.helidon.nima.Nima; import io.helidon.nima.http.media.EntityWriter; import io.helidon.nima.http.media.jsonp.JsonpMediaSupportProvider; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http.HttpRules; import io.helidon.nima.webserver.http.HttpService; import io.helidon.nima.webserver.http.ServerRequest; @@ -101,12 +100,7 @@ private void value(ServerRequest req, ServerResponse res) { json.add("value", obfuscate(name, value.get())); write(req, res, json.build()); } else { - throw HttpException.builder() - .type(DirectHandler.EventType.NOT_FOUND) - .message("Config value for key: " + name) - .request(req) - .response(res) - .build(); + throw new NotFoundException("Config value for key: " + name); } } diff --git a/nima/observe/health/src/main/java/io/helidon/nima/observe/health/SingleCheckHandler.java b/nima/observe/health/src/main/java/io/helidon/nima/observe/health/SingleCheckHandler.java index e781d410ea8..abe6a3f4641 100644 --- a/nima/observe/health/src/main/java/io/helidon/nima/observe/health/SingleCheckHandler.java +++ b/nima/observe/health/src/main/java/io/helidon/nima/observe/health/SingleCheckHandler.java @@ -21,15 +21,14 @@ import java.util.HashMap; import java.util.Map; -import io.helidon.common.http.DirectHandler; import io.helidon.common.http.HtmlEncoder; import io.helidon.common.http.Http; +import io.helidon.common.http.NotFoundException; import io.helidon.health.HealthCheck; import io.helidon.health.HealthCheckResponse; import io.helidon.nima.http.media.EntityWriter; import io.helidon.nima.http.media.jsonp.JsonpMediaSupportProvider; import io.helidon.nima.webserver.http.Handler; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http.ServerRequest; import io.helidon.nima.webserver.http.ServerResponse; @@ -54,12 +53,7 @@ public void handle(ServerRequest req, ServerResponse res) { String name = req.path().pathParameters().value("name"); HealthCheck check = checks.get(name); if (check == null) { - throw HttpException.builder() - .request(req) - .response(res) - .type(DirectHandler.EventType.NOT_FOUND) - .message("Health check " + name + " does not exist") - .build(); + throw new NotFoundException(name); } HealthCheckResponse response; diff --git a/nima/observe/info/src/main/java/io/helidon/nima/observe/info/InfoService.java b/nima/observe/info/src/main/java/io/helidon/nima/observe/info/InfoService.java index 244bda57912..c25b8e03da6 100644 --- a/nima/observe/info/src/main/java/io/helidon/nima/observe/info/InfoService.java +++ b/nima/observe/info/src/main/java/io/helidon/nima/observe/info/InfoService.java @@ -19,11 +19,10 @@ import java.util.LinkedHashMap; import java.util.Map; -import io.helidon.common.http.DirectHandler; +import io.helidon.common.http.NotFoundException; import io.helidon.config.Config; import io.helidon.nima.http.media.EntityWriter; import io.helidon.nima.http.media.jsonp.JsonpMediaSupportProvider; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http.HttpRules; import io.helidon.nima.webserver.http.HttpService; import io.helidon.nima.webserver.http.ServerRequest; @@ -61,12 +60,7 @@ private void namedInfo(ServerRequest req, ServerResponse res) { Object value = info.get(name); if (value == null) { - throw HttpException.builder() - .type(DirectHandler.EventType.NOT_FOUND) - .message("Application info value for " + name + " is not defined.") - .request(req) - .response(res) - .build(); + throw new NotFoundException("Application info value for " + name + " is not defined."); } else { JsonObjectBuilder json = JSON.createObjectBuilder() .add("name", name) diff --git a/nima/observe/observe/src/main/java/io/helidon/nima/observe/ObserveSupport.java b/nima/observe/observe/src/main/java/io/helidon/nima/observe/ObserveSupport.java index a6895ef16f7..d618afb33e7 100644 --- a/nima/observe/observe/src/main/java/io/helidon/nima/observe/ObserveSupport.java +++ b/nima/observe/observe/src/main/java/io/helidon/nima/observe/ObserveSupport.java @@ -22,13 +22,12 @@ import java.util.function.Consumer; import io.helidon.common.HelidonServiceLoader; -import io.helidon.common.http.DirectHandler; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.config.Config; import io.helidon.nima.Nima; import io.helidon.nima.observe.spi.ObserveProvider; import io.helidon.nima.webserver.cors.CorsSupport; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http.HttpRouting; /** @@ -91,13 +90,7 @@ public void accept(HttpRouting.Builder builder) { } } else { builder.get(endpoint, (req, res) -> { - throw HttpException.builder() - .type(DirectHandler.EventType.OTHER) - .status(Http.Status.SERVICE_UNAVAILABLE_503) - .message("Observe endpoint is disabled") - .request(req) - .response(res) - .build(); + throw new HttpException("Observe endpoint is disabled", Http.Status.SERVICE_UNAVAILABLE_503, true); }); } } diff --git a/nima/tests/integration/webserver/webserver/src/test/resources/logging-test.properties b/nima/tests/integration/webserver/webserver/src/test/resources/logging-test.properties index 0fe286be554..cbd14c1bc5f 100644 --- a/nima/tests/integration/webserver/webserver/src/test/resources/logging-test.properties +++ b/nima/tests/integration/webserver/webserver/src/test/resources/logging-test.properties @@ -19,7 +19,7 @@ java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter java.util.logging.SimpleFormatter.format=%1$tH:%1$tM:%1$tS %4$s %3$s %5$s%6$s%n # Global logging level. Can be overridden by specific loggers .level=INFO -io.helidon.nima.level=WARNING -io.helidon.nima.webserver.http.Http1Connection.level=SEVERE -io.helidon.nima.webserver.http.HttpRouting.level=SEVERE +io.helidon.nima.level=FINEST +io.helidon.nima.webserver.http.Http1Connection.level=FINEST +io.helidon.nima.webserver.http.HttpRouting.level=FINEST diff --git a/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/ByteRangeRequest.java b/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/ByteRangeRequest.java index 649fc0c00a4..ecbb5e94577 100644 --- a/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/ByteRangeRequest.java +++ b/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/ByteRangeRequest.java @@ -21,10 +21,10 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import io.helidon.common.http.DirectHandler; +import io.helidon.common.http.BadRequestException; import io.helidon.common.http.Http; import io.helidon.common.http.Http.Header; -import io.helidon.nima.webserver.http.HttpException; +import io.helidon.common.http.HttpException; import io.helidon.nima.webserver.http.ServerRequest; import io.helidon.nima.webserver.http.ServerResponse; @@ -62,12 +62,7 @@ static List parse(ServerRequest req, ServerResponse res, Strin parts.add(ByteRangeRequest.create(req, res, from, last, fileLength)); } if (!found) { - throw HttpException.builder() - .request(req) - .response(res) - .type(DirectHandler.EventType.BAD_REQUEST) - .message("Invalid range header") - .build(); + throw new BadRequestException("Invalid range header"); } return parts; @@ -88,12 +83,7 @@ void setContentRange(ServerResponse response) { private static ByteRangeRequest create(ServerRequest req, ServerResponse res, long offset, long last, long fileLength) { if (offset >= fileLength || last < offset) { res.header(Header.CONTENT_RANGE.withValue("*/" + fileLength)); - throw HttpException.builder() - .request(req) - .response(res) - .type(DirectHandler.EventType.BAD_REQUEST) - .status(Http.Status.REQUESTED_RANGE_NOT_SATISFIABLE_416) - .build(); + throw new HttpException("Wrong range", Http.Status.REQUESTED_RANGE_NOT_SATISFIABLE_416, true); } long length = (last - offset) + 1; diff --git a/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/ClassPathContentHandler.java b/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/ClassPathContentHandler.java index f2902c6b4d4..e013873d061 100644 --- a/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/ClassPathContentHandler.java +++ b/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/ClassPathContentHandler.java @@ -36,9 +36,8 @@ import java.util.jar.JarEntry; import java.util.jar.JarFile; -import io.helidon.common.http.DirectHandler; import io.helidon.common.http.Http; -import io.helidon.nima.webserver.http.HttpException; +import io.helidon.common.http.InternalServerException; import io.helidon.nima.webserver.http.ServerRequest; import io.helidon.nima.webserver.http.ServerResponse; @@ -69,11 +68,7 @@ class ClassPathContentHandler extends FileBasedContentHandler { try { return Files.createTempFile(prefix, suffix); } catch (IOException e) { - throw HttpException.builder() - .message("Static content processing issue") - .type(DirectHandler.EventType.INTERNAL_ERROR) - .cause(e) - .build(); + throw new InternalServerException("Failed to create temporary file", e, true); } }; } else { @@ -81,11 +76,7 @@ class ClassPathContentHandler extends FileBasedContentHandler { try { return Files.createTempFile(tmpDir, prefix, suffix); } catch (IOException e) { - throw HttpException.builder() - .message("Static content processing issue") - .type(DirectHandler.EventType.INTERNAL_ERROR) - .cause(e) - .build(); + throw new InternalServerException("Failed to create temporary file", e, true); } }; } @@ -265,11 +256,7 @@ private ExtractedJarEntry extractJarEntry(URL url) { } } } catch (IOException ioe) { - throw HttpException.builder() - .message("Cannot load JAR file!") - .type(DirectHandler.EventType.INTERNAL_ERROR) - .cause(ioe) - .build(); + throw new InternalServerException("Cannot load resource", ioe); } } diff --git a/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/FileBasedContentHandler.java b/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/FileBasedContentHandler.java index aa3a2e21346..d4434f58204 100644 --- a/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/FileBasedContentHandler.java +++ b/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/FileBasedContentHandler.java @@ -32,16 +32,16 @@ import java.util.Objects; import java.util.Optional; -import io.helidon.common.http.DirectHandler; +import io.helidon.common.http.ForbiddenException; import io.helidon.common.http.HeadersServerRequest; import io.helidon.common.http.HeadersServerResponse; import io.helidon.common.http.Http; import io.helidon.common.http.Http.Header; import io.helidon.common.http.Http.HeaderValues; +import io.helidon.common.http.HttpException; import io.helidon.common.http.HttpMediaType; import io.helidon.common.media.type.MediaType; import io.helidon.common.media.type.MediaTypes; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http.ServerRequest; import io.helidon.nima.webserver.http.ServerResponse; @@ -120,11 +120,7 @@ void sendFile(Http.Method method, // now it exists and is a file if (!Files.isRegularFile(path) || !Files.isReadable(path) || Files.isHidden(path)) { - throw HttpException.builder() - .message("File is not accessible") - .type(DirectHandler.EventType.OTHER) - .status(Http.Status.FORBIDDEN_403) - .build(); + throw new ForbiddenException("File is not accessible"); } // Caching headers support @@ -206,12 +202,12 @@ void send(ServerRequest request, ServerResponse response, Path path) throws IOEx } /** - * Find welcome file in provided directory or throw not found {@link io.helidon.nima.webserver.http.HttpException}. + * Find welcome file in provided directory or throw not found {@link io.helidon.common.http.RequestException}. * * @param directory a directory to find in * @param name welcome file name * @return a path of the welcome file - * @throws io.helidon.nima.webserver.http.HttpException if welcome file doesn't exists + * @throws io.helidon.common.http.RequestException if welcome file doesn't exists */ private static Path resolveWelcomeFile(Path directory, String name) { throwNotFoundIf(name == null || name.isEmpty()); @@ -233,11 +229,9 @@ private MediaType detectType(String fileName, HeadersServerRequest requestHeader if (requestHeaders.isAccepted(it)) { return it; } - throw HttpException.builder() - .message("Media type " + it + " is not accepted by request") - .type(DirectHandler.EventType.OTHER) - .status(Http.Status.UNSUPPORTED_MEDIA_TYPE_415) - .build(); + throw new HttpException("Media type " + it + " is not accepted by request", + Http.Status.UNSUPPORTED_MEDIA_TYPE_415, + true); }) .orElseGet(() -> { List acceptedTypes = requestHeaders.acceptedTypes(); diff --git a/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/StaticContentHandler.java b/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/StaticContentHandler.java index 82d7dfc8db8..5cc68e639d3 100644 --- a/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/StaticContentHandler.java +++ b/nima/webserver/static-content/src/main/java/io/helidon/nima/webserver/staticcontent/StaticContentHandler.java @@ -25,13 +25,15 @@ import java.util.Optional; import java.util.function.Function; -import io.helidon.common.http.DirectHandler; import io.helidon.common.http.HeadersServerRequest; import io.helidon.common.http.HeadersServerResponse; import io.helidon.common.http.Http; import io.helidon.common.http.Http.Header; +import io.helidon.common.http.HttpException; +import io.helidon.common.http.InternalServerException; +import io.helidon.common.http.NotFoundException; +import io.helidon.common.http.RequestException; import io.helidon.common.uri.UriQuery; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http.HttpRules; import io.helidon.nima.webserver.http.PathMatchers; import io.helidon.nima.webserver.http.ServerRequest; @@ -59,7 +61,7 @@ abstract class StaticContentHandler implements StaticContentSupport { * @param etag the proposed ETag. If {@code null} then method returns false * @param requestHeaders an HTTP request headers * @param responseHeaders an HTTP response headers - * @throws io.helidon.nima.webserver.http.HttpException if ETag is checked + * @throws io.helidon.common.http.RequestException if ETag is checked */ static void processEtag(String etag, HeadersServerRequest requestHeaders, HeadersServerResponse responseHeaders) { if (etag == null || etag.isEmpty()) { @@ -74,11 +76,8 @@ static void processEtag(String etag, HeadersServerRequest requestHeaders, Header for (String ifNoneMatch : ifNoneMatches) { ifNoneMatch = unquoteETag(ifNoneMatch); if ("*".equals(ifNoneMatch) || ifNoneMatch.equals(etag)) { - throw HttpException.builder() - .message("Accepted by If-None-Match header!") - .type(DirectHandler.EventType.OTHER) - .status(Http.Status.NOT_MODIFIED_304) - .build(); + // using exception to handle normal flow (same as in reactive static content) + throw new HttpException("Accepted by If-None-Match header", Http.Status.NOT_MODIFIED_304, true); } } } @@ -96,11 +95,7 @@ static void processEtag(String etag, HeadersServerRequest requestHeaders, Header } } if (!ifMatchChecked) { - throw HttpException.builder() - .message("Not accepted by If-Match header!") - .type(DirectHandler.EventType.OTHER) - .status(Http.Status.PRECONDITION_FAILED_412) - .build(); + throw new HttpException("Not accepted by If-Match header", Http.Status.PRECONDITION_FAILED_412, true); } } } @@ -113,7 +108,7 @@ static void processEtag(String etag, HeadersServerRequest requestHeaders, Header * @param modified the last modification instance. If {@code null} then method just returns {@code false}. * @param requestHeaders an HTTP request headers * @param responseHeaders an HTTP response headers - * @throws io.helidon.nima.webserver.http.HttpException if (un)modify since header is checked + * @throws io.helidon.common.http.RequestException if (un)modify since header is checked */ static void processModifyHeaders(Instant modified, HeadersServerRequest requestHeaders, @@ -128,37 +123,26 @@ static void processModifyHeaders(Instant modified, .ifModifiedSince() .map(ChronoZonedDateTime::toInstant); if (ifModSince.isPresent() && !ifModSince.get().isBefore(modified)) { - throw HttpException.builder() - .message("Not valid for If-Modified-Since header!") - .type(DirectHandler.EventType.OTHER) - .status(Http.Status.NOT_MODIFIED_304) - .build(); + throw new HttpException("Not valid for If-Modified-Since header", Http.Status.NOT_MODIFIED_304, true); } // If-Unmodified-Since Optional ifUnmodSince = requestHeaders .ifUnmodifiedSince() .map(ChronoZonedDateTime::toInstant); if (ifUnmodSince.isPresent() && ifUnmodSince.get().isBefore(modified)) { - throw HttpException.builder() - .message("Not valid for If-Unmodified-Since header!") - .type(DirectHandler.EventType.OTHER) - .status(Http.Status.PRECONDITION_FAILED_412) - .build(); + throw new HttpException("Not valid for If-Unmodified-Since header", Http.Status.PRECONDITION_FAILED_412, true); } } /** - * If provided {@code condition} is {@code true} then throws not found {@link io.helidon.nima.webserver.http.HttpException}. + * If provided {@code condition} is {@code true} then throws not found {@link io.helidon.common.http.RequestException}. * * @param condition if condition is true then throws an exception otherwise not - * @throws io.helidon.nima.webserver.http.HttpException if {@code condition} parameter is {@code true}. + * @throws io.helidon.common.http.RequestException if {@code condition} parameter is {@code true}. */ static void throwNotFoundIf(boolean condition) { if (condition) { - throw HttpException.builder() - .message("Static content not found!") - .type(DirectHandler.EventType.NOT_FOUND) - .build(); + throw new NotFoundException("Static content not found"); } } @@ -231,7 +215,7 @@ void handle(ServerRequest request, ServerResponse response) { if (!doHandle(method, requestPath, request, response)) { response.next(); } - } catch (HttpException httpException) { + } catch (RequestException httpException) { if (httpException.status().code() == Http.Status.NOT_FOUND_404.code()) { // Prefer to next() before NOT_FOUND response.next(); @@ -240,11 +224,7 @@ void handle(ServerRequest request, ServerResponse response) { } } catch (Exception e) { LOGGER.log(Level.TRACE, "Failed to access static resource", e); - throw HttpException.builder() - .message("Cannot access static resource!") - .type(DirectHandler.EventType.INTERNAL_ERROR) - .cause(e) - .build(); + throw new InternalServerException("Cannot access static resource", e); } } @@ -257,7 +237,7 @@ void handle(ServerRequest request, ServerResponse response) { * @param response an HTTP response * @return {@code true} only if static content was found and processed. * @throws java.io.IOException if resource is not acceptable - * @throws io.helidon.nima.webserver.http.HttpException if some known WEB error + * @throws io.helidon.common.http.RequestException if some known WEB error */ abstract boolean doHandle(Http.Method method, String requestedPath, ServerRequest request, ServerResponse response) throws IOException, URISyntaxException; diff --git a/nima/webserver/static-content/src/test/java/io/helidon/nima/webserver/staticcontent/StaticContentHandlerTest.java b/nima/webserver/static-content/src/test/java/io/helidon/nima/webserver/staticcontent/StaticContentHandlerTest.java index e47722a3d61..bb26666378e 100644 --- a/nima/webserver/static-content/src/test/java/io/helidon/nima/webserver/staticcontent/StaticContentHandlerTest.java +++ b/nima/webserver/static-content/src/test/java/io/helidon/nima/webserver/staticcontent/StaticContentHandlerTest.java @@ -27,12 +27,13 @@ import io.helidon.common.http.HeadersServerRequest; import io.helidon.common.http.HeadersServerResponse; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.http.HttpPrologue; +import io.helidon.common.http.RequestException; import io.helidon.common.parameters.Parameters; import io.helidon.common.uri.UriFragment; import io.helidon.common.uri.UriPath; import io.helidon.common.uri.UriQuery; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http.RoutedPath; import io.helidon.nima.webserver.http.ServerRequest; import io.helidon.nima.webserver.http.ServerResponse; @@ -210,7 +211,7 @@ private static void assertHttpException(Runnable runnable, Http.Status status) { throw new AssertionError("Expected HttpException was not thrown!"); } catch (HttpException he) { if (status != null && status.code() != he.status().code()) { - throw new AssertionError("Unexpected status in HttpException. " + throw new AssertionError("Unexpected status in RequestException. " + "(Expected: " + status.code() + ", Actual: " + status.code() + ")"); } } diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/ConnectionHandler.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/ConnectionHandler.java index cddd1578dc2..c122faa12f0 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/ConnectionHandler.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/ConnectionHandler.java @@ -23,12 +23,12 @@ import io.helidon.common.buffers.BufferData; import io.helidon.common.buffers.DataReader; +import io.helidon.common.http.RequestException; import io.helidon.common.socket.HelidonSocket; import io.helidon.common.socket.SocketWriter; import io.helidon.nima.http.encoding.ContentEncodingContext; import io.helidon.nima.http.media.MediaContext; import io.helidon.nima.webserver.http.DirectHandlers; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.spi.ServerConnection; import io.helidon.nima.webserver.spi.ServerConnectionProvider; @@ -121,7 +121,7 @@ public final void run() { } finally { writer.close(); } - } catch (HttpException e) { + } catch (RequestException e) { ctx.log(LOGGER, WARNING, "escaped HTTP exception", e); } catch (CloseConnectionException e) { // end of request stream - safe to close the connection, as it was requested by our client diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/DirectHandlers.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/DirectHandlers.java index 31f74793869..784fbd39829 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/DirectHandlers.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/DirectHandlers.java @@ -22,6 +22,7 @@ import io.helidon.common.http.DirectHandler; import io.helidon.common.http.DirectHandler.EventType; import io.helidon.common.http.Http; +import io.helidon.common.http.RequestException; import io.helidon.nima.webserver.CloseConnectionException; import static java.lang.System.Logger.Level.WARNING; @@ -64,7 +65,7 @@ public DirectHandler handler(EventType eventType) { * @param httpException exception to handle * @param res response */ - public void handle(HttpException httpException, ServerResponse res) { + public void handle(RequestException httpException, ServerResponse res) { DirectHandler.TransportResponse response = handler(httpException.eventType()).handle( httpException.request(), httpException.eventType(), diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpSimpleRequest.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/DirectTransportRequest.java similarity index 70% rename from nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpSimpleRequest.java rename to nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/DirectTransportRequest.java index 3bb70391fa6..4e95b44fbbb 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpSimpleRequest.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/DirectTransportRequest.java @@ -23,18 +23,18 @@ import io.helidon.common.http.HttpPrologue; /** - * Simple request to use with {@link HttpException}. + * Simple request to use with {@link io.helidon.common.http.RequestException}. */ -public class HttpSimpleRequest implements DirectHandler.TransportRequest { +public class DirectTransportRequest implements DirectHandler.TransportRequest { private final String version; private final String method; private final String path; private final HeadersServerRequest headers; - private HttpSimpleRequest(String version, - String method, - String path, - HeadersServerRequest headers) { + private DirectTransportRequest(String version, + String method, + String path, + HeadersServerRequest headers) { this.version = version; this.method = method; this.path = path; @@ -52,10 +52,10 @@ private HttpSimpleRequest(String version, public static DirectHandler.TransportRequest create(String protocolAndVersion, String method, String path) { - return new HttpSimpleRequest(protocolAndVersion, - method, - path, - HeadersServerRequest.create(HeadersWritable.create())); + return new DirectTransportRequest(protocolAndVersion, + method, + path, + HeadersServerRequest.create(HeadersWritable.create())); } /** @@ -66,10 +66,10 @@ public static DirectHandler.TransportRequest create(String protocolAndVersion, * @return a new simple request */ public static DirectHandler.TransportRequest create(HttpPrologue prologue, Headers headers) { - return new HttpSimpleRequest(prologue.protocol() + "/" + prologue.protocolVersion(), - prologue.method().text(), - prologue.uriPath().rawPathNoParams(), - HeadersServerRequest.create(headers)); + return new DirectTransportRequest(prologue.protocol() + "/" + prologue.protocolVersion(), + prologue.method().text(), + prologue.uriPath().rawPathNoParams(), + HeadersServerRequest.create(headers)); } @Override diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ErrorHandlers.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ErrorHandlers.java new file mode 100644 index 00000000000..a485e68348a --- /dev/null +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ErrorHandlers.java @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.nima.webserver.http; + +import java.io.UncheckedIOException; + +import io.helidon.common.http.BadRequestException; +import io.helidon.common.http.DirectHandler; +import io.helidon.common.http.HttpException; +import io.helidon.common.http.InternalServerException; +import io.helidon.common.http.RequestException; +import io.helidon.nima.webserver.CloseConnectionException; +import io.helidon.nima.webserver.ConnectionContext; + +/** + * Http routing Error handlers. + */ +public class ErrorHandlers { + ErrorHandlers() { + } + + /** + * Run a task and handle the errors, if any. Uses error handlers configured on routing. + * Correctly handles all expected (and unexpected) exceptions that can happen in filters and routes. + * + * @param ctx connection context + * @param request HTTP server request + * @param response HTTP server response + * @param task task to execute + */ + public void runWithErrorHandling(ConnectionContext ctx, ServerRequest request, ServerResponse response, Runnable task) { + try { + task.run(); + } catch (CloseConnectionException | UncheckedIOException e) { + // these errors must "bubble up" + throw e; + } catch (RequestException e) { + handleRequestException(ctx, response, e); + } catch (BadRequestException e) { + // bad request exception MUST be handled by direct handlers + handleRequestException(ctx, response, RequestException.builder() + .message(e.getMessage()) + .cause(e) + .type(DirectHandler.EventType.BAD_REQUEST) + .status(e.status()) + .setKeepAlive(e.keepAlive()) + .build()); + } catch (InternalServerException e) { + // this is the place error handling must be done + // check if error handler exists for cause - if so, use it + if (hasErrorHandler(e.getCause())) { + handleError(ctx, request, response, e.getCause()); + } else { + handleError(ctx, request, response, e); + } + } catch (HttpException e) { + handleError(ctx, request, response, e); + } catch (RuntimeException e) { + handleError(ctx, request, response, e); + } + } + + private void handleRequestException(ConnectionContext ctx, ServerResponse response, RequestException e) { + ctx.directHandlers().handle(e, response); + } + + private boolean hasErrorHandler(Throwable cause) { + // TODO needs implementation (separate issue) + return true; + } + + private void handleError(ConnectionContext ctx, ServerRequest request, ServerResponse response, Throwable e) { + // to be handled by error handler + handleRequestException(ctx, response, RequestException.builder() + .cause(e) + .type(DirectHandler.EventType.INTERNAL_ERROR) + .message(e.getMessage()) + .request(DirectTransportRequest.create(request.prologue(), request.headers())) + .build()); + } + + private void handleError(ConnectionContext ctx, ServerRequest request, ServerResponse response, HttpException e) { + // to be handled by error handler + handleRequestException(ctx, response, RequestException.builder() + .cause(e) + .type(DirectHandler.EventType.OTHER) + .message(e.getMessage()) + .status(e.status()) + .setKeepAlive(e.keepAlive()) + .request(DirectTransportRequest.create(request.prologue(), request.headers())) + .build()); + } +} diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Filter.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Filter.java index d6a0ce332ec..d7f73e28d8c 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Filter.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Filter.java @@ -25,6 +25,15 @@ public interface Filter { * If request processing should be terminated, do not call {@link FilterChain#proceed()}. * If request processing should continue with next filter, * call {@link FilterChain#proceed()}. + * {@link io.helidon.nima.webserver.http.FilterChain#proceed()} may throw an exception. The following exceptions + * mean the connection will close (and the request cannot finish): + *
    + *
  • {@link io.helidon.nima.webserver.CloseConnectionException}
    • + *
  • {@link java.io.UncheckedIOException}
    • + *
+ * Filters may throw {@link io.helidon.common.http.HttpException} (or its subclasses), all filters before the current + * filter will not receive such an exception - it will be processed before the method + * {@link io.helidon.nima.webserver.http.FilterChain#proceed()} returns. * * @param chain to proceed with further filters (or routing if this is the last filter) * @param req routing request diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Filters.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Filters.java index 27200a1065d..bf10dd52012 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Filters.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Filters.java @@ -19,18 +19,22 @@ import java.util.Iterator; import java.util.List; -import io.helidon.common.http.DirectHandler; +import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.parameters.Parameters; import io.helidon.common.uri.UriPath; +import io.helidon.nima.webserver.ConnectionContext; /** * Handler of HTTP filters. */ public class Filters { + private final ErrorHandlers errorHandlers; private final List filters; private final boolean noFilters; - private Filters(List filters) { + private Filters(ErrorHandlers errorHandlers, List filters) { + this.errorHandlers = errorHandlers; this.filters = filters; this.noFilters = filters.isEmpty(); } @@ -38,42 +42,50 @@ private Filters(List filters) { /** * Create filters. * - * @param filters list of filters to use + * @param errorHandlers + * @param filters list of filters to use * @return filters */ - public static Filters create(List filters) { - return new Filters(filters); + public static Filters create(ErrorHandlers errorHandlers, List filters) { + return new Filters(errorHandlers, filters); } /** * Filter request. * + * @param ctx connection context * @param request request * @param response response * @param routingExecutor this handler is called after all filters finish processing * (unless a filter does not invoke the chain) */ - public void filter(RoutingRequest request, RoutingResponse response, Runnable routingExecutor) { + public void filter(ConnectionContext ctx, RoutingRequest request, RoutingResponse response, Runnable routingExecutor) { if (noFilters) { - routingExecutor.run(); + errorHandlers.runWithErrorHandling(ctx, request, response, routingExecutor); return; } - FilterChain chain = new FilterChainImpl(filters, request, response, routingExecutor); + FilterChain chain = new FilterChainImpl(ctx, errorHandlers, filters, request, response, routingExecutor); request.path(new FilterRoutedPath(request.prologue().uriPath())); chain.proceed(); } private static final class FilterChainImpl implements FilterChain { + private final ConnectionContext ctx; + private final ErrorHandlers errorHandlers; private final Iterator filters; private final Runnable routingExecutor; private RoutingRequest request; private RoutingResponse response; - private FilterChainImpl(List filters, + private FilterChainImpl(ConnectionContext ctx, + ErrorHandlers errorHandlers, + List filters, RoutingRequest request, RoutingResponse response, Runnable routingExecutor) { + this.ctx = ctx; + this.errorHandlers = errorHandlers; this.filters = filters.iterator(); this.request = request; this.response = response; @@ -86,20 +98,21 @@ public void proceed() { return; } if (filters.hasNext()) { - filters.next().filter(this, request, response); + errorHandlers.runWithErrorHandling(ctx, request, response, this::runNextFilter); } else { - routingExecutor.run(); + errorHandlers.runWithErrorHandling(ctx, request, response, routingExecutor); if (!response.isSent()) { - throw HttpException.builder() - .request(request) - .response(response) - .type(DirectHandler.EventType.INTERNAL_ERROR) - .message("Routing finished but response was not sent. Níma does not support asynchronous responses. " - + "Please block until a response is sent.") - .build(); + // intentionally not using InternalServerException, as we do not have a cause + throw new HttpException("Routing finished but response was not sent. Níma does not support asynchronous" + + " responses. Please block until a response can be sent.", + Http.Status.INTERNAL_SERVER_ERROR_500); } } } + + private void runNextFilter() { + filters.next().filter(this, request, response); + } } private static final class FilterRoutedPath implements RoutedPath { diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpRouting.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpRouting.java index 69a2a8f0dbc..a8af327c137 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpRouting.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpRouting.java @@ -25,9 +25,12 @@ import java.util.function.Predicate; import java.util.function.Supplier; -import io.helidon.common.http.DirectHandler; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.http.HttpPrologue; +import io.helidon.common.http.InternalServerException; +import io.helidon.common.http.NotFoundException; +import io.helidon.common.http.RequestException; import io.helidon.nima.webserver.CloseConnectionException; import io.helidon.nima.webserver.ConnectionContext; import io.helidon.nima.webserver.Routing; @@ -42,9 +45,11 @@ public final class HttpRouting implements Routing { private final Filters filters; private final ServiceRoute rootRoute; + // todo configure on HTTP routing + private final ErrorHandlers errorHandlers = new ErrorHandlers(); private HttpRouting(Builder builder) { - this.filters = Filters.create(List.copyOf(builder.filters)); + this.filters = Filters.create(errorHandlers, List.copyOf(builder.filters)); this.rootRoute = builder.rootRules.build(); } @@ -88,8 +93,10 @@ public static HttpRouting empty() { * @param response routing response */ public void route(ConnectionContext ctx, RoutingRequest request, RoutingResponse response) { - RoutingExecutor routingExecutor = new RoutingExecutor(rootRoute, ctx, request, response); - filters.filter(request, response, routingExecutor); + RoutingExecutor routingExecutor = new RoutingExecutor(ctx, errorHandlers, rootRoute, request, response); + // we cannot throw an exception to the filters, as then the filter would not have information about actual status + // code, so error handling is done in routing executor and for each filter + filters.filter(ctx, request, response, routingExecutor); } @Override @@ -281,52 +288,45 @@ public Builder any(String pattern, Handler handler) { private static final class RoutingExecutor implements Runnable { private final ConnectionContext ctx; + private final ErrorHandlers errorHandlers; private final RoutingRequest request; private final RoutingResponse response; private final ServiceRoute rootRoute; - private RoutingExecutor(ServiceRoute rootRoute, ConnectionContext ctx, RoutingRequest request, RoutingResponse response) { - this.rootRoute = rootRoute; + private RoutingExecutor(ConnectionContext ctx, + ErrorHandlers errorHandlers, + ServiceRoute rootRoute, + RoutingRequest request, + RoutingResponse response) { this.ctx = ctx; + this.errorHandlers = errorHandlers; + this.rootRoute = rootRoute; this.request = request; this.response = response; } @Override public void run() { - try { - HttpPrologue prologue = request.prologue(); - response.resetRouting(); + response.resetRouting(); - RoutingResult result = RoutingResult.ROUTE; - int counter = 0; - while (result == RoutingResult.ROUTE) { - counter++; - if (counter == 10) { - LOGGER.log(System.Logger.Level.ERROR, "Rerouted more than 10 times. Will not attempt further routing"); - - throw HttpException.builder() - .request(HttpSimpleRequest.create(prologue, request.headers())) - .type(DirectHandler.EventType.INTERNAL_ERROR) - .build(); - } + RoutingResult result = RoutingResult.ROUTE; + int counter = 0; + while (result == RoutingResult.ROUTE) { + counter++; + if (counter == 10) { + LOGGER.log(System.Logger.Level.ERROR, "Rerouted more than 10 times. Will not attempt further routing"); - result = doRoute(ctx, request, response); + throw new HttpException("Too many reroutes", Http.Status.INTERNAL_SERVER_ERROR_500, true); } - // finished and done - if (result == RoutingResult.FINISH) { - return; - } - ctx.directHandlers().handle(HttpException.builder() - .request(request) - .response(response) - .type(DirectHandler.EventType.NOT_FOUND) - .message("Endpoint not found") - .build(), response); - } catch (HttpException e) { - ctx.directHandlers().handle(e, response); + result = doRoute(ctx, request, response); + } + + // finished and done + if (result == RoutingResult.FINISH) { + return; } + throw new NotFoundException("Endpoint not found"); } // todo we may have a lru cache (or most commonly used - weighted by number of usages) to @@ -370,11 +370,9 @@ private RoutingResult doRoute(ConnectionContext ctx, RoutingRequest request, Rou response.send(); return RoutingResult.FINISH; - } catch (CloseConnectionException | HttpException e) { + } catch (CloseConnectionException | RequestException | HttpException e) { throw e; } catch (Exception thrown) { - // TODO need to use error handler, error handler may reroute/next - // prevent infinite loops here if (thrown.getCause() instanceof SocketException se) { throw new UncheckedIOException(se); } @@ -390,15 +388,8 @@ private RoutingResult doRoute(ConnectionContext ctx, RoutingRequest request, Rou } // attempt to consume the request entity ctx.log(LOGGER, System.Logger.Level.WARNING, "Request failed", thrown); - // we must close connection, as we could not consume request - throw HttpException.builder() - .type(DirectHandler.EventType.INTERNAL_ERROR) - .cause(thrown) - .request(request) - .response(response) - .message(thrown.getMessage()) - .setKeepAlive(keepAlive) - .build(); + + throw new InternalServerException(thrown.getMessage(), thrown, keepAlive); } } } diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ServerResponseBase.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ServerResponseBase.java index b21acf888bf..bdb3876b600 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ServerResponseBase.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ServerResponseBase.java @@ -25,9 +25,9 @@ import io.helidon.common.GenericType; import io.helidon.common.buffers.BufferData; -import io.helidon.common.http.DirectHandler; import io.helidon.common.http.HeadersServerRequest; import io.helidon.common.http.Http; +import io.helidon.common.http.HttpException; import io.helidon.common.http.HttpPrologue; import io.helidon.common.uri.UriPath; import io.helidon.common.uri.UriQuery; @@ -106,12 +106,7 @@ public void send(Object entity) { mediaContext.writer(type, requestHeaders, headers()) .write(type, entity, outputStream, requestHeaders, this.headers()); } catch (IllegalArgumentException e) { - throw HttpException.builder() - .message(e.getMessage()) - .type(DirectHandler.EventType.OTHER) - .status(Http.Status.UNSUPPORTED_MEDIA_TYPE_415) - .request(HttpSimpleRequest.create(requestPrologue, requestHeaders)) - .build(); + throw new HttpException(e.getMessage(), Http.Status.UNSUPPORTED_MEDIA_TYPE_415, e, true); } } diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Connection.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Connection.java index e0b4e1409a3..fd5f69f042c 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Connection.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Connection.java @@ -23,6 +23,7 @@ import io.helidon.common.buffers.BufferData; import io.helidon.common.buffers.DataReader; import io.helidon.common.buffers.DataWriter; +import io.helidon.common.http.BadRequestException; import io.helidon.common.http.DirectHandler; import io.helidon.common.http.DirectHandler.EventType; import io.helidon.common.http.HeadersServerRequest; @@ -31,14 +32,15 @@ import io.helidon.common.http.Http; import io.helidon.common.http.Http.HeaderValues; import io.helidon.common.http.HttpPrologue; +import io.helidon.common.http.InternalServerException; +import io.helidon.common.http.RequestException; import io.helidon.common.mapper.MapperException; import io.helidon.nima.http.encoding.ContentDecoder; import io.helidon.nima.http.encoding.ContentEncodingContext; import io.helidon.nima.webserver.CloseConnectionException; import io.helidon.nima.webserver.ConnectionContext; -import io.helidon.nima.webserver.http.HttpException; +import io.helidon.nima.webserver.http.DirectTransportRequest; import io.helidon.nima.webserver.http.HttpRouting; -import io.helidon.nima.webserver.http.HttpSimpleRequest; import io.helidon.nima.webserver.http1.spi.Http1UpgradeProvider; import io.helidon.nima.webserver.spi.ServerConnection; @@ -139,14 +141,22 @@ public void handle() throws InterruptedException { } } catch (CloseConnectionException | UncheckedIOException e) { throw e; - } catch (HttpException e) { - handleHttpException(e); + } catch (BadRequestException e) { + handleRequestException(RequestException.builder() + .message(e.getMessage()) + .cause(e) + .type(EventType.BAD_REQUEST) + .status(e.status()) + .setKeepAlive(e.keepAlive()) + .build()); + } catch (RequestException e) { + handleRequestException(e); } catch (Throwable e) { - handleHttpException(HttpException.builder() - .message("Internal error") - .type(EventType.INTERNAL_ERROR) - .cause(e) - .build()); + handleRequestException(RequestException.builder() + .message("Internal error") + .type(EventType.INTERNAL_ERROR) + .cause(e) + .build()); } } @@ -167,10 +177,10 @@ private BufferData readNextChunk(HttpPrologue prologue, HeadersWritable heade currentEntitySizeRead += chunkLength; if (maxPayloadSize != -1 && currentEntitySizeRead > maxPayloadSize) { - throw HttpException.builder() + throw RequestException.builder() .type(EventType.BAD_REQUEST) .status(Http.Status.REQUEST_ENTITY_TOO_LARGE_413) - .request(HttpSimpleRequest.create(prologue, headers)) + .request(DirectTransportRequest.create(prologue, headers)) .setKeepAlive(false) .build(); } @@ -178,7 +188,7 @@ private BufferData readNextChunk(HttpPrologue prologue, HeadersWritable heade if (chunkLength == 0) { String end = reader.readLine(); if (!end.isEmpty()) { - throw HttpException.builder() + throw RequestException.builder() .type(EventType.BAD_REQUEST) .message("Invalid terminating chunk") .build(); @@ -213,18 +223,18 @@ private void route(HttpPrologue prologue, HeadersWritable headers) { try { this.currentEntitySize = headers.get(Http.Header.CONTENT_LENGTH).value(long.class); if (maxPayloadSize != -1 && currentEntitySize > maxPayloadSize) { - throw HttpException.builder() + throw RequestException.builder() .type(EventType.BAD_REQUEST) .status(Http.Status.REQUEST_ENTITY_TOO_LARGE_413) - .request(HttpSimpleRequest.create(prologue, headers)) + .request(DirectTransportRequest.create(prologue, headers)) .setKeepAlive(false) .build(); } entity = currentEntitySize == 0 ? EntityStyle.NONE : EntityStyle.LENGTH; } catch (MapperException e) { - throw HttpException.builder() + throw RequestException.builder() .type(EventType.BAD_REQUEST) - .request(HttpSimpleRequest.create(prologue, headers)) + .request(DirectTransportRequest.create(prologue, headers)) .message("Content length is not a number") .cause(e) .build(); @@ -266,9 +276,9 @@ private void route(HttpPrologue prologue, HeadersWritable headers) { if (contentEncodingContext.contentDecodingSupported(contentEncoding)) { decoder = contentEncodingContext.decoder(contentEncoding); } else { - throw HttpException.builder() + throw RequestException.builder() .type(EventType.BAD_REQUEST) - .request(HttpSimpleRequest.create(prologue, headers)) + .request(DirectTransportRequest.create(prologue, headers)) .message("Unsupported content encoding") .build(); } @@ -301,9 +311,9 @@ private void route(HttpPrologue prologue, HeadersWritable headers) { try { entityReadLatch.await(); } catch (InterruptedException e) { - throw HttpException.builder() + throw RequestException.builder() .type(EventType.INTERNAL_ERROR) - .request(HttpSimpleRequest.create(prologue, headers)) + .request(DirectTransportRequest.create(prologue, headers)) .message("Failed to wait for pipeline") .cause(e) .build(); @@ -322,25 +332,13 @@ private void consumeEntity(Http1ServerRequest request, Http1ServerResponse respo boolean keepAlive = request.content().consumed() && response.headers().contains(HeaderValues.CONNECTION_KEEP_ALIVE); // we must close connection, as we could not consume request if (!response.isSent()) { - throw HttpException.builder() - .type(EventType.INTERNAL_ERROR) - .cause(e) - .request(request) - .response(response) - .message(e.getMessage()) - .setKeepAlive(keepAlive) - .build(); + throw new InternalServerException(e.getMessage(), e, keepAlive); } throw new CloseConnectionException("Failed to consume request entity, must close", e); } } - private void handleHttpException(HttpException e) { - if (e.fullResponse().isPresent()) { - ctx.directHandlers().handle(e, e.fullResponse().get()); - return; - } - + private void handleRequestException(RequestException e) { DirectHandler handler = ctx.directHandlers().handler(e.eventType()); DirectHandler.TransportResponse response = handler.handle(e.request(), e.eventType(), @@ -362,6 +360,8 @@ private void handleHttpException(HttpException e) { buffer.write(message); } + sendListener.headers(ctx, headers); + sendListener.data(ctx, buffer); writer.write(buffer); if (response.status() == Http.Status.INTERNAL_SERVER_ERROR_500) { diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Headers.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Headers.java index 0746cd43894..33f6ba187fb 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Headers.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Headers.java @@ -21,8 +21,8 @@ import io.helidon.common.http.HeadersWritable; import io.helidon.common.http.Http1HeadersParser; import io.helidon.common.http.HttpPrologue; -import io.helidon.nima.webserver.http.HttpException; -import io.helidon.nima.webserver.http.HttpSimpleRequest; +import io.helidon.common.http.RequestException; +import io.helidon.nima.webserver.http.DirectTransportRequest; /** * HTTP/1 headers reader. @@ -56,9 +56,9 @@ public HeadersWritable readHeaders(HttpPrologue prologue) { try { return Http1HeadersParser.readHeaders(reader, maxHeadersSize, validateHeaders); } catch (IllegalStateException | IllegalArgumentException e) { - throw HttpException.builder() + throw RequestException.builder() .type(DirectHandler.EventType.BAD_REQUEST) - .request(HttpSimpleRequest.create(prologue, HeadersWritable.create())) + .request(DirectTransportRequest.create(prologue, HeadersWritable.create())) .message(e.getMessage()) .cause(e) .build(); diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Prologue.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Prologue.java index 2fb96163191..84b0e6cb853 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Prologue.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Prologue.java @@ -23,9 +23,9 @@ import io.helidon.common.http.DirectHandler; import io.helidon.common.http.Http; import io.helidon.common.http.HttpPrologue; +import io.helidon.common.http.RequestException; import io.helidon.nima.webserver.CloseConnectionException; -import io.helidon.nima.webserver.http.HttpException; -import io.helidon.nima.webserver.http.HttpSimpleRequest; +import io.helidon.nima.webserver.http.DirectTransportRequest; /** * HTTP 1 prologue parsing support. @@ -63,16 +63,16 @@ public HttpPrologue readPrologue() { } } - private static HttpException badRequest(String message, String method, String path, String protocol, String version) { + private static RequestException badRequest(String message, String method, String path, String protocol, String version) { String protocolAndVersion; if (protocol.isBlank() && version.isBlank()) { protocolAndVersion = ""; } else { protocolAndVersion = protocol + "/" + version; } - return HttpException.builder() + return RequestException.builder() .type(DirectHandler.EventType.BAD_REQUEST) - .request(HttpSimpleRequest.create(protocolAndVersion, method, path)) + .request(DirectTransportRequest.create(protocolAndVersion, method, path)) .message(message) .build(); } @@ -113,11 +113,11 @@ private HttpPrologue doRead() { if (secondSpace < 0) { throw badRequest("Invalid prologue" + reader.debugDataHex(), method.text(), "", "", ""); } else if (secondSpace == maxLen) { - throw HttpException.builder() + throw RequestException.builder() .message("Request URI too long.") .type(DirectHandler.EventType.BAD_REQUEST) .status(Http.Status.REQUEST_URI_TOO_LONG_414) - .request(HttpSimpleRequest.create("", method.text(), reader.readAsciiString(secondSpace))) + .request(DirectTransportRequest.create("", method.text(), reader.readAsciiString(secondSpace))) .build(); } path = reader.readAsciiString(secondSpace); @@ -132,7 +132,7 @@ private HttpPrologue doRead() { protocol = reader.readAsciiString(eol); reader.skip(2); // \r\n } catch (DataReader.IncorrectNewLineException e) { - throw HttpException.builder() + throw RequestException.builder() .message("Invalid prologue: " + e.getMessage()) .type(DirectHandler.EventType.BAD_REQUEST) .cause(e) diff --git a/nima/websocket/webserver/src/main/java/io/helidon/nima/websocket/webserver/WsUpgradeProvider.java b/nima/websocket/webserver/src/main/java/io/helidon/nima/websocket/webserver/WsUpgradeProvider.java index 79c904257cf..707fa8611b6 100644 --- a/nima/websocket/webserver/src/main/java/io/helidon/nima/websocket/webserver/WsUpgradeProvider.java +++ b/nima/websocket/webserver/src/main/java/io/helidon/nima/websocket/webserver/WsUpgradeProvider.java @@ -32,8 +32,8 @@ import io.helidon.common.http.Http.Header; import io.helidon.common.http.Http.HeaderName; import io.helidon.common.http.HttpPrologue; +import io.helidon.common.http.RequestException; import io.helidon.nima.webserver.ConnectionContext; -import io.helidon.nima.webserver.http.HttpException; import io.helidon.nima.webserver.http1.spi.Http1UpgradeProvider; import io.helidon.nima.webserver.spi.ServerConnection; @@ -105,7 +105,7 @@ public ServerConnection upgrade(ConnectionContext ctx, HttpPrologue prologue, He } if (!SUPPORTED_VERSION.equals(version)) { - throw HttpException.builder() + throw RequestException.builder() .type(DirectHandler.EventType.BAD_REQUEST) .message("Unsupported WebSocket Version") .header(SUPPORTED_VERSION_HEADER) @@ -123,7 +123,7 @@ public ServerConnection upgrade(ConnectionContext ctx, HttpPrologue prologue, He if (headers.contains(Header.ORIGIN)) { String origin = headers.get(Header.ORIGIN).value(); if (!origins.contains(origin)) { - throw HttpException.builder() + throw RequestException.builder() .message("Invalid Origin") .type(DirectHandler.EventType.FORBIDDEN) .build(); @@ -155,7 +155,7 @@ private String hash(ConnectionContext ctx, String wsKey) { been base64-encoded (see Section 4 of [RFC4648]). The nonce MUST be selected randomly for each connection. */ - throw HttpException.builder() + throw RequestException.builder() .type(DirectHandler.EventType.BAD_REQUEST) .message("Invalid Sec-WebSocket-Key header") .build(); From bd423edce97ccb2e065976154428c826e8e1e9d9 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Mon, 29 Aug 2022 11:27:18 +0200 Subject: [PATCH 3/3] Review fix. --- .../main/java/io/helidon/common/http/ForbiddenException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/http/src/main/java/io/helidon/common/http/ForbiddenException.java b/common/http/src/main/java/io/helidon/common/http/ForbiddenException.java index 2d545fda290..76b615b84ff 100644 --- a/common/http/src/main/java/io/helidon/common/http/ForbiddenException.java +++ b/common/http/src/main/java/io/helidon/common/http/ForbiddenException.java @@ -17,7 +17,7 @@ package io.helidon.common.http; /** - * A runtime exception indicating a {@link io.helidon.common.http.Http.Status#NOT_FOUND_404 not found}. + * A runtime exception indicating a {@link io.helidon.common.http.Http.Status#FORBIDDEN_403 not found}. */ public class ForbiddenException extends HttpException {