ChristopherSchultz commented on code in PR #976: URL: https://github.com/apache/tomcat/pull/976#discussion_r3057673953
########## test/org/apache/catalina/valves/TestFilterValve.java: ########## @@ -0,0 +1,229 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.valves; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestFilterValve extends TomcatBaseTest { + + + @Test + public void testFilterPassthrough() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(PassthroughFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertEquals(HelloWorldServlet.RESPONSE_TEXT, res.toString()); + } + + + @Test + public void testFilterBlocks() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(BlockingFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_FORBIDDEN, rc); + } + + @Test + public void testFilterWrappingRequestThrows() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(WrappingFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + int rc = getUrl("http://localhost:"; + getPort(), new ByteChunk(), null); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + } + + + @Test(expected = LifecycleException.class) + public void testNullFilterClassThrowsOnStart() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + FilterValve valve = new FilterValve(); + // Do NOT set filterClassName + ctx.getPipeline().addValve(valve); + + tomcat.start(); + } + + + @Test(expected = LifecycleException.class) + public void testInvalidFilterClassThrowsOnStart() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + FilterValve valve = new FilterValve(); + valve.setFilterClass("com.nonexistent.FakeFilter"); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + } + + + @Test + public void testGetFilterNameReturnsNull() { + FilterValve valve = new FilterValve(); + Assert.assertNull(valve.getFilterName()); + } + + + @Test + public void testInitParams() { Review Comment: Is this (specific) test really necessary? ########## test/org/apache/catalina/valves/TestFilterValve.java: ########## @@ -0,0 +1,229 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.valves; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestFilterValve extends TomcatBaseTest { + + + @Test + public void testFilterPassthrough() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(PassthroughFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertEquals(HelloWorldServlet.RESPONSE_TEXT, res.toString()); + } + + + @Test + public void testFilterBlocks() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(BlockingFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_FORBIDDEN, rc); + } + + @Test + public void testFilterWrappingRequestThrows() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(WrappingFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + int rc = getUrl("http://localhost:"; + getPort(), new ByteChunk(), null); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + } + + + @Test(expected = LifecycleException.class) + public void testNullFilterClassThrowsOnStart() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + FilterValve valve = new FilterValve(); + // Do NOT set filterClassName + ctx.getPipeline().addValve(valve); + + tomcat.start(); + } + + + @Test(expected = LifecycleException.class) + public void testInvalidFilterClassThrowsOnStart() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + FilterValve valve = new FilterValve(); + valve.setFilterClass("com.nonexistent.FakeFilter"); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + } + + + @Test + public void testGetFilterNameReturnsNull() { + FilterValve valve = new FilterValve(); + Assert.assertNull(valve.getFilterName()); + } + + + @Test + public void testInitParams() { + FilterValve valve = new FilterValve(); + + valve.addInitParam("key1", "value1"); + valve.addInitParam("key2", "value2"); + + Assert.assertEquals("value1", valve.getInitParameter("key1")); + Assert.assertEquals("value2", valve.getInitParameter("key2")); + Assert.assertNull(valve.getInitParameter("nonexistent")); + + List<String> names = Collections.list(valve.getInitParameterNames()); + Assert.assertEquals(2, names.size()); + Assert.assertTrue(names.contains("key1")); + Assert.assertTrue(names.contains("key2")); + } + + + @Test + public void testInitParamsEmpty() { + FilterValve valve = new FilterValve(); + + Assert.assertNull(valve.getInitParameter("anything")); + Assert.assertFalse(valve.getInitParameterNames().hasMoreElements()); + } + + + @Test + public void testGetSetFilterClassName() { Review Comment: Is this (specific) test really necessary? ########## test/org/apache/catalina/valves/TestFilterValve.java: ########## @@ -0,0 +1,229 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.valves; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestFilterValve extends TomcatBaseTest { + + + @Test + public void testFilterPassthrough() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(PassthroughFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertEquals(HelloWorldServlet.RESPONSE_TEXT, res.toString()); + } + + + @Test + public void testFilterBlocks() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(BlockingFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_FORBIDDEN, rc); + } + + @Test + public void testFilterWrappingRequestThrows() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(WrappingFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + int rc = getUrl("http://localhost:"; + getPort(), new ByteChunk(), null); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + } + + + @Test(expected = LifecycleException.class) + public void testNullFilterClassThrowsOnStart() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + FilterValve valve = new FilterValve(); + // Do NOT set filterClassName + ctx.getPipeline().addValve(valve); + + tomcat.start(); + } + + + @Test(expected = LifecycleException.class) + public void testInvalidFilterClassThrowsOnStart() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + FilterValve valve = new FilterValve(); + valve.setFilterClass("com.nonexistent.FakeFilter"); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + } + + + @Test + public void testGetFilterNameReturnsNull() { + FilterValve valve = new FilterValve(); + Assert.assertNull(valve.getFilterName()); + } + + + @Test + public void testInitParams() { + FilterValve valve = new FilterValve(); + + valve.addInitParam("key1", "value1"); + valve.addInitParam("key2", "value2"); + + Assert.assertEquals("value1", valve.getInitParameter("key1")); + Assert.assertEquals("value2", valve.getInitParameter("key2")); + Assert.assertNull(valve.getInitParameter("nonexistent")); + + List<String> names = Collections.list(valve.getInitParameterNames()); + Assert.assertEquals(2, names.size()); + Assert.assertTrue(names.contains("key1")); + Assert.assertTrue(names.contains("key2")); + } + + + @Test + public void testInitParamsEmpty() { + FilterValve valve = new FilterValve(); + + Assert.assertNull(valve.getInitParameter("anything")); + Assert.assertFalse(valve.getInitParameterNames().hasMoreElements()); + } + + + @Test + public void testGetSetFilterClassName() { + FilterValve valve = new FilterValve(); + + Assert.assertNull(valve.getFilterClassName()); + + valve.setFilterClassName("com.example.MyFilter"); + Assert.assertEquals("com.example.MyFilter", valve.getFilterClassName()); + + valve.setFilterClass("com.example.OtherFilter"); + Assert.assertEquals("com.example.OtherFilter", valve.getFilterClassName()); + } + + @Test(expected = IllegalStateException.class) + public void testGetServletContextThrowsBeforeStart() { Review Comment: Is this (specific) test really necessary? ########## test/org/apache/catalina/valves/TestSemaphoreValve.java: ########## @@ -0,0 +1,440 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.valves; + +import java.io.IOException; +import java.io.Serial; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.connector.Request; +import org.apache.catalina.connector.Response; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestSemaphoreValve extends TomcatBaseTest { + + + @Test + public void testBasicConcurrency() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + SemaphoreValve valve = new SemaphoreValve(); + valve.setConcurrency(10); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertEquals(HelloWorldServlet.RESPONSE_TEXT, res.toString()); + } + + @Test + public void testInterruptedConcurrency() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + SemaphoreValve valve = new SemaphoreValve(); + valve.setConcurrency(10); + valve.setInterruptible(true); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertEquals(HelloWorldServlet.RESPONSE_TEXT, res.toString()); + } + + + @Test + public void testNonBlockingDenied() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + CountDownLatch insideServlet = new CountDownLatch(1); + CountDownLatch canReturn = new CountDownLatch(1); + Tomcat.addServlet(ctx, "slow", new SlowServlet(insideServlet, canReturn)); + ctx.addServletMappingDecoded("/", "slow"); + + SemaphoreValve valve = new SemaphoreValve(); + valve.setConcurrency(1); + valve.setBlock(false); + valve.setHighConcurrencyStatus(HttpServletResponse.SC_SERVICE_UNAVAILABLE); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + // First request — should acquire the permit and block inside the servlet + AtomicInteger firstRc = new AtomicInteger(); + Thread firstThread = new Thread(() -> { + try { + firstRc.set(getUrl("http://localhost:"; + getPort(), new ByteChunk(), null)); + } catch (IOException e) { + // Ignore + } + }); + firstThread.start(); + + // Wait until the first request is inside the servlet + Assert.assertTrue("First request should reach servlet", + insideServlet.await(10, TimeUnit.SECONDS)); + + // Second request — should be denied because concurrency=1 and block=false + int rc2 = getUrl("http://localhost:"; + getPort(), new ByteChunk(), null); + + Assert.assertEquals(HttpServletResponse.SC_SERVICE_UNAVAILABLE, rc2); + + // Release the first request + canReturn.countDown(); + firstThread.join(10000); + Assert.assertFalse(firstThread.isAlive()); + Assert.assertEquals(HttpServletResponse.SC_OK, firstRc.get()); + } + + + @Test + public void testHighConcurrencyStatusNotSet() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + CountDownLatch insideServlet = new CountDownLatch(1); + CountDownLatch canReturn = new CountDownLatch(1); + Tomcat.addServlet(ctx, "slow", new SlowServlet(insideServlet, canReturn)); + ctx.addServletMappingDecoded("/", "slow"); + + SemaphoreValve valve = new SemaphoreValve(); + valve.setConcurrency(1); + valve.setBlock(false); + // highConcurrencyStatus is -1 by default (no error sent) + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + // First request holds the permit + Thread firstThread = new Thread(() -> { + try { + getUrl("http://localhost:"; + getPort(), new ByteChunk(), null); + } catch (IOException e) { + // Ignore + } + }); + firstThread.start(); + + Assert.assertTrue("First request should reach servlet", + insideServlet.await(10, TimeUnit.SECONDS)); + + // Second request — denied but no error status is sent + int rc2 = getUrl("http://localhost:"; + getPort(), new ByteChunk(), null); + + // With no highConcurrencyStatus, response is 200 without body + Assert.assertEquals(HttpServletResponse.SC_OK, rc2); + + canReturn.countDown(); + firstThread.join(10000); + } + + + @Test + public void testGetSetProperties() { Review Comment: Is this (specific) test really necessary? ########## test/org/apache/catalina/valves/TestFilterValve.java: ########## @@ -0,0 +1,229 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.valves; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestFilterValve extends TomcatBaseTest { + + + @Test + public void testFilterPassthrough() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(PassthroughFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertEquals(HelloWorldServlet.RESPONSE_TEXT, res.toString()); + } + + + @Test + public void testFilterBlocks() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(BlockingFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_FORBIDDEN, rc); + } + + @Test + public void testFilterWrappingRequestThrows() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + FilterValve valve = new FilterValve(); + valve.setFilterClass(WrappingFilter.class.getName()); + ctx.getPipeline().addValve(valve); + + tomcat.start(); + + int rc = getUrl("http://localhost:"; + getPort(), new ByteChunk(), null); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + } + + + @Test(expected = LifecycleException.class) + public void testNullFilterClassThrowsOnStart() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + Context ctx = getProgrammaticRootContext(); + + FilterValve valve = new FilterValve(); + // Do NOT set filterClassName + ctx.getPipeline().addValve(valve); + + tomcat.start(); + } + + + @Test(expected = LifecycleException.class) + public void testInvalidFilterClassThrowsOnStart() throws Exception { Review Comment: Is this (specific) test really necessary? ########## test/org/apache/catalina/valves/TestProxyErrorReportValve.java: ########## @@ -0,0 +1,277 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.valves; + +import java.io.IOException; +import java.io.Serial; + +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.core.StandardHost; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestProxyErrorReportValve extends TomcatBaseTest { + + private static final String PROXY_VALVE = + "org.apache.catalina.valves.ProxyErrorReportValve"; + + + @Test + public void testRedirectMode() throws Exception { + Tomcat tomcat = getTomcatInstance(); + StandardHost host = (StandardHost) tomcat.getHost(); + host.setErrorReportValveClass(PROXY_VALVE); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "error", new SendErrorServlet( + HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Server broke")); + ctx.addServletMappingDecoded("/", "error"); + + // Register an error page at the Host's error report valve level + // so findErrorPage() returns a URL for the redirect + Tomcat.addServlet(ctx, "errorPage", new ErrorPageServlet()); + ctx.addServletMappingDecoded("/error-page", "errorPage"); + + tomcat.start(); + + ProxyErrorReportValve valve = (ProxyErrorReportValve) host.getPipeline().getFirst(); + valve.setProperty("errorCode." + HttpServletResponse.SC_INTERNAL_SERVER_ERROR, + "http://localhost:"; + getPort() + "/error-page"); + + int rc = getUrl("http://localhost:"; + getPort(), new ByteChunk(), false); + + Assert.assertEquals(HttpServletResponse.SC_FOUND, rc); + } + + @Test + public void testProxyMode() throws Exception { + Tomcat tomcat = getTomcatInstance(); + StandardHost host = (StandardHost) tomcat.getHost(); + host.setErrorReportValveClass(PROXY_VALVE); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "error", new SendErrorServlet( + HttpServletResponse.SC_NOT_FOUND, "Not found")); + ctx.addServletMappingDecoded("/", "error"); + + Tomcat.addServlet(ctx, "errorPage", new ErrorPageServlet()); + ctx.addServletMappingDecoded("/error-page", "errorPage"); + + tomcat.start(); + + ProxyErrorReportValve valve = (ProxyErrorReportValve) host.getPipeline().getFirst(); + valve.setUseRedirect(false); + valve.setProperty("errorCode." + HttpServletResponse.SC_NOT_FOUND, + "http://localhost:"; + getPort() + "/error-page"); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_NOT_FOUND, rc); + Assert.assertTrue(res.toString().contains("ERROR_PAGE_OK")); + } + + + @Test + public void testNoErrorPageFallsBackToSuper() throws Exception { + Tomcat tomcat = getTomcatInstance(); + ((StandardHost) tomcat.getHost()).setErrorReportValveClass(PROXY_VALVE); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "error", new SendErrorServlet( + HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "No page configured")); + ctx.addServletMappingDecoded("/", "error"); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + + String body = res.toString(); + Assert.assertNotNull(body); + Assert.assertTrue("Should contain HTML error report", + body.contains("html") && + body.contains(String.valueOf(HttpServletResponse.SC_INTERNAL_SERVER_ERROR))); + } + + + @Test + public void testStatusBelow400Ignored() throws Exception { + Tomcat tomcat = getTomcatInstance(); + ((StandardHost) tomcat.getHost()).setErrorReportValveClass(PROXY_VALVE); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "hello", new HelloWorldServlet()); + ctx.addServletMappingDecoded("/", "hello"); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertEquals(HelloWorldServlet.RESPONSE_TEXT, res.toString()); + } + + + @Test + public void testStatusNotFound() throws Exception { + Tomcat tomcat = getTomcatInstance(); + ((StandardHost) tomcat.getHost()).setErrorReportValveClass(PROXY_VALVE); + + Context ctx = getProgrammaticRootContext(); + + Tomcat.addServlet(ctx, "notFound", new SendErrorServlet( + HttpServletResponse.SC_NOT_FOUND, "Resource not found")); + ctx.addServletMappingDecoded("/", "notFound"); + + tomcat.start(); + + ByteChunk res = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort(), res, null); + + Assert.assertEquals(HttpServletResponse.SC_NOT_FOUND, rc); + + String body = res.toString(); + Assert.assertNotNull(body); + Assert.assertTrue("Should contain error report", + body.contains(String.valueOf(HttpServletResponse.SC_NOT_FOUND))); + } + + + @Test + public void testGetSetProperties() { Review Comment: Is this (specific) test really necessary? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
