Skip to content

Commit

Permalink
Copy ServletHolder class/instance properly during startWebapp (#6214)
Browse files Browse the repository at this point in the history
 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>
  • Loading branch information
joakime authored May 16, 2021
1 parent 1c05b0b commit edcaf70
Show file tree
Hide file tree
Showing 5 changed files with 300 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ public String toString()

protected class HolderConfig
{

public ServletContext getServletContext()
{
return getServletHandler().getServletContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,14 @@ public void setForcedPath(String forcedPath)
_forcedPath = forcedPath;
}

private void setClassFrom(ServletHolder holder) throws ServletException
{
if (_servlet != null || getInstance() != null)
throw new IllegalStateException();
this.setClassName(holder.getClassName());
this.setHeldClass(holder.getHeldClass());
}

public boolean isEnabled()
{
return _enabled;
Expand Down Expand Up @@ -325,8 +333,8 @@ public void doStart()
{
if (LOG.isDebugEnabled())
LOG.debug("JSP file {} for {} mapped to Servlet {}", _forcedPath, getName(), jsp.getClassName());
// set the className for this servlet to the precompiled one
setClassName(jsp.getClassName());
// set the className/servlet/instance for this servlet to the precompiled one
setClassFrom(jsp);
}
else
{
Expand All @@ -336,7 +344,7 @@ public void doStart()
{
if (LOG.isDebugEnabled())
LOG.debug("JSP file {} for {} mapped to JspServlet class {}", _forcedPath, getName(), jsp.getClassName());
setClassName(jsp.getClassName());
setClassFrom(jsp);
//copy jsp init params that don't exist for this servlet
for (Map.Entry<String, String> entry : jsp.getInitParameters().entrySet())
{
Expand Down Expand Up @@ -964,7 +972,6 @@ protected void appendPath(StringBuffer path, String element)

protected class Config extends HolderConfig implements ServletConfig
{

@Override
public String getServletName()
{
Expand Down Expand Up @@ -1225,9 +1232,9 @@ public void dump(Appendable out, String indent) throws IOException
@Override
public String toString()
{
return String.format("%s==%s@%x{jsp=%s,order=%d,inst=%b,async=%b,src=%s}",
return String.format("%s==%s@%x{jsp=%s,order=%d,inst=%b,async=%b,src=%s,%s}",
getName(), getClassName(), hashCode(),
_forcedPath, _initOrder, _servlet != null, isAsyncSupported(), getSource());
_forcedPath, _initOrder, _servlet != null, isAsyncSupported(), getSource(), getState());
}

private class UnavailableServlet extends Wrapper
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.webapp;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Iterator;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.servlet.ServletMapping;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.resource.PathResource;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;

public class ForcedServletTest
{
private Server server;
private LocalConnector connector;

@BeforeEach
public void setup() throws Exception
{
server = new Server();
connector = new LocalConnector(server);
server.addConnector(connector);

WebAppContext context = new ForcedWebAppContext();

// Lets setup the Webapp base resource properly
Path basePath = MavenTestingUtils.getTargetTestingPath(ForcedServletTest.class.getName()).resolve("webapp");
FS.ensureEmpty(basePath);
Path srcWebApp = MavenTestingUtils.getProjectDirPath("src/test/webapp-alt-jsp");
copyDir(srcWebApp, basePath);
copyClass(FakePrecompiledJSP.class, basePath.resolve("WEB-INF/classes"));

// Use the new base
context.setWarResource(new PathResource(basePath));

server.setHandler(context);
// server.setDumpAfterStart(true);
server.start();
}

private void copyClass(Class<?> clazz, Path destClasses) throws IOException
{
String classRelativeFilename = clazz.getName().replace('.', '/') + ".class";
Path destFile = destClasses.resolve(classRelativeFilename);
FS.ensureDirExists(destFile.getParent());

Path srcFile = MavenTestingUtils.getTargetPath("test-classes/" + classRelativeFilename);
Files.copy(srcFile, destFile);
}

private void copyDir(Path src, Path dest) throws IOException
{
FS.ensureDirExists(dest);
for (Iterator<Path> it = Files.list(src).iterator(); it.hasNext(); )
{
Path path = it.next();
if (Files.isRegularFile(path))
Files.copy(path, dest.resolve(path.getFileName()));
else if (Files.isDirectory(path))
copyDir(path, dest.resolve(path.getFileName()));
}
}

@AfterEach
public void teardown()
{
LifeCycle.stop(server);
}

/**
* Test access to a jsp resource defined in the web.xml, but doesn't actually exist.
* <p>
* Think of this as a precompiled jsp entry, but the class doesn't exist.
* </p>
*/
@Test
public void testAccessBadDescriptorEntry() throws Exception
{
StringBuilder rawRequest = new StringBuilder();
rawRequest.append("GET /does/not/exist/index.jsp HTTP/1.1\r\n");
rawRequest.append("Host: local\r\n");
rawRequest.append("Connection: close\r\n");
rawRequest.append("\r\n");

String rawResponse = connector.getResponse(rawRequest.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
// Since this was a request to a resource ending in `*.jsp`, the RejectUncompiledJspServlet responded
assertEquals(555, response.getStatus());
}

/**
* Test access of a jsp resource that doesn't exist in the base resource or the descriptor.
*/
@Test
public void testAccessNonExistentEntry() throws Exception
{
StringBuilder rawRequest = new StringBuilder();
rawRequest.append("GET /bogus.jsp HTTP/1.1\r\n");
rawRequest.append("Host: local\r\n");
rawRequest.append("Connection: close\r\n");
rawRequest.append("\r\n");

String rawResponse = connector.getResponse(rawRequest.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
// Since this was a request to a resource ending in `*.jsp`, the RejectUncompiledJspServlet responded
assertEquals(555, response.getStatus());
}

/**
* Test access of a jsp resource that exist in the base resource, but not in the descriptor.
*/
@Test
public void testAccessUncompiledEntry() throws Exception
{
StringBuilder rawRequest = new StringBuilder();
rawRequest.append("GET /hello.jsp HTTP/1.1\r\n");
rawRequest.append("Host: local\r\n");
rawRequest.append("Connection: close\r\n");
rawRequest.append("\r\n");

String rawResponse = connector.getResponse(rawRequest.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
// status code 555 is from RejectUncompiledJspServlet
assertEquals(555, response.getStatus());
}

/**
* Test access of a jsp resource that does not exist in the base resource, but in the descriptor, as a precompiled jsp entry
*/
@Test
public void testPrecompiledEntry() throws Exception
{
StringBuilder rawRequest = new StringBuilder();
rawRequest.append("GET /precompiled/world.jsp HTTP/1.1\r\n");
rawRequest.append("Host: local\r\n");
rawRequest.append("Connection: close\r\n");
rawRequest.append("\r\n");

String rawResponse = connector.getResponse(rawRequest.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertEquals(200, response.getStatus());

String responseBody = response.getContent();
assertThat(responseBody, containsString("This is the FakePrecompiledJSP"));
}

public static class ForcedWebAppContext extends WebAppContext
{
@Override
protected void startWebapp() throws Exception
{
// This will result in a 404 for all requests that don't belong to a more precise servlet
forceServlet("default", ServletHandler.Default404Servlet.class);
addServletMapping("default", "/");

// This will result in any attempt to use an JSP that isn't precompiled and in the descriptor with status code 555
forceServlet("jsp", RejectUncompiledJspServlet.class);
addServletMapping("jsp", "*.jsp");

super.startWebapp();
}

private void addServletMapping(String name, String pathSpec)
{
ServletMapping mapping = new ServletMapping();
mapping.setServletName(name);
mapping.setPathSpec(pathSpec);
getServletHandler().addServletMapping(mapping);
}

private void forceServlet(String name, Class<? extends HttpServlet> servlet) throws Exception
{
ServletHandler handler = getServletHandler();

// Remove existing holder
handler.setServlets(Arrays.stream(handler.getServlets())
.filter(h -> !h.getName().equals(name))
.toArray(ServletHolder[]::new));

// add the forced servlet
ServletHolder holder = new ServletHolder(servlet.getConstructor().newInstance());
holder.setInitOrder(1);
holder.setName(name);
holder.setAsyncSupported(true);
handler.addServlet(holder);
}
}

public static class RejectUncompiledJspServlet extends HttpServlet
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
log(String.format("Uncompiled JSPs not supported by %s", request.getRequestURI()));
response.sendError(555);
}
}

public static class FakeJspServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
resp.setCharacterEncoding("utf-8");
resp.setContentType("text/plain");
resp.setStatus(HttpServletResponse.SC_OK);
resp.getWriter().println("This is the FakeJspServlet");
}
}

public static class FakePrecompiledJSP extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
resp.setCharacterEncoding("utf-8");
resp.setContentType("text/plain");
resp.setStatus(HttpServletResponse.SC_OK);
resp.getWriter().println("This is the FakePrecompiledJSP");
}
}
}
27 changes: 27 additions & 0 deletions jetty-webapp/src/test/webapp-alt-jsp/WEB-INF/web.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="utf-8"?>

<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee
http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"
version="3.1">

<servlet>
<servlet-name>zedName</servlet-name>
<jsp-file>/does/not/exist/index.jsp</jsp-file>
</servlet>
<servlet-mapping>
<servlet-name>zedName</servlet-name>
<url-pattern>/zed</url-pattern>
</servlet-mapping>

<servlet>
<servlet-name>precompiledName</servlet-name>
<servlet-class>org.eclipse.jetty.webapp.ForcedServletTest$FakePrecompiledJSP</servlet-class>
</servlet>
<servlet-mapping>
<servlet-name>precompiledName</servlet-name>
<url-pattern>/precompiled/world.jsp</url-pattern>
</servlet-mapping>

</web-app>
1 change: 1 addition & 0 deletions jetty-webapp/src/test/webapp-alt-jsp/hello.jsp
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This shouldn't be returned

0 comments on commit edcaf70

Please sign in to comment.