Sorry, forgot reply-all
On Mon, Apr 28, 2025 at 5:52 PM Ethan McCue <et...@mccue.dev> wrote:
> Today you would need to do the following:
>
> exchange -> {
> var body = "example".getBytes();
> exchange.sendResponseHeaders(200, body.length);
> try (var os = exchange.getResponseBody()) {
> os.write(body);
> }
> }
>
> I don't think you need to read the request body, but I might be wrong
> about that. If so, I have some thinking to do. (and FileServerHandler
> needs to be fixed)
>
> And if you weren't doing a static value, you'd want to do this.
>
> exchange -> {
> var body = compute().getBytes();
> exchange.sendResponseHeaders(200, body.length == 0 ? -1 : body.length);
> try (var os = exchange.getResponseBody()) {
> os.write(body);
> }
> }
>
> Where the body.length == 0 ? -1 : body.length is for technical
> correctness in case the body is length zero.
>
> Essentially, I think something approximately like what I have here should
> be integrated. https://github.com/bowbahdoe/jdk-httpserver
>
> The reason I'm not pushing on it is:
>
> * The design has a ResponseLength.known(..) and ResponseLength.unknown() which
> handle that 0 ? -1 nonsense, but that only works with corresponding patterns
> * ResponseLength should be value class
> * The Body interface has a defaultContentType which scratches at the back
> of my brain - Could a Body imply additional headers? Is it worth having
> just for JsonBody.of(...);?
>
> But essentially, this is the meat of it.
>
> exchange -> HttpExchanges.sendResponse(exchange, 200, Body.of("example"));
>
> interface Body {
> void writeTo(OutputStream outputStream) throws IOException;
>
> default ResponseLength responseLength() {
> return ResponseLength.unknown();
> }
>
> default Optional<String> defaultContentType() {
> return Optional.empty();
> }
>
> // ...
> }
>
> public static void sendResponse(
> HttpExchange exchange, int rCode, Body body
> ) throws IOException {
> body.defaultContentType()
> .ifPresent(contentType ->
> exchange.getResponseHeaders().putIfAbsent(
> "Content-Type",
> List.of(contentType)
> ));
>
> sendResponseHeaders(exchange, rCode, body.responseLength());
> try (var os = exchange.getResponseBody()) {
> body.writeTo(os);
> }
> }
>
> On Mon, Apr 28, 2025 at 5:36 PM Pavel Rappo <pavel.ra...@gmail.com> wrote:
>
>> You are right that a function would be more general than a supplier.
>> That crossed my mind too. However, here, with greater generality comes
>> less convenience. Like with your example, don't you need to read a
>> request body and then finally close the exchange? Perhaps that can be
>> solved with better documentation rather than one more convenience
>> method. Perhaps documentation could provide (a snippet of) a skeletal
>> implementation of a handler.
>>
>> Also, even the "static string" convenience handler is problematic: no
>> one sets the content type because it's unknown: the passed string can
>> be HTML, JSON, plain text or what have you. And no one sets the
>> character encoding because (AFAIK) it only comes with the content type
>> and not on its own.
>>
>> On Mon, Apr 28, 2025 at 8:53 PM Ethan McCue <et...@mccue.dev> wrote:
>> >
>> > My first thought is that if you are computing the string maybe you'd
>> want to look at the request. Or set a header for content type.
>> >
>> > I'm not going to push hard until valhalla is at least in preview (+
>> we've already talked it to death) but a Body abstraction would probably
>> solve your use-case more robustly than yet another method there.
>> >
>> > exchange -> exchange.sendResponse(200, Body.of("example"));
>> >
>> > On Mon, Apr 28, 2025 at 5:28 AM Pavel Rappo <pavel.ra...@gmail.com>
>> wrote:
>> >>
>> >> I'm using HttpServer to implement an HTTP probe [^1] that provides
>> >> application state at the time of probing. I find that convenience
>> >> handlers provided by HttpHandlers are insufficient for my use case. I
>> >> also find that implementing a custom HttpHandler is tricky without the
>> >> help of documentation.
>> >>
>> >> Perhaps my use case is typical enough so that HttpHandlers could
>> >> provide a new convenience handler. That handler would send a
>> >> dynamically supplied string, rather than a static string. If you also
>> >> find it useful, I'd appreciate it if we could discuss it. Before I
>> >> create a JBS issue, please have a look at this crude patch to see if
>> >> it makes sense to you in principle. Thanks.
>> >>
>> >> [^1]:
>> https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/
>> >>
>> >> diff --git
>> a/src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpHandlers.java
>> >>
>> b/src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpHandlers.java
>> >> index 03642033914..987de0ede5d 100644
>> >> ---
>> a/src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpHandlers.java
>> >> +++
>> b/src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpHandlers.java
>> >> @@ -25,9 +25,11 @@
>> >>
>> >> package com.sun.net.httpserver;
>> >>
>> >> +import java.io.OutputStream;
>> >> import java.nio.charset.StandardCharsets;
>> >> import java.util.Objects;
>> >> import java.util.function.Predicate;
>> >> +import java.util.function.Supplier;
>> >>
>> >> /**
>> >> * Implementations of {@link com.sun.net.httpserver.HttpHandler
>> HttpHandler}
>> >> @@ -140,28 +142,60 @@ public static HttpHandler
>> >> handleOrElse(Predicate<Request> handlerTest,
>> >> * @throws NullPointerException if headers or body are null
>> >> */
>> >> public static HttpHandler of(int statusCode, Headers headers,
>> >> String body) {
>> >> + Objects.requireNonNull(body);
>> >> + return of(statusCode, headers, () -> body);
>> >> + }
>> >> +
>> >> + /**
>> >> + * Returns an {@code HttpHandler} that sends a response
>> >> comprising the given
>> >> + * {@code statusCode}, {@code headers}, and {@code body}.
>> >> + *
>> >> + * <p> This method creates a handler that reads and discards the
>> request
>> >> + * body before it sets the response state and sends the response.
>> >> + *
>> >> + * <p> {@code headers} are the effective headers of the response.
>> The
>> >> + * response <i>body bytes</i> are a {@code UTF-8} encoded byte
>> sequence of
>> >> + * a string, which is supplied by {@code bodySupplier}
>> >> immediately after the request body is read. The response headers
>> >> + * {@linkplain HttpExchange#sendResponseHeaders(int, long) are
>> sent} with
>> >> + * the given {@code statusCode} and the body bytes' length (or
>> {@code -1}
>> >> + * if the body is empty). The body bytes are then sent as
>> response body,
>> >> + * unless the body is empty, in which case no response body is
>> sent.
>> >> + *
>> >> + * @param statusCode a response status code
>> >> + * @param headers a headers
>> >> + * @param bodySupplier a supplier for the response body string
>> >> + * @return a handler
>> >> + * @throws IllegalArgumentException if statusCode is not a
>> positive 3-digit
>> >> + * integer, as per rfc2616,
>> section 6.1.1
>> >> + * @throws NullPointerException if headers or body are null
>> >> + */
>> >> + public static HttpHandler of(int statusCode, Headers headers,
>> >> Supplier<String> bodySupplier) {
>> >> if (statusCode < 100 || statusCode > 999)
>> >> throw new IllegalArgumentException("statusCode must be
>> 3-digit: "
>> >> + statusCode);
>> >> Objects.requireNonNull(headers);
>> >> - Objects.requireNonNull(body);
>> >> + Objects.requireNonNull(bodySupplier);
>> >>
>> >> final var headersCopy = Headers.of(headers);
>> >> - final var bytes = body.getBytes(StandardCharsets.UTF_8);
>> >>
>> >> return exchange -> {
>> >> try (exchange) {
>> >> - exchange.getRequestBody().readAllBytes();
>> >> +
>> >> exchange.getRequestBody().transferTo(OutputStream.nullOutputStream());
>> >> // discard
>> >> exchange.getResponseHeaders().putAll(headersCopy);
>> >> - if (exchange.getRequestMethod().equals("HEAD")) {
>> >> -
>> >> exchange.getResponseHeaders().set("Content-Length",
>> >> Integer.toString(bytes.length));
>> >> - exchange.sendResponseHeaders(statusCode, -1);
>> >> - }
>> >> - else if (bytes.length == 0) {
>> >> - exchange.sendResponseHeaders(statusCode, -1);
>> >> + var body = bodySupplier.get();
>> >> + if (body == null) {
>> >> + exchange.sendResponseHeaders(500, -1); //
>> >> Internal Server Error
>> >> } else {
>> >> - exchange.sendResponseHeaders(statusCode,
>> bytes.length);
>> >> - exchange.getResponseBody().write(bytes);
>> >> + final var bytes =
>> body.getBytes(StandardCharsets.UTF_8);
>> >> + if (exchange.getRequestMethod().equals("HEAD")) {
>> >> +
>> >> exchange.getResponseHeaders().set("Content-Length",
>> >> Integer.toString(bytes.length));
>> >> + exchange.sendResponseHeaders(statusCode, -1);
>> >> + } else if (bytes.length == 0) {
>> >> + exchange.sendResponseHeaders(statusCode, -1);
>> >> + } else {
>> >> + exchange.sendResponseHeaders(statusCode,
>> bytes.length);
>> >> + exchange.getResponseBody().write(bytes);
>> >> + }
>> >> }
>> >> }
>> >> };
>> >> diff --git
>> a/test/jdk/com/sun/net/httpserver/simpleserver/HttpHandlersTest.java
>> >> b/test/jdk/com/sun/net/httpserver/simpleserver/HttpHandlersTest.java
>> >> index 85d271e44fa..d64fa03740f 100644
>> >> ---
>> a/test/jdk/com/sun/net/httpserver/simpleserver/HttpHandlersTest.java
>> >> +++
>> b/test/jdk/com/sun/net/httpserver/simpleserver/HttpHandlersTest.java
>> >> @@ -81,7 +81,7 @@ public void testNull() {
>> >> final var headers = new Headers();
>> >> final var body = "";
>> >> assertThrows(NPE, () -> HttpHandlers.of(200, null, body));
>> >> - assertThrows(NPE, () -> HttpHandlers.of(200, headers, null));
>> >> + assertThrows(NPE, () -> HttpHandlers.of(200, headers,
>> (String) null));
>> >> }
>> >>
>> >> @Test
>>
>
