Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OF-2945: Prevent stack traces when pre-compiling JSP pages #2646

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

guusdk
Copy link
Member

@guusdk guusdk commented Jan 4, 2025

After upgrading to Jetty 12, we also updated the Jetty plugin that performs precompilation of the JPS pages that make up the admin console. We are now using jetty-ee8-jspc-maven

Since this update, the build process is spewing out rather long stack traces, related to missing JAR files. The stack traces can be seen in https://igniterealtime.atlassian.net/browse/OF-2945

We have not noticed any failing functional behavior because of this. Nonetheless, the stack traces are annoying, as they suggest that something is wrong.

The missing JAR files seem to relate to the Xalan project, which is not a dependency of Openfire. It is, however, a dependency of Jetty's JSPC plugin.

On a hunch, I've excluded that dependency from the plugin during our build. That makes the stack trace go away. JSP compilation appears to work just fine with this change.

After upgrading to Jetty 12, we also updated the Jetty plugin that performs precompilation of the JPS pages that make up the admin console. We are now using jetty-ee8-jspc-maven

Since this update, the build process is spewing out rather long stack traces, related to missing JAR files. The stack traces can be seen in https://igniterealtime.atlassian.net/browse/OF-2945

We have not noticed any failing functional behavior because of this. Nonetheless, the stack traces are annoying, as they suggest that something is wrong.

The missing JAR files seem to relate to the Xalan project, which is not a dependency of Openfire. It is, however, a dependency of Jetty's JSPC plugin.

On a hunch, I've excluded that dependency from the plugin during our build. That makes the stack trace go away. JSP compilation appears to work just fine with this change.
@guusdk
Copy link
Member Author

guusdk commented Jan 4, 2025

I must admit that I'm not sure what the cause of this problem, or the impact of my work-around is. @joakime, if you have a minute to have a look: I'm wondering if we're not using this plugin properly, or if it perhaps has a bit of a bug.

@joakime
Copy link

joakime commented Jan 6, 2025

First, the fact that these files do not exist is problematic for Maven and Glassfish JSTL.
Can you correct that first?

That being said, where is the xalan coming from?
Looking at the dependency:tree for jetty-ee8-jspc-maven-plugin ...

