Skip to content

Commit 0df5ece

Browse files
committed
Extract rejectNonPrintableAsciiCharactersInFieldName
Closes gh-11234
1 parent beec7c9 commit 0df5ece

File tree

2 files changed

+42
-5
lines changed

2 files changed

+42
-5
lines changed

web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -431,14 +431,20 @@ public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws
431431
if (!isNormalized(request)) {
432432
throw new RequestRejectedException("The request was rejected because the URL was not normalized.");
433433
}
434-
String requestUri = request.getRequestURI();
435-
if (!containsOnlyPrintableAsciiCharacters(requestUri)) {
436-
throw new RequestRejectedException(
437-
"The requestURI was rejected because it can only contain printable ASCII characters.");
438-
}
434+
rejectNonPrintableAsciiCharactersInFieldName(request.getRequestURI(), "requestURI");
435+
rejectNonPrintableAsciiCharactersInFieldName(request.getServletPath(), "servletPath");
436+
rejectNonPrintableAsciiCharactersInFieldName(request.getPathInfo(), "pathInfo");
437+
rejectNonPrintableAsciiCharactersInFieldName(request.getContextPath(), "contextPath");
439438
return new StrictFirewalledRequest(request);
440439
}
441440

441+
private void rejectNonPrintableAsciiCharactersInFieldName(String toCheck, String propertyName) {
442+
if (!containsOnlyPrintableAsciiCharacters(toCheck)) {
443+
throw new RequestRejectedException(String.format(
444+
"The %s was rejected because it can only contain printable ASCII characters.", propertyName));
445+
}
446+
}
447+
442448
private void rejectForbiddenHttpMethod(HttpServletRequest request) {
443449
if (this.allowedHttpMethods == ALLOW_ANY_HTTP_METHOD) {
444450
return;
@@ -526,6 +532,9 @@ private static boolean decodedUrlContains(HttpServletRequest request, String val
526532
}
527533

528534
private static boolean containsOnlyPrintableAsciiCharacters(String uri) {
535+
if (uri == null) {
536+
return true;
537+
}
529538
int length = uri.length();
530539
for (int i = 0; i < length; i++) {
531540
char ch = uri.charAt(i);

web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,34 @@ public void getFirewalledRequestWhenContainsEncodedNullThenException() {
363363
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
364364
}
365365

366+
@Test
367+
public void getFirewalledRequestWhenContainsLineFeedThenException() {
368+
this.request.setRequestURI("/something\n/");
369+
assertThatExceptionOfType(RequestRejectedException.class)
370+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
371+
}
372+
373+
@Test
374+
public void getFirewalledRequestWhenServletPathContainsLineFeedThenException() {
375+
this.request.setServletPath("/something\n/");
376+
assertThatExceptionOfType(RequestRejectedException.class)
377+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
378+
}
379+
380+
@Test
381+
public void getFirewalledRequestWhenContainsCarriageReturnThenException() {
382+
this.request.setRequestURI("/something\r/");
383+
assertThatExceptionOfType(RequestRejectedException.class)
384+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
385+
}
386+
387+
@Test
388+
public void getFirewalledRequestWhenServletPathContainsCarriageReturnThenException() {
389+
this.request.setServletPath("/something\r/");
390+
assertThatExceptionOfType(RequestRejectedException.class)
391+
.isThrownBy(() -> this.firewall.getFirewalledRequest(this.request));
392+
}
393+
366394
/**
367395
* On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on /a/b/c
368396
* because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c while Spring MVC

0 commit comments

Comments
 (0)