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.

Sign up for GitHub

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

HBASE-28070 Replace javax.servlet.jsp dependency with tomcat-jasper #5607

Merged
merged 4 commits into from
Sep 10, 2024
Merged

HBASE-28070 Replace javax.servlet.jsp dependency with tomcat-jasper #5607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8378368
HBASE-28070 Replace javax.servlet.jsp dependency with tomcat-jasper
NihalJain Jan 6, 2024
d43f8b9
Fix UT failure
NihalJain Jan 7, 2024
f337f39
Remove redundant excludes
NihalJain Jan 7, 2024
b32f263
resolve conflict, address review comments and bump tomcat-jasper to l…
NihalJain Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions hbase-assembly/src/main/assembly/client.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@
<exclude>io.opentelemetry.javaagent:*</exclude>
<exclude>org.hamcrest:hamcrest-core</exclude>
<exclude>org.mockito:mockito-core</exclude>
<!-- Exclude transitive dependencies of tomcat-jasper, not needed at runtime -->
NihalJain marked this conversation as resolved.
Show resolved Hide resolved
<exclude>org.apache.tomcat:tomcat-juli</exclude>
<exclude>org.apache.tomcat:tomcat-api</exclude>
<exclude>org.apache.tomcat:tomcat-util-scan</exclude>
<exclude>org.apache.tomcat:tomcat-util</exclude>
</excludes>
</dependencySet>
</dependencySets>
Expand Down
5 changes: 5 additions & 0 deletions hbase-assembly/src/main/assembly/hadoop-three-compat.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@
<exclude>junit:junit</exclude>
<exclude>org.hamcrest:hamcrest-core</exclude>
<exclude>org.mockito:mockito-core</exclude>
<!-- Exclude transitive dependencies of tomcat-jasper, not needed at runtime -->
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not need this for our release binaries? For client I think it is OK that we do not need to start a web server, but for hbase we do need to start master/rs webs?

Copy link
Contributor Author

@NihalJain NihalJain Jan 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I tested out master startup in local mode. Was able to start with just tomcat-jasper, tomcat-jasper-el and tomcat-el-api and hence removed others. Also these 3 jars bring in all those classes which are getting removed from classpath due to removal of javax.el and javax.servlet.jsp.

Can keep these for safer side. Please let me know WDYT.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then we could just exclude them in the dependencyManagement or dependencies section in our pom file? We need to them in tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes @Apache9, these are required at compile time to generate the jsp pages but i was able to load web pages w/o them during runtime. Hence excluding from assembly.
Few others which were not needed at all, I have already excluded in dependencies section for the dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gentle ping @Apache9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @Apache9 seems someone is interested in this patch. Could you please have another look at the changes and the above discussion and see if we are good here. Please let me know if otherwise.

