Skip to content

Commit

Permalink
Harmonize default for server.tomcat.use-relative-redirects
Browse files Browse the repository at this point in the history
Prior to this commit, the property was a Boolean with a null default.
If it was explicitly set by the user, a context customizer would use
that value to set it on the context. However, if it was not set, the default
wouldn't be tomcat's default but `false` because it was explicitly set to
`false` in `TomcatServletWebServerFactory`. This commit defaults the property
itself to `false` so that the default is more obvious to the user.

Fixes gh-20796
  • Loading branch information
mbhave committed May 6, 2020
1 parent 386d678 commit f29bce6
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 7 deletions.
Expand Up @@ -380,7 +380,7 @@ public static class Tomcat {
* Whether HTTP 1.1 and later location headers generated by a call to sendRedirect
* will use relative or absolute redirects.
*/
private Boolean useRelativeRedirects;
private boolean useRelativeRedirects;

/**
* Character encoding to use to decode the URI.
Expand Down Expand Up @@ -537,14 +537,19 @@ public void setRedirectContextRoot(Boolean redirectContextRoot) {
this.redirectContextRoot = redirectContextRoot;
}

public Boolean getUseRelativeRedirects() {
public boolean getUseRelativeRedirects() {
return this.useRelativeRedirects;
}

public void setUseRelativeRedirects(Boolean useRelativeRedirects) {
public void setUseRelativeRedirects(boolean useRelativeRedirects) {
this.useRelativeRedirects = useRelativeRedirects;
}

@Deprecated
public void setUseRelativeRedirects(Boolean useRelativeRedirects) {
this.useRelativeRedirects = (useRelativeRedirects != null) ? useRelativeRedirects : false;
}

public String getRemoteIpHeader() {
return this.remoteIpHeader;
}
Expand Down
Expand Up @@ -54,9 +54,7 @@ public void customize(TomcatServletWebServerFactory factory) {
if (tomcatProperties.getRedirectContextRoot() != null) {
customizeRedirectContextRoot(factory, tomcatProperties.getRedirectContextRoot());
}
if (tomcatProperties.getUseRelativeRedirects() != null) {
customizeUseRelativeRedirects(factory, tomcatProperties.getUseRelativeRedirects());
}
customizeUseRelativeRedirects(factory, tomcatProperties.getUseRelativeRedirects());
factory.setDisableMBeanRegistry(!tomcatProperties.getMbeanregistry().isEnabled());
}

Expand Down
Expand Up @@ -131,6 +131,7 @@ void testTomcatBinding() {
map.put("server.tomcat.background-processor-delay", "10");
map.put("server.tomcat.relaxed-path-chars", "|,<");
map.put("server.tomcat.relaxed-query-chars", "^ , | ");
map.put("server.tomcat.use-relative-redirects", "true");
bind(map);
ServerProperties.Tomcat tomcat = this.properties.getTomcat();
Accesslog accesslog = tomcat.getAccesslog();
Expand All @@ -152,6 +153,7 @@ void testTomcatBinding() {
assertThat(tomcat.getBackgroundProcessorDelay()).isEqualTo(Duration.ofSeconds(10));
assertThat(tomcat.getRelaxedPathChars()).containsExactly('|', '<');
assertThat(tomcat.getRelaxedQueryChars()).containsExactly('^', '|');
assertThat(tomcat.getUseRelativeRedirects()).isTrue();
}

@Test
Expand Down Expand Up @@ -347,6 +349,11 @@ void tomcatInternalProxiesMatchesDefault() {
.isEqualTo(new RemoteIpValve().getInternalProxies());
}

@Test
void tomcatUseRelativeRedirectsDefaultsToFalse() {
assertThat(this.properties.getTomcat().getUseRelativeRedirects()).isFalse();
}

@Test
void jettyMaxHttpFormPostSizeMatchesDefault() throws Exception {
JettyServletWebServerFactory jettyFactory = new JettyServletWebServerFactory(0);
Expand Down
Expand Up @@ -214,7 +214,6 @@ protected void prepareContext(Host host, ServletContextInitializer[] initializer
: ClassUtils.getDefaultClassLoader());
resetDefaultLocaleMapping(context);
addLocaleMappings(context);
context.setUseRelativeRedirects(false);
try {
context.setCreateUploadTargets(true);
}
Expand Down

0 comments on commit f29bce6

Please sign in to comment.