Skip to content

Commit 5fa7ac6

Browse files
fl4viafl4via
authored
Merge pull request #794 from gaol/Trailing_Slash_Fix
[UNDERTOW-1578] 401 Unauthorized should be returned when requesting a protected directory without trailing slash
2 parents f99c8db + d2715e3 commit 5fa7ac6

File tree

4 files changed

+244
-26
lines changed

4 files changed

+244
-26
lines changed

‎servlet/src/main/java/io/undertow/servlet/core/DeploymentManagerImpl.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import io.undertow.servlet.api.ThreadSetupHandler;
7272
import io.undertow.servlet.api.WebResourceCollection;
7373
import io.undertow.servlet.handlers.CrawlerSessionManagerHandler;
74+
import io.undertow.servlet.handlers.RedirectDirHandler;
7475
import io.undertow.servlet.handlers.ServletDispatchingHandler;
7576
import io.undertow.servlet.handlers.ServletHandler;
7677
import io.undertow.servlet.handlers.ServletInitialHandler;
@@ -218,6 +219,7 @@ public Void call(HttpServerExchange exchange, Object ignore) throws Exception {
218219

219220
HttpHandler wrappedHandlers = ServletDispatchingHandler.INSTANCE;
220221
wrappedHandlers = wrapHandlers(wrappedHandlers, deploymentInfo.getInnerHandlerChainWrappers());
222+
wrappedHandlers = new RedirectDirHandler(wrappedHandlers, deployment.getServletPaths());
221223
if(!deploymentInfo.isSecurityDisabled()) {
222224
HttpHandler securityHandler = setupSecurityHandlers(wrappedHandlers);
223225
wrappedHandlers = new PredicateHandler(DispatcherTypePredicate.REQUEST, securityHandler, wrappedHandlers);

‎servlet/src/main/java/io/undertow/servlet/handlers/RedirectDirHandler.java

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* JBoss, Home of Professional Open Source.
3+
* Copyright 2019 Red Hat, Inc., and individual contributors
4+
* as indicated by the @author tags.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package io.undertow.servlet.handlers;
20+
21+
import io.undertow.server.HttpHandler;
22+
import io.undertow.server.HttpServerExchange;
23+
import io.undertow.util.Headers;
24+
import io.undertow.util.Methods;
25+
import io.undertow.util.RedirectBuilder;
26+
import io.undertow.util.StatusCodes;
27+
28+
/**
29+
* Handler that redirects the directory requests without trailing slash to the one append trailing slash.
30+
*
31+
* @author Lin Gao
32+
*/
33+
public class RedirectDirHandler implements HttpHandler {
34+
35+
private static final String HTTP2_UPGRADE_PREFIX = "h2";
36+
37+
private final HttpHandler next;
38+
private final ServletPathMatches paths;
39+
40+
public RedirectDirHandler(HttpHandler next, ServletPathMatches paths) {
41+
this.next = next;
42+
this.paths = paths;
43+
}
44+
45+
@Override
46+
public void handleRequest(HttpServerExchange exchange) throws Exception {
47+
final String path = exchange.getRelativePath();
48+
final ServletPathMatch info = paths.getServletHandlerByPath(path);
49+
// https://issues.jboss.org/browse/WFLY-3439
50+
// if the request is an upgrade request then we don't want to redirect
51+
// as there is a good chance the web socket client won't understand the redirect
52+
// we make an exception for HTTP2 upgrade requests, as this would have already be handled at
53+
// the connector level if it was going to be handled.
54+
String upgradeString = exchange.getRequestHeaders().getFirst(Headers.UPGRADE);
55+
boolean isUpgradeRequest = upgradeString != null && !upgradeString.startsWith(HTTP2_UPGRADE_PREFIX);
56+
if (info.getType() == ServletPathMatch.Type.REDIRECT && !isUpgradeRequest) {
57+
// UNDERTOW-89
58+
// we redirect on GET requests to the root context to add an / to the end
59+
if (exchange.getRequestMethod().equals(Methods.GET) || exchange.getRequestMethod().equals(Methods.HEAD)) {
60+
exchange.setStatusCode(StatusCodes.FOUND);
61+
} else {
62+
exchange.setStatusCode(StatusCodes.TEMPORARY_REDIRECT);
63+
}
64+
exchange.getResponseHeaders().put(Headers.LOCATION,
65+
RedirectBuilder.redirect(exchange, exchange.getRelativePath() + "/", true));
66+
return;
67+
}
68+
next.handleRequest(exchange);
69+
}
70+
71+
}

‎servlet/src/main/java/io/undertow/servlet/handlers/ServletInitialHandler.java

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,8 @@
3939
import io.undertow.servlet.spec.HttpServletResponseImpl;
4040
import io.undertow.servlet.spec.RequestDispatcherImpl;
4141
import io.undertow.servlet.spec.ServletContextImpl;
42-
import io.undertow.util.Headers;
4342
import io.undertow.util.HttpString;
44-
import io.undertow.util.Methods;
4543
import io.undertow.util.Protocols;
46-
import io.undertow.util.RedirectBuilder;
4744
import io.undertow.util.StatusCodes;
4845
import org.xnio.ChannelListener;
4946
import org.xnio.Option;
@@ -80,8 +77,6 @@
8077
*/
8178
public class ServletInitialHandler implements HttpHandler, ServletDispatcher {
8279

83-
private static final String HTTP2_UPGRADE_PREFIX = "h2";
84-
8580
private static final RuntimePermission PERMISSION = new RuntimePermission("io.undertow.servlet.CREATE_INITIAL_HANDLER");
8681

8782
private final HttpHandler next;
@@ -149,30 +144,12 @@ public void handleRequest(final HttpServerExchange exchange) throws Exception {
149144
return;
150145
}
151146
final ServletPathMatch info = paths.getServletHandlerByPath(path);
152-
//https://issues.jboss.org/browse/WFLY-3439
153-
//if the request is an upgrade request then we don't want to redirect
154-
//as there is a good chance the web socket client won't understand the redirect
155-
//we make an exception for HTTP2 upgrade requests, as this would have already be handled at
156-
//the connector level if it was going to be handled.
157-
String upgradeString = exchange.getRequestHeaders().getFirst(Headers.UPGRADE);
158-
boolean isUpgradeRequest = upgradeString != null && !upgradeString.startsWith(HTTP2_UPGRADE_PREFIX);
159-
if (info.getType() == ServletPathMatch.Type.REDIRECT && !isUpgradeRequest) {
160-
//UNDERTOW-89
161-
//we redirect on GET requests to the root context to add an / to the end
162-
if(exchange.getRequestMethod().equals(Methods.GET) || exchange.getRequestMethod().equals(Methods.HEAD)) {
163-
exchange.setStatusCode(StatusCodes.FOUND);
164-
} else {
165-
exchange.setStatusCode(StatusCodes.TEMPORARY_REDIRECT);
166-
}
167-
exchange.getResponseHeaders().put(Headers.LOCATION, RedirectBuilder.redirect(exchange, exchange.getRelativePath() + "/", true));
168-
return;
169-
} else if (info.getType() == ServletPathMatch.Type.REWRITE) {
170-
//this can only happen if the path ends with a /
171-
//otherwise there would be a redirect instead
147+
if (info.getType() == ServletPathMatch.Type.REWRITE) {
148+
// this can only happen if the path ends with a /
149+
// otherwise there would be a redirect instead
172150
exchange.setRelativePath(info.getRewriteLocation());
173151
exchange.setRequestPath(exchange.getResolvedPath() + info.getRewriteLocation());
174152
}
175-
176153
final HttpServletResponseImpl response = new HttpServletResponseImpl(exchange, servletContext);
177154
final HttpServletRequestImpl request = new HttpServletRequestImpl(exchange, servletContext);
178155
final ServletRequestContext servletRequestContext = new ServletRequestContext(servletContext.getDeployment(), request, response, info);

‎servlet/src/test/java/io/undertow/servlet/test/defaultservlet/SecurityRedirectTestCase.java

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
/*
2+
* JBoss, Home of Professional Open Source.
3+
* Copyright 2019 Red Hat, Inc., and individual contributors
4+
* as indicated by the @author tags.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package io.undertow.servlet.test.defaultservlet;
20+
21+
import static io.undertow.util.Headers.AUTHORIZATION;
22+
import static io.undertow.util.Headers.BASIC;
23+
import static io.undertow.util.Headers.LOCATION;
24+
import static io.undertow.util.Headers.WWW_AUTHENTICATE;
25+
import static org.junit.Assert.assertEquals;
26+
27+
import java.io.IOException;
28+
29+
import javax.servlet.ServletException;
30+
31+
import org.apache.http.Header;
32+
import org.apache.http.HttpResponse;
33+
import org.apache.http.client.HttpClient;
34+
import org.apache.http.client.methods.HttpGet;
35+
import org.apache.http.impl.client.HttpClientBuilder;
36+
import org.junit.Assert;
37+
import org.junit.BeforeClass;
38+
import org.junit.Test;
39+
import org.junit.runner.RunWith;
40+
41+
import io.undertow.server.handlers.PathHandler;
42+
import io.undertow.servlet.api.DeploymentInfo;
43+
import io.undertow.servlet.api.DeploymentManager;
44+
import io.undertow.servlet.api.LoginConfig;
45+
import io.undertow.servlet.api.SecurityConstraint;
46+
import io.undertow.servlet.api.ServletContainer;
47+
import io.undertow.servlet.api.WebResourceCollection;
48+
import io.undertow.servlet.test.path.ServletPathMappingTestCase;
49+
import io.undertow.servlet.test.security.constraint.ServletIdentityManager;
50+
import io.undertow.servlet.test.util.TestClassIntrospector;
51+
import io.undertow.servlet.test.util.TestResourceLoader;
52+
import io.undertow.testutils.DefaultServer;
53+
import io.undertow.testutils.HttpClientUtils;
54+
import io.undertow.util.FlexBase64;
55+
import io.undertow.util.StatusCodes;
56+
57+
/**
58+
* TestCase on redirect with or without trailing slash when requesting protected path.
59+
*
60+
* @author Lin Gao
61+
*/
62+
@RunWith(DefaultServer.class)
63+
public class SecurityRedirectTestCase {
64+
65+
@BeforeClass
66+
public static void setup() throws ServletException {
67+
68+
final PathHandler root = new PathHandler();
69+
final ServletContainer container = ServletContainer.Factory.newInstance();
70+
71+
ServletIdentityManager identityManager = new ServletIdentityManager();
72+
identityManager.addUser("user1", "password1", "role1");
73+
74+
DeploymentInfo builder = new DeploymentInfo()
75+
.setClassIntrospecter(TestClassIntrospector.INSTANCE)
76+
.setClassLoader(ServletPathMappingTestCase.class.getClassLoader())
77+
.setContextPath("/servletContext")
78+
.setDeploymentName("servletContext.war")
79+
.setResourceManager(new TestResourceLoader(SecurityRedirectTestCase.class))
80+
.addWelcomePages("index.html")
81+
.setIdentityManager(identityManager)
82+
.setLoginConfig(new LoginConfig("BASIC", "Test Realm"))
83+
.addSecurityConstraint(new SecurityConstraint()
84+
.addRoleAllowed("role1")
85+
.addWebResourceCollection(new WebResourceCollection()
86+
.addUrlPatterns("/index.html", "/filterpath/*")));
87+
88+
DeploymentManager manager = container.addDeployment(builder);
89+
manager.deploy();
90+
root.addPrefixPath(builder.getContextPath(), manager.start());
91+
DefaultServer.setRootHandler(root);
92+
}
93+
94+
@SuppressWarnings("deprecation")
95+
@Test
96+
public void testSecurityWithWelcomeFileRedirect() throws IOException {
97+
// disable following redirect
98+
HttpClient client = HttpClientBuilder.create().disableRedirectHandling().build();
99+
try {
100+
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext");
101+
HttpResponse result = client.execute(get);
102+
Assert.assertEquals(StatusCodes.FOUND, result.getStatusLine().getStatusCode());
103+
Header[] values = result.getHeaders(LOCATION.toString());
104+
assertEquals(1, values.length);
105+
assertEquals(DefaultServer.getDefaultServerURL() + "/servletContext/", values[0].getValue());
106+
HttpClientUtils.readResponse(result);
107+
108+
get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/");
109+
result = client.execute(get);
110+
Assert.assertEquals(StatusCodes.UNAUTHORIZED, result.getStatusLine().getStatusCode());
111+
112+
values = result.getHeaders(WWW_AUTHENTICATE.toString());
113+
assertEquals(1, values.length);
114+
assertEquals(BASIC + " realm=\"Test Realm\"", values[0].getValue());
115+
HttpClientUtils.readResponse(result);
116+
117+
get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/");
118+
get.addHeader(AUTHORIZATION.toString(), BASIC + " " + FlexBase64.encodeString("user1:password1".getBytes(), false));
119+
result = client.execute(get);
120+
String response = HttpClientUtils.readResponse(result);
121+
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
122+
Assert.assertTrue(response.contains("Redirected home page"));
123+
} finally {
124+
client.getConnectionManager().shutdown();
125+
}
126+
}
127+
128+
@SuppressWarnings("deprecation")
129+
@Test
130+
public void testSecurityWithoutWelcomeFileRedirect() throws IOException {
131+
// disable following redirect
132+
HttpClient client = HttpClientBuilder.create().disableRedirectHandling().build();
133+
try {
134+
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/filterpath");
135+
HttpResponse result = client.execute(get);
136+
Assert.assertEquals(StatusCodes.UNAUTHORIZED, result.getStatusLine().getStatusCode());
137+
HttpClientUtils.readResponse(result);
138+
139+
get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/filterpath/");
140+
result = client.execute(get);
141+
Assert.assertEquals(StatusCodes.UNAUTHORIZED, result.getStatusLine().getStatusCode());
142+
143+
Header[] values = result.getHeaders(WWW_AUTHENTICATE.toString());
144+
assertEquals(1, values.length);
145+
assertEquals(BASIC + " realm=\"Test Realm\"", values[0].getValue());
146+
HttpClientUtils.readResponse(result);
147+
148+
get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/filterpath");
149+
get.addHeader(AUTHORIZATION.toString(), BASIC + " " + FlexBase64.encodeString("user1:password1".getBytes(), false));
150+
result = client.execute(get);
151+
Assert.assertEquals(StatusCodes.FOUND, result.getStatusLine().getStatusCode());
152+
values = result.getHeaders(LOCATION.toString());
153+
assertEquals(1, values.length);
154+
assertEquals(DefaultServer.getDefaultServerURL() + "/servletContext/filterpath/", values[0].getValue());
155+
HttpClientUtils.readResponse(result);
156+
157+
get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext/filterpath/filtered.txt");
158+
get.addHeader(AUTHORIZATION.toString(), BASIC + " " + FlexBase64.encodeString("user1:password1".getBytes(), false));
159+
result = client.execute(get);
160+
String response = HttpClientUtils.readResponse(result);
161+
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
162+
Assert.assertTrue(response.equals("Stuart"));
163+
} finally {
164+
client.getConnectionManager().shutdown();
165+
}
166+
}
167+
168+
}

0 commit comments

Comments
 (0)