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