Skip to content

Commit

Permalink
Adapt to Servlet API 6 changes and support Jakarta WebSocket 2.1
Browse files Browse the repository at this point in the history
Closes gh-12146
Closes gh-12148
  • Loading branch information
marcusdacoregio committed Nov 8, 2022
1 parent 9e628fb commit 3b5d19c
Show file tree
Hide file tree
Showing 14 changed files with 59 additions and 142 deletions.
2 changes: 2 additions & 0 deletions config/spring-security-config.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ dependencies {
testImplementation 'io.rsocket:rsocket-transport-netty'
testImplementation 'jakarta.annotation:jakarta.annotation-api'
testImplementation 'jakarta.xml.bind:jakarta.xml.bind-api'
testImplementation 'jakarta.websocket:jakarta.websocket-api'
testImplementation 'jakarta.websocket:jakarta.websocket-client-api'
testImplementation 'ldapsdk:ldapsdk:4.1'
testImplementation('net.sourceforge.htmlunit:htmlunit') {
exclude group: 'commons-logging', module: 'commons-logging'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,7 @@ public void whenEnableSessionUrlRewritingTrueThenEncodeNotInvoked() throws Excep
HttpServletResponse responseToSpy = spy((HttpServletResponse) response);
chain.doFilter(request, responseToSpy);
verify(responseToSpy, atLeastOnce()).encodeRedirectURL(any());
verify(responseToSpy, atLeastOnce()).encodeRedirectUrl(any());
verify(responseToSpy, atLeastOnce()).encodeURL(any());
verify(responseToSpy, atLeastOnce()).encodeUrl(any());
})
.apply(springSecurity())
.build();
Expand All @@ -348,9 +346,7 @@ public void whenDefaultThenEncodeNotInvoked() throws Exception {
HttpServletResponse responseToSpy = spy((HttpServletResponse) response);
chain.doFilter(request, responseToSpy);
verify(responseToSpy, never()).encodeRedirectURL(any());
verify(responseToSpy, never()).encodeRedirectUrl(any());
verify(responseToSpy, never()).encodeURL(any());
verify(responseToSpy, never()).encodeUrl(any());
})
.apply(springSecurity())
.build();
Expand Down Expand Up @@ -807,9 +803,7 @@ static class EncodesUrls {
@RequestMapping("/")
String encoded(HttpServletResponse response) {
response.encodeURL("/foo");
response.encodeUrl("/foo");
response.encodeRedirectURL("/foo");
response.encodeRedirectUrl("/foo");
return "encoded";
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -149,16 +149,6 @@ public String encodeRedirectURL(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

@Override
public String encodeUrl(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

@Override
public String encodeRedirectUrl(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

}

public static final class MockObservationRegistry implements FactoryBean<ObservationRegistry> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -578,16 +578,12 @@ public void configureWhenUsingDisableUrlRewritingAndCustomRepositoryThenRedirect
FilterChainProxy proxy = this.spring.getContext().getBean(FilterChainProxy.class);
proxy.doFilter(request, responseToSpy, (req, resp) -> {
HttpServletResponse httpResponse = (HttpServletResponse) resp;
httpResponse.encodeUrl("/");
httpResponse.encodeURL("/");
httpResponse.encodeRedirectUrl("/");
httpResponse.encodeRedirectURL("/");
httpResponse.getWriter().write("encodeRedirect");
});
verify(responseToSpy, never()).encodeRedirectURL(any());
verify(responseToSpy, never()).encodeRedirectUrl(any());
verify(responseToSpy, never()).encodeURL(any());
verify(responseToSpy, never()).encodeUrl(any());
assertThat(responseToSpy.getContentAsString()).isEqualTo("encodeRedirect");
}

Expand Down Expand Up @@ -1028,16 +1024,6 @@ public String encodeRedirectURL(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

@Override
public String encodeUrl(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

@Override
public String encodeRedirectUrl(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -620,16 +620,6 @@ public String encodeRedirectURL(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

@Override
public String encodeUrl(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

@Override
public String encodeRedirectUrl(String url) {
throw new RuntimeException("Unexpected invocation of encodeURL");
}

}

}
8 changes: 5 additions & 3 deletions dependencies/spring-security-dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ dependencies {
api "io.micrometer:micrometer-observation:$micrometerVersion"
api "jakarta.annotation:jakarta.annotation-api:2.1.1"
api "jakarta.inject:jakarta.inject-api:2.0.1"
api "jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api:2.0.0"
api "jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api:3.0.0"
api "jakarta.servlet.jsp:jakarta.servlet.jsp-api:3.1.0"
api "jakarta.servlet:jakarta.servlet-api:5.0.0"
api "jakarta.xml.bind:jakarta.xml.bind-api:3.0.1"
api "jakarta.servlet:jakarta.servlet-api:6.0.0"
api "jakarta.xml.bind:jakarta.xml.bind-api:4.0.0"
api "jakarta.persistence:jakarta.persistence-api:3.1.0"
api "jakarta.websocket:jakarta.websocket-api:2.1.0"
api "jakarta.websocket:jakarta.websocket-client-api:2.1.0"
api "ldapsdk:ldapsdk:4.1"
api "net.sourceforge.htmlunit:htmlunit:2.65.1"
api "net.sourceforge.nekohtml:nekohtml:1.9.22"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,6 @@ protected void onResponseCommitted() {
this.contextSaved = true;
}

@Override
public final String encodeRedirectUrl(String url) {
if (this.disableUrlRewriting) {
return url;
}
return super.encodeRedirectUrl(url);
}

@Override
public final String encodeRedirectURL(String url) {
if (this.disableUrlRewriting) {
Expand All @@ -121,14 +113,6 @@ public final String encodeRedirectURL(String url) {
return super.encodeRedirectURL(url);
}

@Override
public final String encodeUrl(String url) {
if (this.disableUrlRewriting) {
return url;
}
return super.encodeUrl(url);
}

@Override
public final String encodeURL(String url) {
if (this.disableUrlRewriting) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2016 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -51,7 +51,8 @@ public Cookie deserialize(JsonParser jp, DeserializationContext ctxt) throws IOE
cookie.setSecure(readJsonNode(jsonNode, "secure").asBoolean());
cookie.setVersion(readJsonNode(jsonNode, "version").asInt());
cookie.setPath(readJsonNode(jsonNode, "path").asText());
cookie.setHttpOnly(readJsonNode(jsonNode, "httpOnly").asBoolean());
JsonNode attributes = readJsonNode(jsonNode, "attributes");
cookie.setHttpOnly(readJsonNode(attributes, "HttpOnly").asBoolean());
return cookie;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,11 @@ private DisableEncodeUrlResponseWrapper(HttpServletResponse response) {
super(response);
}

@Override
public String encodeRedirectUrl(String url) {
return url;
}

@Override
public String encodeRedirectURL(String url) {
return url;
}

@Override
public String encodeUrl(String url) {
return url;
}

@Override
public String encodeURL(String url) {
return url;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,28 +362,6 @@ public void setCookieSetsIsHttpOnlyFlagByDefault() {
assertThat(cookie.isHttpOnly()).isTrue();
}

// SEC-2791
@Test
public void setCookieMaxAge0VersionSet() {
MockRememberMeServices services = new MockRememberMeServices();
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
services.setCookie(new String[] { "value" }, 0, request, response);
Cookie cookie = response.getCookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY);
assertThat(cookie.getVersion()).isEqualTo(1);
}

// SEC-2791
@Test
public void setCookieMaxAgeNegativeVersionSet() {
MockRememberMeServices services = new MockRememberMeServices();
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse();
services.setCookie(new String[] { "value" }, -1, request, response);
Cookie cookie = response.getCookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY);
assertThat(cookie.getVersion()).isEqualTo(1);
}

// SEC-2791
@Test
public void setCookieMaxAge1VersionSet() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,21 +540,11 @@ public void sessionDisableUrlRewritingPreventsSessionIdBeingWrittenToUrl() {
MockHttpServletRequest request = new MockHttpServletRequest();
final String sessionId = ";jsessionid=id";
MockHttpServletResponse response = new MockHttpServletResponse() {
@Override
public String encodeRedirectUrl(String url) {
return url + sessionId;
}

@Override
public String encodeRedirectURL(String url) {
return url + sessionId;
}

@Override
public String encodeUrl(String url) {
return url + sessionId;
}

@Override
public String encodeURL(String url) {
return url + sessionId;
Expand All @@ -563,16 +553,12 @@ public String encodeURL(String url) {
HttpRequestResponseHolder holder = new HttpRequestResponseHolder(request, response);
repo.loadContext(holder);
String url = "/aUrl";
assertThat(holder.getResponse().encodeRedirectUrl(url)).isEqualTo(url + sessionId);
assertThat(holder.getResponse().encodeRedirectURL(url)).isEqualTo(url + sessionId);
assertThat(holder.getResponse().encodeUrl(url)).isEqualTo(url + sessionId);
assertThat(holder.getResponse().encodeURL(url)).isEqualTo(url + sessionId);
repo.setDisableUrlRewriting(true);
holder = new HttpRequestResponseHolder(request, response);
repo.loadContext(holder);
assertThat(holder.getResponse().encodeRedirectUrl(url)).isEqualTo(url);
assertThat(holder.getResponse().encodeRedirectURL(url)).isEqualTo(url);
assertThat(holder.getResponse().encodeUrl(url)).isEqualTo(url);
assertThat(holder.getResponse().encodeURL(url)).isEqualTo(url);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -140,14 +140,6 @@ public void addCookieWhenCookieDomainContainsCrlfThenException() {
.withMessageContaining(CRLF_MESSAGE);
}

@Test
public void addCookieWhenCookieCommentContainsCrlfThenException() {
Cookie cookie = new Cookie("foo", "bar");
cookie.setComment("foo\r\nbar");
assertThatIllegalArgumentException().isThrownBy(() -> this.fwResponse.addCookie(cookie))
.withMessageContaining(CRLF_MESSAGE);
}

@Test
public void rejectAnyLineEndingInNameAndValue() {
validateLineEnding("foo", "foo\rbar");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2016 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,19 +33,35 @@
public class CookieMixinTests extends AbstractMixinTests {

// @formatter:off
private static final String COOKIE_JSON = "{"
+ "\"@class\": \"jakarta.servlet.http.Cookie\", "
+ "\"name\": \"demo\", "
+ "\"value\": \"cookie1\","
+ "\"comment\": null, "
+ "\"maxAge\": -1, "
+ "\"path\": null, "
+ "\"secure\": false, "
+ "\"version\": 0, "
+ "\"isHttpOnly\": false, "
+ "\"domain\": null"
+ "}";
private static final String COOKIE_JSON = "{" +
" \"@class\": \"jakarta.servlet.http.Cookie\"," +
" \"name\": \"demo\"," +
" \"value\": \"cookie1\"," +
" \"attributes\":{\"@class\":\"java.util.Collections$EmptyMap\"}," +
" \"comment\": null," +
" \"maxAge\": -1," +
" \"path\": null," +
" \"secure\": false," +
" \"version\": 0," +
" \"domain\": null" +
"}";
// @formatter:on

// @formatter:off
private static final String COOKIE_HTTP_ONLY_JSON = "{" +
" \"@class\": \"jakarta.servlet.http.Cookie\"," +
" \"name\": \"demo\"," +
" \"value\": \"cookie1\"," +
" \"attributes\":{\"@class\":\"java.util.Collections$UnmodifiableMap\", \"HttpOnly\": \"true\"}," +
" \"comment\": null," +
" \"maxAge\": -1," +
" \"path\": null," +
" \"secure\": false," +
" \"version\": 0," +
" \"domain\": null" +
"}";
// @formatter:on

@Test
public void serializeCookie() throws JsonProcessingException, JSONException {
Cookie cookie = new Cookie("demo", "cookie1");
Expand All @@ -59,7 +75,23 @@ public void deserializeCookie() throws IOException {
assertThat(cookie).isNotNull();
assertThat(cookie.getName()).isEqualTo("demo");
assertThat(cookie.getDomain()).isEqualTo("");
assertThat(cookie.isHttpOnly()).isEqualTo(false);
}

@Test
public void serializeCookieWithHttpOnly() throws JsonProcessingException, JSONException {
Cookie cookie = new Cookie("demo", "cookie1");
cookie.setHttpOnly(true);
String actualString = this.mapper.writeValueAsString(cookie);
JSONAssert.assertEquals(COOKIE_HTTP_ONLY_JSON, actualString, true);
}

@Test
public void deserializeCookieWithHttpOnly() throws IOException {
Cookie cookie = this.mapper.readValue(COOKIE_HTTP_ONLY_JSON, Cookie.class);
assertThat(cookie).isNotNull();
assertThat(cookie.getName()).isEqualTo("demo");
assertThat(cookie.getDomain()).isEqualTo("");
assertThat(cookie.isHttpOnly()).isEqualTo(true);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,11 @@ void doFilterDisablesEncodeURL() throws Exception {
verifyDoFilterDoesNotInteractWithResponse((httpResponse) -> httpResponse.encodeURL("/"));
}

@Test
void doFilterDisablesEncodeUrl() throws Exception {
verifyDoFilterDoesNotInteractWithResponse((httpResponse) -> httpResponse.encodeUrl("/"));
}

@Test
void doFilterDisablesEncodeRedirectURL() throws Exception {
verifyDoFilterDoesNotInteractWithResponse((httpResponse) -> httpResponse.encodeRedirectURL("/"));
}

@Test
void doFilterDisablesEncodeRedirectUrl() throws Exception {
verifyDoFilterDoesNotInteractWithResponse((httpResponse) -> httpResponse.encodeRedirectUrl("/"));
}

private void verifyDoFilterDoesNotInteractWithResponse(Consumer<HttpServletResponse> toInvoke) throws Exception {
this.filter.doFilter(this.request, this.response, (request, response) -> {
HttpServletResponse httpResponse = (HttpServletResponse) response;
Expand Down

0 comments on commit 3b5d19c

Please sign in to comment.