You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "breken-ai (via GitHub)" <gi...@apache.org> on 2026/09/30 03:01:06 UTC
[PR] Fix Map.Entry contract for CaseInsensitiveKeyMap entries [tomcat]
breken-ai opened a new pull request, #1075:
URL: https://github.com/apache/tomcat/pull/1075
## Fix `Map.Entry` contract for `CaseInsensitiveKeyMap` entries
`CaseInsensitiveKeyMap.entrySet()` returns `EntryImpl` instances. `EntryImpl` is a record, so:
- `equals()` only returns true for another `EntryImpl`. An entry therefore does not equal an equivalent entry from a `HashMap`, even though the reverse comparison returns true (asymmetric `equals()`).
- `hashCode()` uses the record formula rather than the `Map.Entry` definition (`key.hashCode() ^ value.hashCode()`). Because `AbstractMap.hashCode()` sums the entry hash codes, a `CaseInsensitiveKeyMap` that `equals()` a `HashMap` has a different `hashCode()`, which breaks the `Map`/`Object` contract.
```java
Map<String,Object> ci = new CaseInsensitiveKeyMap<>();
ci.put("a", "b");
Map<String,Object> hm = new HashMap<>(ci);
hm.equals(ci); // true
hm.hashCode() == ci.hashCode(); // false
```
This map backs the header maps from the WebSocket `HandshakeRequest.getHeaders()`, `HandshakeResponse.getHeaders()` and the client `Configurator` callbacks, so applications can see these maps.
The fix returns `AbstractMap.SimpleImmutableEntry` instead of `EntryImpl` and removes the record. `SimpleImmutableEntry` implements the `Map.Entry` contract and still throws `UnsupportedOperationException` from `setValue()`, so that behaviour is unchanged.
### Testing
- Added `TestCaseInsensitiveKeyMap.testEntryEqualsAndHashCode`. On `main` (2a7e82c) it fails at `entry.equals(hashMapEntry)`, and it passes with the fix.
- `ant test -Dtest.name='org/apache/tomcat/util/collections/Test*.java'`: all pass. `org.apache.tomcat.websocket.TestWsWebSocketContainer`: 24/24 pass (NIO, JDK 21.0.12).
- `ant validate -Dexecute.validate=true` (Checkstyle) passes.
- Added a changelog entry under WebSocket.
I wrote this change with help from an AI coding assistant, then reviewed it and ran the tests above myself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Fix Map.Entry contract for CaseInsensitiveKeyMap entries [tomcat]
Posted by "breken-ai (via GitHub)" <gi...@apache.org>.
breken-ai commented on PR #1075:
URL: https://github.com/apache/tomcat/pull/1075#issuecomment-6049588623
`SimpleImmutableEntry` here is the JDK's `java.util.AbstractMap.SimpleImmutableEntry` (since Java 6). `CaseInsensitiveKeyMap` extends `AbstractMap`, so the nested `EntrySet`/`EntryIterator` code can refer to it by its simple name through the inherited member type, and no import or new class is needed. The PR doesn't add a class of its own.
I checked this against the branch's `CaseInsensitiveKeyMap.java`, with a `StringManager` stub, using `javac -Xlint:all` from JDK 17. It compiles without warnings. An entry from `entrySet().iterator().next()` is a `java.util.AbstractMap$SimpleImmutableEntry`, `entry.equals(hashMapEntry)` is true, `hashMap.hashCode() == map.hashCode()` is true, and `setValue()` still throws `UnsupportedOperationException`. The full `ant test` run in the description was on JDK 21.
If you meant that it should be written out explicitly, I can change the call to `new AbstractMap.SimpleImmutableEntry<>(...)`. Or, if you'd rather keep a Tomcat-owned type, I can keep `EntryImpl` and give it `equals()`/`hashCode()` that follow the `Map.Entry` definitions. Let me know which you prefer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Fix Map.Entry contract for CaseInsensitiveKeyMap entries [tomcat]
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on PR #1075:
URL: https://github.com/apache/tomcat/pull/1075#issuecomment-5913287108
SimpleImmutableEntry has not been provided.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org