<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">I am confused though - I reviewed the robaho source code which came from the JDK, and the setAttribute is on the exchange, which is per request, not per context.<br><div><br><blockquote type="cite"><div>On Dec 6, 2024, at 8:41 AM, robert engels <rengels@ix.netcom.com> wrote:</div><br class="Apple-interchange-newline"><div><meta http-equiv="content-type" content="text/html; charset=utf-8"><div style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">In minor defense of the JDK (I am worried more about ambiguity) - I’ve seen the JDK users implement this by add to the request headers - but with the JDK making this read-only, this is no longer possible, so the get/set attribute is the only viable way to pass per request information between layers.<br><div><br><blockquote type="cite"><div>On Dec 5, 2024, at 7:27 PM, Ethan McCue <ethan@mccue.dev> wrote:</div><br class="Apple-interchange-newline"><div><div dir="ltr"><div>After mulling it over some more, I think that as is there is actually no valid use for .setAttribute as implemented by the JDK<br><br>Even the most trivial usages of it are broken under moderate load. This includes the usage in SimpleFileServer.createOutputFilter and SimpleFileServer.createFileHandler<br><br><font face="monospace">import com.sun.net.httpserver.HttpServer;<br>import com.sun.net.httpserver.SimpleFileServer;<br><br>import java.io.ByteArrayOutputStream;<br>import java.net.InetSocketAddress;<br>import java.net.URI;<br>import java.net.http.HttpClient;<br>import java.net.http.HttpRequest;<br>import java.net.http.HttpResponse;<br>import java.nio.charset.StandardCharsets;<br>import java.nio.file.Files;<br>import java.nio.file.Path;<br>import java.util.concurrent.Executors;<br>import java.util.concurrent.atomic.AtomicInteger;<br>import java.util.regex.Pattern;<br><br>public class Main {<br> public static void main(String[] args) throws Exception {<br> Files.writeString(Path.of("./a"), "a".repeat(100000));<br> Files.writeString(Path.of("./b"), "b".repeat(100000));<br><br> var serverExecutor = Executors.newFixedThreadPool(2);<br> var baos = new ByteArrayOutputStream();<br> var server = HttpServer.create(new InetSocketAddress(8841), 0);<br> server.createContext("/", SimpleFileServer.createFileHandler(Path.of(".").toAbsolutePath()))<br> .getFilters().add(SimpleFileServer.createOutputFilter(baos, SimpleFileServer.OutputLevel.VERBOSE));<br> server.setExecutor(serverExecutor);<br> server.start();<br><br> Thread.sleep(1000);<br><br> var executor = Executors.newFixedThreadPool(2);<br><br> int total = 10000;<br><br> AtomicInteger failures = new AtomicInteger();<br> for (int i = 0; i < total; i++) {<br> String file = i % 2 == 0 ? "a" : "b";<br> executor.submit(() -> {<br> try {<br> var client = HttpClient.newHttpClient();<br> client.send(<br> HttpRequest.newBuilder(URI.create("<a href="http://0.0.0.0:8841/">http://0.0.0.0:8841/</a>" + file)).build(),<br> HttpResponse.BodyHandlers.ofString()<br> );<br> } catch (Exception e) {<br> e.printStackTrace(System.out);<br> failures.getAndIncrement();<br> }<br> return null;<br> });<br> }<br><br> executor.close();<br><br> int a = 0;<br> int b = 0;<br> var s = baos.toString(StandardCharsets.UTF_8).split("\n");<br> for (var line : s) {<br> Pattern aPattern = Pattern.compile("Resource requested: (.+)/a");<br> Pattern bPattern = Pattern.compile("Resource requested: (.+)/b");<br> if (aPattern.matcher(line).find()) {<br> a++;<br> }<br> else if (bPattern.matcher(line).find()) {<br> b++;<br> }<br> }<br> System.out.println("Reported a request to /a: " + a);<br> System.out.println("Reported a request to /b: " + b);<br> System.out.println("Failures: " + failures);<br><br> server.stop(0);<br> serverExecutor.close();<br> }<br>}</font><br><br>Despite an equal number of requests being made to /a and /b the output filter will report a randomly diverging amount. This is because there is simply no way to avoid concurrent requests clobbering each-others state while calling setAttribute on an exchange does not actually set an attribute on that exchange.<br><br></div><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Thu, Dec 5, 2024 at 11:15 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">Sorry, meant to send this to the list:<br><br>I will add as maybe obvious context that the way the JDK currently implements this is (I think, correct me if I am wrong) a security nightmare. That it might not be obvious (or uniform across an ecosystem of implementations) that <font face="monospace">exchange.setAttribute("CURRENTLY_AUTHENTICATED_USER", "...");</font><font face="arial, sans-serif"> will not actually be setting an attribute on the exchange, but instead a global thing, <i><b>is </b></i><i style="font-weight:bold">an issue</i><br></font><br>If this is a deliberate choice in the existing implementation, I am curious to know how it came about.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 5, 2024 at 11:13 PM Robert Engels <<a href="mailto:rengels@ix.netcom.com" target="_blank">rengels@ix.netcom.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"><div dir="auto"><div dir="ltr"></div><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div dir="ltr">I read the bug report. I don’t think this is sufficient. This is a specification so in order to have compliant behavior no matter the implementation there cannot be a different set of rules for the reference implementation vs others. </div><div dir="ltr"><br></div><div dir="ltr">So the api should be clarified in a non ambiguous manner and then one or more implementations can be classified as non compliant. </div><div dir="ltr"><br></div><div dir="ltr">Robert</div><div dir="ltr"><br><blockquote type="cite">On Dec 5, 2024, at 6:31 AM, Jaikiran Pai <<a href="mailto:jai.forums2013@gmail.com" target="_blank">jai.forums2013@gmail.com</a>> wrote:<br><br></blockquote></div><blockquote type="cite"><div dir="ltr">
<p>Hello Ethan,</p><p>Thank you for noticing this and bringing this up here. I've
raised <a href="https://bugs.openjdk.org/browse/JDK-8345577" target="_blank">https://bugs.openjdk.org/browse/JDK-8345577</a> and we will
address this shortly.</p><p>-Jaikiran<br>
</p>
<div>On 05/12/24 3:22 am, Ethan McCue wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Sorry<br>
<br>
Before:<br>
<br>
<font face="monospace"> * {@link Filter} modules may store
arbitrary objects with {@code HttpExchange}</font><br>
<font face="monospace"> * instances as an out-of-band
communication mechanism. Other filters</font><br>
<font face="monospace"> * or the exchange handler may then
access these objects.</font><br>
<br>
<font face="arial, sans-serif">Bungled the copy-paste</font></div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Dec 5, 2024 at 6:49 AM
Ethan McCue <<a href="mailto:ethan@mccue.dev" target="_blank">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">Hi all,<br>
<br>
This change (<a href="https://github.com/openjdk/jdk/commit/40ae4699622cca72830acd146b7b5c4efd5a43ec" target="_blank">https://github.com/openjdk/jdk/commit/40ae4699622cca72830acd146b7b5c4efd5a43ec</a>)
makes the Jetty implementation of the SPI be no longer fit
the Javadoc.<br>
<br>
HttpContexts are not per-request while the previous Javadoc
implied that the attribute mechanism on exchanges was.<br>
<br>
Before:<br>
<br>
<font face="monospace"> * Sets an attribute with the
given {@code name} and {@code value} in this exchange's<br>
* {@linkplain HttpContext#getAttributes() context
attributes}.<br>
* or the exchange handler may then access these
objects.</font><br>
<br>
After:<br>
<br>
<font face="monospace"> * Sets an attribute with the
given {@code name} and {@code value} in this exchange's<br>
* {@linkplain HttpContext#getAttributes() context
attributes}.<br>
*<br>
* @apiNote {@link Filter} modules may store arbitrary
objects as attributes through<br>
* {@code HttpExchange} instances as an out-of-band
communication mechanism. Other filters<br>
* or the exchange handler may then access these
objects.<br>
</font><br>
The Jetty implementation, I think rightfully, assumed that
this context was per-request and implemented it as so.<br>
<br>
<a href="https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-core/jetty-http-spi/src/main/java/org/eclipse/jetty/http/spi/JettyHttpExchangeDelegate.java#L223" target="_blank">https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-core/jetty-http-spi/src/main/java/org/eclipse/jetty/http/spi/JettyHttpExchangeDelegate.java#L223</a><br>
<br>
As such, I don't think simply a javadoc change as a
resolution to these issues is applicable<br>
<br>
<a href="https://bugs.openjdk.org/browse/JDK-8345233" target="_blank">https://bugs.openjdk.org/browse/JDK-8345233</a><br>
<a href="https://bugs.openjdk.org/browse/JDK-8235786" target="_blank">https://bugs.openjdk.org/browse/JDK-8235786</a><br>
<br>
</div>
</blockquote>
</div>
</blockquote>
</div></blockquote></div></blockquote></div>
</blockquote></div></div>
</div></blockquote></div><br></div></div></blockquote></div><br></body></html>