[INFO] --- dependency:3.8.1:tree (default-cli) @ jetty-ee8-jspc-maven-plugin ---
[INFO] org.eclipse.jetty.ee8:jetty-ee8-jspc-maven-plugin:maven-plugin:12.0.16
[INFO] +- org.apache.ant:ant:jar:1.10.15:compile
[INFO] |  \- org.apache.ant:ant-launcher:jar:1.10.15:compile
[INFO] +- org.apache.maven.plugin-tools:maven-plugin-tools-api:jar:3.15.1:compile
[INFO] |  +- org.slf4j:slf4j-api:jar:2.0.16:compile
[INFO] |  +- org.apache.maven.reporting:maven-reporting-api:jar:4.0.0:compile
[INFO] |  |  \- org.apache.maven.doxia:doxia-sink-api:jar:2.0.0:compile
[INFO] |  +- org.apache.maven:maven-settings:jar:3.9.9:compile
[INFO] |  +- org.apache.maven.resolver:maven-resolver-api:jar:1.9.22:compile
[INFO] |  +- org.codehaus.plexus:plexus-classworlds:jar:2.8.0:compile
[INFO] |  +- org.eclipse.sisu:org.eclipse.sisu.plexus:jar:0.9.0.M3:compile
[INFO] |  +- org.codehaus.plexus:plexus-utils:jar:4.0.2:compile
[INFO] |  +- org.codehaus.plexus:plexus-xml:jar:4.0.4:compile
[INFO] |  |  \- org.apache.maven:maven-xml-impl:jar:4.0.0-alpha-9:compile
[INFO] |  |     +- org.apache.maven:maven-api-xml:jar:4.0.0-alpha-9:compile
[INFO] |  |     |  \- org.apache.maven:maven-api-meta:jar:4.0.0-alpha-9:compile
[INFO] |  |     \- com.fasterxml.woodstox:woodstox-core:jar:6.5.1:compile
[INFO] |  |        \- org.codehaus.woodstox:stax2-api:jar:4.2.1:compile
[INFO] |  +- org.apache.httpcomponents:httpclient:jar:4.5.14:compile
[INFO] |  |  +- commons-logging:commons-logging:jar:1.2:compile
[INFO] |  |  \- commons-codec:commons-codec:jar:1.17.1:compile
[INFO] |  +- org.apache.httpcomponents:httpcore:jar:4.4.16:compile
[INFO] |  +- org.apache.maven.wagon:wagon-provider-api:jar:3.5.3:compile
[INFO] |  \- org.codehaus.plexus:plexus-java:jar:1.3.0:compile
[INFO] |     +- org.ow2.asm:asm:jar:9.7.1:compile
[INFO] |     \- com.thoughtworks.qdox:qdox:jar:2.1.0:compile
[INFO] +- org.eclipse.jetty:jetty-util:jar:12.0.16:compile
[INFO] +- org.eclipse.jetty.ee8:jetty-ee8-apache-jsp:jar:12.0.16:compile
[INFO] |  +- org.eclipse.jetty.toolchain:jetty-servlet-api:jar:4.0.6:compile
[INFO] |  \- org.mortbay.jasper:apache-jsp:jar:9.0.96:compile
[INFO] |     +- org.eclipse.jdt:ecj:jar:3.38.0:compile
[INFO] |     \- org.mortbay.jasper:apache-el:jar:9.0.96:compile
[INFO] +- org.eclipse.jetty.ee8:jetty-ee8-glassfish-jstl:jar:12.0.16:compile
[INFO] |  +- jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api:jar:1.2.7:compile
[INFO] |  \- org.glassfish.web:javax.servlet.jsp.jstl:jar:1.2.5:compile
[INFO] |     \- xalan:xalan:jar:2.7.3:compile
[INFO] +- org.apache.maven:maven-artifact:jar:3.9.9:provided
[INFO] +- org.apache.maven:maven-core:jar:3.9.9:provided
[INFO] |  +- org.apache.maven:maven-settings-builder:jar:3.9.9:provided
[INFO] |  |  \- org.codehaus.plexus:plexus-sec-dispatcher:jar:2.0:provided
[INFO] |  |     \- org.codehaus.plexus:plexus-cipher:jar:2.0:provided
[INFO] |  +- org.apache.maven:maven-builder-support:jar:3.9.9:provided
[INFO] |  +- org.apache.maven:maven-repository-metadata:jar:3.9.9:provided
[INFO] |  +- org.apache.maven:maven-model-builder:jar:3.9.9:provided
[INFO] |  +- org.apache.maven:maven-resolver-provider:jar:3.9.9:provided
[INFO] |  +- org.apache.maven.resolver:maven-resolver-impl:jar:1.9.22:provided
[INFO] |  |  \- org.apache.maven.resolver:maven-resolver-named-locks:jar:1.9.22:provided
[INFO] |  +- org.apache.maven.resolver:maven-resolver-spi:jar:1.9.22:provided
[INFO] |  +- org.apache.maven.resolver:maven-resolver-util:jar:1.9.22:provided
[INFO] |  +- org.apache.maven.shared:maven-shared-utils:jar:3.4.2:provided
[INFO] |  +- org.eclipse.sisu:org.eclipse.sisu.inject:jar:0.9.0.M3:compile
[INFO] |  +- com.google.inject:guice:jar:7.0.0:provided
[INFO] |  |  +- jakarta.inject:jakarta.inject-api:jar:2.0.1:provided
[INFO] |  |  \- aopalliance:aopalliance:jar:1.0:provided
[INFO] |  +- com.google.guava:guava:jar:33.3.1-jre:provided
[INFO] |  +- com.google.guava:failureaccess:jar:1.0.2:provided
[INFO] |  +- org.codehaus.plexus:plexus-interpolation:jar:1.27:provided
[INFO] |  \- org.codehaus.plexus:plexus-component-annotations:jar:2.2.0:compile
[INFO] +- org.apache.maven:maven-model:jar:3.9.9:provided
[INFO] +- org.apache.maven:maven-plugin-api:jar:3.9.9:provided
[INFO] +- org.apache.maven.plugin-tools:maven-plugin-annotations:jar:3.15.1:provided
[INFO] +- org.eclipse.jetty.toolchain:jetty-test-helper:jar:6.3:test
[INFO] |  +- org.hamcrest:hamcrest:jar:3.0:test
[INFO] |  \- org.junit.platform:junit-platform-commons:jar:1.11.3:test
[INFO] |     \- org.apiguardian:apiguardian-api:jar:1.1.2:test
[INFO] \- org.junit.jupiter:junit-jupiter:jar:5.11.3:test
[INFO]    +- org.junit.jupiter:junit-jupiter-api:jar:5.11.3:test
[INFO]    |  \- org.opentest4j:opentest4j:jar:1.3.0:test
[INFO]    +- org.junit.jupiter:junit-jupiter-params:jar:5.11.3:test
[INFO]    \- org.junit.jupiter:junit-jupiter-engine:jar:5.11.3:test
[INFO]       \- org.junit.platform:junit-platform-engine:jar:1.11.3:test

