jungm commented on PR #2844: URL: https://github.com/apache/tomee/pull/2844#issuecomment-5108836430
Thanks — I verified all of it independently and you're right on every count. Pushed in 164b44ba10. **Precedence.** Confirmed against tomcat-jasper 11.0.23 bytecode: `scan()` calls `scanPlatform()` first, `parseTld()` guards its put with `containsKey` + `ifne` (first writer wins), and `scanJspConfig()` does `containsKey` → `Log.warn` → skip. So the description was backwards and a bundled Jakarta-native JSTL does not win. Taking your steer and keeping the precedence as-is, since it matches how the legacy `http://java.sun.com/jsp/jstl/*` URIs already behave. It is now stated in the `aliasJakartaTagsUris()` javadoc and in the commit message, and pinned by `containerAliasWinsOverAnApplicationBundledTld`, which drives the real `parseTld()` with an app TLD declaring `jakarta.tags.core` — so the assertion tracks Tomcat rather than restating my reading of it. **The `isEmpty()` guard.** Real bug, exactly as you described: `populateMyfacesAndJstlData()` guards myfaces (L125) and JSTL (L4142) independently, so myfaces-without-JSTL left the map non-empty and warned five times. Now guards on `JSTL_URL`. To make that reachable from a test I added a package-private `aliasJakartaTagsUris(URL)` overload — `JSTL_URL` is `static final` and modern JDKs reject reflective writes, so stubbing it wasn't an option. The test asserts zero warnings, which is what actually distinguishes the two guards. **`permittedTaglibs` split.** Confirmed — `PermittedTaglibsTLV` uses a single-arg `StringTokenizer`, i.e. any whitespace. Split is now `\s+`; a space-separated list previously came back as one token so the widening silently no-oped. The test helper read it back with the same wrong delimiter, which is why nothing caught it. **Static-initialiser wiring.** Added `staticInitialiserRegistersTheAliases`. Since the shaded jar isn't on the module's test classpath, it asserts over `<clinit>` bytecode that `aliasJakartaTagsUris` is called and that `populateMyfacesAndJstlData` precedes it. **Seven vs five.** Stale — bfc0e16bb0 already cut the map to the five URIs the spec actually renamed (`permittedTaglibs`/`scriptfree` are Apache-Standard-Taglibs TLVs that kept their URIs). The PR description still said seven; I've corrected it. I mutation-tested each fix — reverting the guard, the split, the wiring, and the precedence each makes exactly the corresponding test fail. Module suite is 19/19. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_ -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
