RFR: 7105350: HttpExchange's attributes are the same as HttpContext's attributes [v14]

[Daniel Fuchs]

On Tue, 21 Oct 2025 15:14:48 GMT, Josiah Noel <duke at openjdk.org> wrote:

>> Now ExchangeImpl will default to having a separate attribute map for the request duration.
>
> Josiah Noel has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Refactor module-info.java for imports and notes
>   
>   Updated import statement and modified implementation notes.

Changes requested by dfuchs (Reviewer).

src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java line 2:

> 1: /*
> 2:  * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.

Suggestion:

 * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.

test/jdk/com/sun/net/httpserver/ExchangeAttributePerExchangeTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.

Suggestion:

 * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.

test/jdk/com/sun/net/httpserver/ExchangeAttributePerExchangeTest.java line 29:

> 27:  * @summary Test HttpExchange set/getAttribute do not affect HttpContext attributes
> 28:  * @library /test/lib
> 29:  * @run junit/othervm ExchangeAttributePerExchangeTest

Suggestion:

 * @run junit/othervm ExchangeAttributePerExchangeTest
 * @run junit/othervm -Djdk.httpserver.attributes=context ExchangeAttributePerExchangeTest
 * @run junit/othervm -Djdk.httpserver.attributes=random-string ExchangeAttributePerExchangeTest
 * @run junit/othervm -Djdk.httpserver.attributes ExchangeAttributePerExchangeTest

test/jdk/com/sun/net/httpserver/ExchangeAttributePerExchangeTest.java line 109:

> 107:                 assertNotEquals("val", exchange.getHttpContext().getAttributes().get("attr"));
> 108:                 exchange.setAttribute("attr", null);
> 109:                 assertNull(exchange.getAttribute("attr"));

Suggestion:

                assertNull(exchange.getAttribute("attr"));
                assertEquals("context-val", exchange.getHttpContext().getAttributes().get("attr"));

test/jdk/com/sun/net/httpserver/ExchangeAttributePerExchangeTest.java line 117:

> 115:         }
> 116:     }
> 117: }

Please add newline at the end

test/jdk/com/sun/net/httpserver/ExchangeAttributeTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.

Suggestion:

 * Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved.

test/jdk/com/sun/net/httpserver/ExchangeAttributeTest.java line 72:

> 70:     @Test
> 71:     public void testExchangeAttributes() throws Exception {
> 72:         System.getProperty("jdk.httpserver.attributes", "context")

I am suspecting `System.setProperty()` was intended here. Since the property is only consulted once in a static analyzer then using `System.setProperty()` is not appropriate. Instead the property should passed on the command line. And I think it would be better to test the effect of the property in your new test, rathar than here.

-------------

PR Review: https://git.openjdk.org/jdk/pull/27652#pullrequestreview-3361381907
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448759770
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448757328
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448824400
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448776861
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448781343
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448788295
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448818096