<exclude>org.apache.tomcat:tomcat-juli</exclude>
<exclude>org.apache.tomcat:tomcat-api</exclude>
<exclude>org.apache.tomcat:tomcat-util-scan</exclude>
<exclude>org.apache.tomcat:tomcat-util</exclude>
</excludes>
</dependencySet>
</dependencySets>
Expand Down
39 changes: 0 additions & 39 deletions hbase-resource-bundle/src/main/resources/supplemental-models.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,6 @@ under the License.
</licenses>
</project>
</supplement>
<supplement>
<project>
<groupId>org.glassfish</groupId>
<artifactId>javax.el</artifactId>
<licenses>
<license>
<name>CDDL 1.1</name>
<url>https://glassfish.java.net/public/CDDL+GPL_1_1.html</url>
<distribution>repo</distribution>
</license>
</licenses>
</project>
</supplement>
<supplement>
<project>
<groupId>org.glassfish.hk2</groupId>
Expand Down Expand Up @@ -264,32 +251,6 @@ under the License.
</licenses>
</project>
</supplement>
<supplement>
<project>
<groupId>org.glassfish.web</groupId>
<artifactId>javax.servlet.jsp</artifactId>
<licenses>
<license>
<name>CDDL 1.1</name>
<url>https://glassfish.java.net/public/CDDL+GPL_1_1.html</url>
<distribution>repo</distribution>
</license>
</licenses>
</project>
</supplement>
<supplement>
<project>
<groupId>org.glassfish.web</groupId>
<artifactId>javax.servlet.jsp.jstl</artifactId>
<licenses>
<license>
<name>CDDL 1.1</name>
<url>https://glassfish.java.net/public/CDDL+GPL_1_1.html</url>
<distribution>repo</distribution>
</license>
</licenses>
</project>
</supplement>
<supplement>
<project>
<groupId>org.bouncycastle</groupId>
Expand Down
11 changes: 2 additions & 9 deletions hbase-rest/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,8 @@
</dependency>
<dependency>
<!--For JspC used in ant task-->
<groupId>org.glassfish.web</groupId>
<artifactId>javax.servlet.jsp</artifactId>
</dependency>
<!-- Specifically needed for jetty-jsp, included
to bypass version scanning that hits a bad repo
see HBASE-18831 -->
<dependency>
<groupId>org.glassfish</groupId>
<artifactId>javax.el</artifactId>
<groupId>org.apache.tomcat</groupId>
<artifactId>tomcat-jasper</artifactId>
</dependency>
<dependency>
<groupId>org.apache.kerby</groupId>
Expand Down
4 changes: 2 additions & 2 deletions hbase-server/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@
<!-- For JspC used in ant task, then needed at compile /runtime
because the source code made from the JSP refers to its runtime
-->
<groupId>org.glassfish.web</groupId>
<artifactId>javax.servlet.jsp</artifactId>
<groupId>org.apache.tomcat</groupId>
<artifactId>tomcat-jasper</artifactId>
</dependency>
<!-- Also used by generated sources from our JSP -->
<dependency>
Expand Down
4 changes: 2 additions & 2 deletions hbase-shaded/hbase-shaded-mapreduce/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@
<artifactId>jetty-util-ajax</artifactId>
</exclusion>
<exclusion>
<groupId>org.glassfish</groupId>
<artifactId>javax.el</artifactId>
<groupId>org.apache.tomcat</groupId>
<artifactId>tomcat-jasper</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
Expand Down
11 changes: 2 additions & 9 deletions hbase-thrift/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,8 @@
</dependency>
<dependency>
<!--For JspC used in ant task-->
<groupId>org.glassfish.web</groupId>
<artifactId>javax.servlet.jsp</artifactId>
</dependency>
<!-- Specifically needed for jetty-jsp, included
to bypass version scanning that hits a bad repo
see HBASE-18831 -->
<dependency>
<groupId>org.glassfish</groupId>
<artifactId>javax.el</artifactId>
<groupId>org.apache.tomcat</groupId>
<artifactId>tomcat-jasper</artifactId>
</dependency>
<dependency>
<groupId>org.apache.kerby</groupId>
Expand Down
28 changes: 18 additions & 10 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,8 +857,7 @@
<jaxb-api.version>2.3.1</jaxb-api.version>
<servlet.api.version>3.1.0</servlet.api.version>
<wx.rs.api.version>2.1.1</wx.rs.api.version>
<glassfish.jsp.version>2.3.2</glassfish.jsp.version>
<glassfish.el.version>3.0.1-b08</glassfish.el.version>
<tomcat.jasper.version>9.0.93</tomcat.jasper.version>
<jruby.version>9.4.8.0</jruby.version>
<junit.version>4.13.2</junit.version>
<hamcrest.version>1.3</hamcrest.version>
Expand Down Expand Up @@ -1566,21 +1565,30 @@
</dependency>
<dependency>
<!--This lib has JspC in it. Needed precompiling jsps in hbase-rest, etc.-->
<groupId>org.glassfish.web</groupId>
<artifactId>javax.servlet.jsp</artifactId>
<version>${glassfish.jsp.version}</version>
<groupId>org.apache.tomcat</groupId>
chrajeshbabu marked this conversation as resolved.
Show resolved Hide resolved
<artifactId>tomcat-jasper</artifactId>
<version>${tomcat.jasper.version}</version>
<exclusions>
<exclusion>
<groupId>org.eclipse.jdt</groupId>
<artifactId>ecj</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.tomcat</groupId>
<artifactId>tomcat-servlet-api</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.tomcat</groupId>
<artifactId>tomcat-jsp-api</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<!-- this lib is used by the compiled Jsp from the above JspC -->
<groupId>javax.servlet.jsp</groupId>
<artifactId>javax.servlet.jsp-api</artifactId>
<version>2.3.1</version>
</dependency>
<dependency>
<groupId>org.glassfish</groupId>
<artifactId>javax.el</artifactId>
<version>${glassfish.el.version}</version>
</dependency>
<dependency>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
Expand Down