Fix #8442 server propagation

Test setServer propagation
protect from inadvertent null setting
This commit is contained in:
Greg Wilkins 2022-08-11 09:05:12 +10:00
parent 842af3ed22
commit a0d6f42e73
6 changed files with 83 additions and 22 deletions

View File

@ -535,19 +535,10 @@ public interface Handler extends LifeCycle, Destroyable, Invocable
{
Handler next = getHandler();
if (next == null)
return List.of();
return Collections.emptyList();
return List.of(next);
}
@Override
public void setServer(Server server)
{
super.setServer(server);
Handler next = getHandler();
if (next != null)
next.setServer(getServer());
}
@Override
public Request.Processor handle(Request request) throws Exception
{
@ -632,7 +623,8 @@ public interface Handler extends LifeCycle, Destroyable, Invocable
server.getInvocationType() != Invocable.combine(server.getInvocationType(), handler.getInvocationType()))
throw new IllegalArgumentException("Cannot change invocation type of started server");
handler.setServer(getServer());
if (server != null)
handler.setServer(server);
}
updateBeans(_handlers, handlers);

View File

@ -58,7 +58,6 @@ public class ResourceListing
base = URIUtil.normalizePath(base);
if (base == null || !resource.isDirectory())
return null;
Path path = resource.getPath();
if (path == null) // Should never happen, as new Resource contract is that all Resources are a Path.
return null;

View File

@ -19,6 +19,7 @@ import java.io.OutputStreamWriter;
import java.nio.ByteBuffer;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import org.eclipse.jetty.http.DateGenerator;
import org.eclipse.jetty.http.HttpField;
@ -69,7 +70,8 @@ public class DefaultHandler extends Handler.Processor
public void setServer(Server server)
{
super.setServer(server);
initFavIcon();
if (server != null)
initFavIcon();
}
private void initFavIcon()
@ -77,18 +79,12 @@ public class DefaultHandler extends Handler.Processor
if (_favicon != null)
return;
if (getServer() == null)
{
// TODO: investigate why DefaultHandler.setServer(server) is passing null?
// See bug https://github.com/eclipse/jetty.project/issues/8442
LOG.warn("favicon.ico not supported with null Server");
return;
}
Server server = Objects.requireNonNull(getServer());
byte[] favbytes = null;
try
{
Resource faviconRes = getServer().getDefaultFavicon();
Resource faviconRes = server.getDefaultFavicon();
if (faviconRes != null)
{
try (InputStream is = faviconRes.newInputStream())

View File

@ -23,6 +23,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.jupiter.api.Assertions.assertThrows;
/**
@ -275,4 +276,50 @@ public class HandlerTest
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> a.insertHandler(b));
assertThat(e.getMessage(), containsString("bad tail"));
}
@Test
public void testSetServerPropagation()
{
Handler.Wrapper wrapper = new Handler.Wrapper();
Handler.Collection collection = new Handler.Collection();
Handler handler = new Handler.Abstract()
{
@Override
public Request.Processor handle(Request request) throws Exception
{
return null;
}
};
collection.addHandler(wrapper);
wrapper.setHandler(handler);
Server server = new Server();
collection.setServer(server);
assertThat(handler.getServer(), sameInstance(server));
}
@Test
public void testSetHandlerServerPropagation()
{
Handler.Wrapper wrapper = new Handler.Wrapper();
Handler.Collection collection = new Handler.Collection();
Handler handler = new Handler.Abstract()
{
@Override
public Request.Processor handle(Request request) throws Exception
{
return null;
}
};
Server server = new Server();
collection.setServer(server);
collection.addHandler(wrapper);
wrapper.setHandler(handler);
assertThat(handler.getServer(), sameInstance(server));
}
}

View File

@ -40,6 +40,7 @@ import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.ContextHandlerCollection;
import org.eclipse.jetty.server.handler.DefaultHandler;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
@ -67,6 +68,7 @@ import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
@ -605,4 +607,16 @@ public class WebAppContextTest
extLibs = extLibs.toAbsolutePath();
assertThat("URL[0]", urls[0].toURI(), is(extLibs.toUri()));
}
@Test
void testSetServerPropagation()
{
Server server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
DefaultHandler handler = new DefaultHandler();
server.setHandler(new Handler.Collection(context, handler));
assertThat(handler.getServer(), sameInstance(server));
}
}

View File

@ -40,6 +40,7 @@ import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.ContextHandlerCollection;
import org.eclipse.jetty.server.handler.DefaultHandler;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
@ -47,7 +48,6 @@ import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.resource.FileSystemPool;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
@ -67,6 +67,7 @@ import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
@ -611,4 +612,16 @@ public class WebAppContextTest
extLibs = extLibs.toAbsolutePath();
assertThat("URL[0]", urls[0].toURI(), is(extLibs.toUri()));
}
@Test
void testSetServerPropagation()
{
Server server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
DefaultHandler handler = new DefaultHandler();
server.setHandler(new Handler.Collection(context.get(), handler));
assertThat(handler.getServer(), sameInstance(server));
}
}