Skip to content

Commit edcaf70

Browse files
joakimejoakime
authored
Copy ServletHolder class/instance properly during startWebapp (#6214)
 ServletHolder.copyClassServlet() method added to correctly copy held class. Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> Signed-off-by: Greg Wilkins <gregw@webtide.com>
1 parent 1c05b0b commit edcaf70

File tree

5 files changed

+300
-7
lines changed

5 files changed

+300
-7
lines changed

‎jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Holder.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,6 @@ public String toString()
187187

188188
protected class HolderConfig
189189
{
190-
191190
public ServletContext getServletContext()
192191
{
193192
return getServletHandler().getServletContext();

‎jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,14 @@ public void setForcedPath(String forcedPath)
294294
_forcedPath = forcedPath;
295295
}
296296

297+
private void setClassFrom(ServletHolder holder) throws ServletException
298+
{
299+
if (_servlet != null || getInstance() != null)
300+
throw new IllegalStateException();
301+
this.setClassName(holder.getClassName());
302+
this.setHeldClass(holder.getHeldClass());
303+
}
304+
297305
public boolean isEnabled()
298306
{
299307
return _enabled;
@@ -325,8 +333,8 @@ public void doStart()
325333
{
326334
if (LOG.isDebugEnabled())
327335
LOG.debug("JSP file {} for {} mapped to Servlet {}", _forcedPath, getName(), jsp.getClassName());
328-
// set the className for this servlet to the precompiled one
329-
setClassName(jsp.getClassName());
336+
// set the className/servlet/instance for this servlet to the precompiled one
337+
setClassFrom(jsp);
330338
}
331339
else
332340
{
@@ -336,7 +344,7 @@ public void doStart()
336344
{
337345
if (LOG.isDebugEnabled())
338346
LOG.debug("JSP file {} for {} mapped to JspServlet class {}", _forcedPath, getName(), jsp.getClassName());
339-
setClassName(jsp.getClassName());
347+
setClassFrom(jsp);
340348
//copy jsp init params that don't exist for this servlet
341349
for (Map.Entry<String, String> entry : jsp.getInitParameters().entrySet())
342350
{
@@ -964,7 +972,6 @@ protected void appendPath(StringBuffer path, String element)
964972

965973
protected class Config extends HolderConfig implements ServletConfig
966974
{
967-
968975
@Override
969976
public String getServletName()
970977
{
@@ -1225,9 +1232,9 @@ public void dump(Appendable out, String indent) throws IOException
12251232
@Override
12261233
public String toString()
12271234
{
1228-
return String.format("%s==%s@%x{jsp=%s,order=%d,inst=%b,async=%b,src=%s}",
1235+
return String.format("%s==%s@%x{jsp=%s,order=%d,inst=%b,async=%b,src=%s,%s}",
12291236
getName(), getClassName(), hashCode(),
1230-
_forcedPath, _initOrder, _servlet != null, isAsyncSupported(), getSource());
1237+
_forcedPath, _initOrder, _servlet != null, isAsyncSupported(), getSource(), getState());
12311238
}
12321239

12331240
private class UnavailableServlet extends Wrapper

‎jetty-webapp/src/test/java/org/eclipse/jetty/webapp/ForcedServletTest.java

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
//
2+
// ========================================================================
3+
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
4+
// ------------------------------------------------------------------------
5+
// All rights reserved. This program and the accompanying materials
6+
// are made available under the terms of the Eclipse Public License v1.0
7+
// and Apache License v2.0 which accompanies this distribution.
8+
//
9+
// The Eclipse Public License is available at
10+
// http://www.eclipse.org/legal/epl-v10.html
11+
//
12+
// The Apache License v2.0 is available at
13+
// http://www.opensource.org/licenses/apache2.0.php
14+
//
15+
// You may elect to redistribute this code under either of these licenses.
16+
// ========================================================================
17+
//
18+
19+
package org.eclipse.jetty.webapp;
20+
21+
import java.io.IOException;
22+
import java.nio.file.Files;
23+
import java.nio.file.Path;
24+
import java.util.Arrays;
25+
import java.util.Iterator;
26+
import javax.servlet.ServletException;
27+
import javax.servlet.http.HttpServlet;
28+
import javax.servlet.http.HttpServletRequest;
29+
import javax.servlet.http.HttpServletResponse;
30+
31+
import org.eclipse.jetty.http.HttpTester;
32+
import org.eclipse.jetty.server.LocalConnector;
33+
import org.eclipse.jetty.server.Server;
34+
import org.eclipse.jetty.servlet.ServletHandler;
35+
import org.eclipse.jetty.servlet.ServletHolder;
36+
import org.eclipse.jetty.servlet.ServletMapping;
37+
import org.eclipse.jetty.toolchain.test.FS;
38+
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
39+
import org.eclipse.jetty.util.component.LifeCycle;
40+
import org.eclipse.jetty.util.resource.PathResource;
41+
import org.junit.jupiter.api.AfterEach;
42+
import org.junit.jupiter.api.BeforeEach;
43+
import org.junit.jupiter.api.Test;
44+
45+
import static org.hamcrest.MatcherAssert.assertThat;
46+
import static org.hamcrest.Matchers.containsString;
47+
import static org.junit.jupiter.api.Assertions.assertEquals;
48+
49+
public class ForcedServletTest
50+
{
51+
private Server server;
52+
private LocalConnector connector;
53+
54+
@BeforeEach
55+
public void setup() throws Exception
56+
{
57+
server = new Server();
58+
connector = new LocalConnector(server);
59+
server.addConnector(connector);
60+
61+
WebAppContext context = new ForcedWebAppContext();
62+
63+
// Lets setup the Webapp base resource properly
64+
Path basePath = MavenTestingUtils.getTargetTestingPath(ForcedServletTest.class.getName()).resolve("webapp");
65+
FS.ensureEmpty(basePath);
66+
Path srcWebApp = MavenTestingUtils.getProjectDirPath("src/test/webapp-alt-jsp");
67+
copyDir(srcWebApp, basePath);
68+
copyClass(FakePrecompiledJSP.class, basePath.resolve("WEB-INF/classes"));
69+
70+
// Use the new base
71+
context.setWarResource(new PathResource(basePath));
72+
73+
server.setHandler(context);
74+
// server.setDumpAfterStart(true);
75+
server.start();
76+
}
77+
78+
private void copyClass(Class<?> clazz, Path destClasses) throws IOException
79+
{
80+
String classRelativeFilename = clazz.getName().replace('.', '/') + ".class";
81+
Path destFile = destClasses.resolve(classRelativeFilename);
82+
FS.ensureDirExists(destFile.getParent());
83+
84+
Path srcFile = MavenTestingUtils.getTargetPath("test-classes/" + classRelativeFilename);
85+
Files.copy(srcFile, destFile);
86+
}
87+
88+
private void copyDir(Path src, Path dest) throws IOException
89+
{
90+
FS.ensureDirExists(dest);
91+
for (Iterator<Path> it = Files.list(src).iterator(); it.hasNext(); )
92+
{
93+
Path path = it.next();
94+
if (Files.isRegularFile(path))
95+
Files.copy(path, dest.resolve(path.getFileName()));
96+
else if (Files.isDirectory(path))
97+
copyDir(path, dest.resolve(path.getFileName()));
98+
}
99+
}
100+
101+
@AfterEach
102+
public void teardown()
103+
{
104+
LifeCycle.stop(server);
105+
}
106+
107+
/**
108+
* Test access to a jsp resource defined in the web.xml, but doesn't actually exist.
109+
* <p>
110+
* Think of this as a precompiled jsp entry, but the class doesn't exist.
111+
* </p>
112+
*/
113+
@Test
114+
public void testAccessBadDescriptorEntry() throws Exception
115+
{
116+
StringBuilder rawRequest = new StringBuilder();
117+
rawRequest.append("GET /does/not/exist/index.jsp HTTP/1.1\r\n");
118+
rawRequest.append("Host: local\r\n");
119+
rawRequest.append("Connection: close\r\n");
120+
rawRequest.append("\r\n");
121+
122+
String rawResponse = connector.getResponse(rawRequest.toString());
123+
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
124+
// Since this was a request to a resource ending in `*.jsp`, the RejectUncompiledJspServlet responded
125+
assertEquals(555, response.getStatus());
126+
}
127+
128+
/**
129+
* Test access of a jsp resource that doesn't exist in the base resource or the descriptor.
130+
*/
131+
@Test
132+
public void testAccessNonExistentEntry() throws Exception
133+
{
134+
StringBuilder rawRequest = new StringBuilder();
135+
rawRequest.append("GET /bogus.jsp HTTP/1.1\r\n");
136+
rawRequest.append("Host: local\r\n");
137+
rawRequest.append("Connection: close\r\n");
138+
rawRequest.append("\r\n");
139+
140+
String rawResponse = connector.getResponse(rawRequest.toString());
141+
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
142+
// Since this was a request to a resource ending in `*.jsp`, the RejectUncompiledJspServlet responded
143+
assertEquals(555, response.getStatus());
144+
}
145+
146+
/**
147+
* Test access of a jsp resource that exist in the base resource, but not in the descriptor.
148+
*/
149+
@Test
150+
public void testAccessUncompiledEntry() throws Exception
151+
{
152+
StringBuilder rawRequest = new StringBuilder();
153+
rawRequest.append("GET /hello.jsp HTTP/1.1\r\n");
154+
rawRequest.append("Host: local\r\n");
155+
rawRequest.append("Connection: close\r\n");
156+
rawRequest.append("\r\n");
157+
158+
String rawResponse = connector.getResponse(rawRequest.toString());
159+
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
160+
// status code 555 is from RejectUncompiledJspServlet
161+
assertEquals(555, response.getStatus());
162+
}
163+
164+
/**
165+
* Test access of a jsp resource that does not exist in the base resource, but in the descriptor, as a precompiled jsp entry
166+
*/
167+
@Test
168+
public void testPrecompiledEntry() throws Exception
169+
{
170+
StringBuilder rawRequest = new StringBuilder();
171+
rawRequest.append("GET /precompiled/world.jsp HTTP/1.1\r\n");
172+
rawRequest.append("Host: local\r\n");
173+
rawRequest.append("Connection: close\r\n");
174+
rawRequest.append("\r\n");
175+
176+
String rawResponse = connector.getResponse(rawRequest.toString());
177+
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
178+
assertEquals(200, response.getStatus());
179+
180+
String responseBody = response.getContent();
181+
assertThat(responseBody, containsString("This is the FakePrecompiledJSP"));
182+
}
183+
184+
public static class ForcedWebAppContext extends WebAppContext
185+
{
186+
@Override
187+
protected void startWebapp() throws Exception
188+
{
189+
// This will result in a 404 for all requests that don't belong to a more precise servlet
190+
forceServlet("default", ServletHandler.Default404Servlet.class);
191+
addServletMapping("default", "/");
192+
193+
// This will result in any attempt to use an JSP that isn't precompiled and in the descriptor with status code 555
194+
forceServlet("jsp", RejectUncompiledJspServlet.class);
195+
addServletMapping("jsp", "*.jsp");
196+
197+
super.startWebapp();
198+
}
199+
200+
private void addServletMapping(String name, String pathSpec)
201+
{
202+
ServletMapping mapping = new ServletMapping();
203+
mapping.setServletName(name);
204+
mapping.setPathSpec(pathSpec);
205+
getServletHandler().addServletMapping(mapping);
206+
}
207+
208+
private void forceServlet(String name, Class<? extends HttpServlet> servlet) throws Exception
209+
{
210+
ServletHandler handler = getServletHandler();
211+
212+
// Remove existing holder
213+
handler.setServlets(Arrays.stream(handler.getServlets())
214+
.filter(h -> !h.getName().equals(name))
215+
.toArray(ServletHolder[]::new));
216+
217+
// add the forced servlet
218+
ServletHolder holder = new ServletHolder(servlet.getConstructor().newInstance());
219+
holder.setInitOrder(1);
220+
holder.setName(name);
221+
holder.setAsyncSupported(true);
222+
handler.addServlet(holder);
223+
}
224+
}
225+
226+
public static class RejectUncompiledJspServlet extends HttpServlet
227+
{
228+
@Override
229+
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
230+
{
231+
log(String.format("Uncompiled JSPs not supported by %s", request.getRequestURI()));
232+
response.sendError(555);
233+
}
234+
}
235+
236+
public static class FakeJspServlet extends HttpServlet
237+
{
238+
@Override
239+
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
240+
{
241+
resp.setCharacterEncoding("utf-8");
242+
resp.setContentType("text/plain");
243+
resp.setStatus(HttpServletResponse.SC_OK);
244+
resp.getWriter().println("This is the FakeJspServlet");
245+
}
246+
}
247+
248+
public static class FakePrecompiledJSP extends HttpServlet
249+
{
250+
@Override
251+
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
252+
{
253+
resp.setCharacterEncoding("utf-8");
254+
resp.setContentType("text/plain");
255+
resp.setStatus(HttpServletResponse.SC_OK);
256+
resp.getWriter().println("This is the FakePrecompiledJSP");
257+
}
258+
}
259+
}

‎jetty-webapp/src/test/webapp-alt-jsp/WEB-INF/web.xml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
3+
<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee"
4+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5+
xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee
6+
http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"
7+
version="3.1">
8+
9+
<servlet>
10+
<servlet-name>zedName</servlet-name>
11+
<jsp-file>/does/not/exist/index.jsp</jsp-file>
12+
</servlet>
13+
<servlet-mapping>
14+
<servlet-name>zedName</servlet-name>
15+
<url-pattern>/zed</url-pattern>
16+
</servlet-mapping>
17+
18+
<servlet>
19+
<servlet-name>precompiledName</servlet-name>
20+
<servlet-class>org.eclipse.jetty.webapp.ForcedServletTest$FakePrecompiledJSP</servlet-class>
21+
</servlet>
22+
<servlet-mapping>
23+
<servlet-name>precompiledName</servlet-name>
24+
<url-pattern>/precompiled/world.jsp</url-pattern>
25+
</servlet-mapping>
26+
27+
</web-app>

‎jetty-webapp/src/test/webapp-alt-jsp/hello.jsp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
This shouldn't be returned

0 commit comments

Comments
 (0)