Skip to content

Fix JPMS dependency reads in adapter modules - #98

Open
jmanico wants to merge 1 commit into
mainfrom
fix/jpms-adapter-dependencies
Open

Fix JPMS dependency reads in adapter modules#98
jmanico wants to merge 1 commit into
mainfrom
fix/jpms-adapter-dependencies

Conversation

@jmanico

@jmanico jmanico commented Sep 11, 2026

Copy link
Copy Markdown
Member

Why this issue

Of the open modernization issues (#91-#97), #92 is the highest priority because it is a shipped runtime correctness bug affecting all three optional adapter artifacts. The other issues improve maintenance, release infrastructure, or future compatibility; this one can prevent valid named-module consumers from compiling or linking today.

What changed

  • declare each adapter API dependency with requires transitive
    • owasp.encoder.jsp -> javax.servlet.jsp.api
    • owasp.encoder.jakarta -> jakarta.servlet.jsp
    • owasp.encoder.esapi -> esapi
  • add isolated named-module integration tests that compile and launch consumers requiring only the adapter module
  • exercise real JSP/Jakarta types and a real ESAPI encoding call without --add-reads or --add-opens
  • document exact JPMS module names, Maven coordinates, and automatic-module filename constraints
  • preserve Java 8 bytecode, existing module identities, OSGi metadata, TLDs, and dependency scopes

Validation

  • mvn -B -ntp clean verify (1,150 tests, no failures/errors/skips)
  • named-module adapter tests on JDK 11, 17, 21, and 25
  • affected-module Checkstyle and POM XML validation
  • packaged-JAR checks for module descriptors, Java 8 bytecode, OSGi metadata, TLDs, and exclusion of test harness sources
  • full GitHub Actions build passed, including the Docker/Selenium Jakarta test application

The three javac warnings about transitive automatic modules are expected: the upstream JSP and ESAPI API artifacts are automatic modules, and the README now documents their required filenames.

Fixes #92

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The descriptors, consumer tests, Maven configuration, and documentation consistently address the reported JPMS linkage failures.

Pull request overview

Fixes JPMS readability for all optional adapters and verifies named-module consumers.

Changes:

  • Adds transitive API dependencies to adapter module descriptors.
  • Adds isolated compile-and-launch integration tests.
  • Documents module names, coordinates, and filename constraints.
File summaries
File Description
src/test-support/java/org/owasp/encoder/testing/ModulePathIT.java Adds shared JPMS integration harness.
README.md Documents adapter module dependencies.
jsp/src/test/modules/owasp.encoder.jsp.consumer/org/owasp/encoder/consumer/JspConsumer.java Exercises the JSP adapter.
jsp/src/test/modules/owasp.encoder.jsp.consumer/module-info.java Defines the JSP test consumer.
jsp/src/main/java9/module-info.java Adds transitive JSP API readability.
jsp/pom.xml Configures the JSP integration test.
jakarta/src/test/modules/owasp.encoder.jakarta.consumer/org/owasp/encoder/consumer/JakartaConsumer.java Exercises the Jakarta adapter.
jakarta/src/test/modules/owasp.encoder.jakarta.consumer/module-info.java Defines the Jakarta test consumer.
jakarta/src/main/java9/module-info.java Adds transitive Jakarta JSP readability.
jakarta/pom.xml Configures the Jakarta integration test.
esapi/src/test/modules/owasp.encoder.esapi.consumer/org/owasp/encoder/consumer/EsapiConsumer.java Exercises real ESAPI encoding.
esapi/src/test/modules/owasp.encoder.esapi.consumer/module-info.java Defines the ESAPI test consumer.
esapi/src/main/java9/module-info.java Adds transitive ESAPI readability.
esapi/pom.xml Configures the ESAPI integration test.
Review details
  • Files reviewed: 14/14 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix JPMS dependency reads in the JSP, Jakarta, and ESAPI adapters

2 participants