Looks like the following entry is the culprit ...

[INFO] +- org.eclipse.jetty.ee8:jetty-ee8-glassfish-jstl:jar:12.0.16:compile
[INFO] |  +- jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api:jar:1.2.7:compile
[INFO] |  \- org.glassfish.web:javax.servlet.jsp.jstl:jar:1.2.5:compile
[INFO] |     \- xalan:xalan:jar:2.7.3:compile

Can you make a dependencyManagement that excludes xalan from jetty-ee8-glassfish-jstl and try again?

  <dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>org.eclipse.jetty.ee8</groupId>
        <artifactId>jetty-ee8-glassfish-jstl</artifactId>
        <version>12.0.16</version>
        <exclusions>
          <exclusion>
            <groupId>xalan</groupId>
            <artifactId>xalan</artifactId>
          </exclusion>
        </exclusions>
      </dependency>
    </dependencies>
  </dependencyManagement>

@guusdk
Copy link
Member Author

guusdk commented Jan 6, 2025

Thanks for looking into this, @joakime.

First, the fact that these files do not exist is problematic for Maven and Glassfish JSTL.
Can you correct that first?

I can probably do that on my local computer, but this appears to be an issue for every computer that runs this build (and doesn't happen to have the files locally yet). Isn't this what Maven is supposed to do: download the files when/if they're needed? I guess I could add xalan as a dependency to our project to force Maven to download it, but as we don't use it, I'd prefer not to.

Can you make a dependencyManagement that excludes xalan from jetty-ee8-glassfish-jstl and try again?

Instead of going through dependencyManagement, I excluded xalan on the dependency directly (as you can see in the changeset of this PR). That stops the stack traces from being logged while building. I think that's basically the same test (or do you want me to use dependencyManagement explicitly?

@joakime
Copy link

joakime commented Jan 6, 2025

Thanks for looking into this, @joakime.

First, the fact that these files do not exist is problematic for Maven and Glassfish JSTL.
Can you correct that first?

I can probably do that on my local computer, but this appears to be an issue for every computer that runs this build (and doesn't happen to have the files locally yet). Isn't this what Maven is supposed to do: download the files when/if they're needed? I guess I could add xalan as a dependency to our project to force Maven to download it, but as we don't use it, I'd prefer not to.

I wouldn't add xalan.
Exclude it.
It's not needed on JDK 17+

Can you make a dependencyManagement that excludes xalan from jetty-ee8-glassfish-jstl and try again?

Instead of going through dependencyManagement, I excluded xalan on the dependency directly (as you can see in the changeset of this PR). That stops the stack traces from being logged while building. I think that's basically the same test (or do you want me to use dependencyManagement explicitly?

Nah, that works too.

I've gone ahead and added a new issue (and PR) to remove xalan from the ee8 tree (it's already excluded in the ee9/ee10/ee11 trees)

@joakime
Copy link

joakime commented Jan 6, 2025

I can probably do that on my local computer, but this appears to be an issue for every computer that runs this build (and doesn't happen to have the files locally yet). Isn't this what Maven is supposed to do: download the files when/if they're needed? I guess I could add xalan as a dependency to our project to force Maven to download it, but as we don't use it, I'd prefer not to.

Do you happen to have a security plugin that rejects/bans xalan use in your maven project?
That would be a possible reason why xalan doesn't exist on your local repository.

@guusdk
Copy link
Member Author

guusdk commented Jan 6, 2025

Do you happen to have a security plugin that rejects/bans xalan use in your maven project?
That would be a possible reason why xalan doesn't exist on your local repository.

I don't believe we have such configuration in our project. We don't have the text xalan anywhere in our source.

To check if this problem is related to Openfire, I created a minimal project that does JSPC compliation of a hello-world JSP. It shows the exact same problem: https://github.com/guusdk/jspctest

I don't think this problem is specific to Openfire. I think it's a problem for everyone that uses this plugin, that does not happen to have those dependencies already in their local repository.

Can you remove those three files in your local repository, and try on your end?

@guusdk
Copy link
Member Author

guusdk commented Jan 6, 2025

I don't think this is an issue with my computer. I've added a GitHub workflow that causes that jspctest project to build on GitHub hardware: it show's the exact same stack traces during build: https://github.com/guusdk/jspctest/actions/runs/12639158708/job/35216882686

@guusdk
Copy link
Member Author

guusdk commented Jan 6, 2025

Oh, I only now see your larger comment. From that, I take it that excluding the dependency (as done in this PR) is safe. For later versions of Jetty's JSPC plugin, it won't be necessary anymore.

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.

2 participants