nevermind - i see the file server does read and discard any incoming bodies.
On Mon, Apr 28, 2025 at 5:52 PM Ethan McCue <et...@mccue.dev> wrote:
> 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
>>>
>>
