Skip to content

Commit

Permalink
Addressed code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@rhamedy
rhamedy committed Oct 14, 2019
1 parent fd954d6 commit 270a6ef
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 114 deletions.
24 changes: 6 additions & 18 deletions ...oconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java
View file
Expand Up @@ -325,11 +325,6 @@ public static class Tomcat {
*/
private int minSpareThreads = 10;

/**
* Maximum size of the HTTP post content.
*/
private DataSize maxHttpPostSize = DataSize.ofMegabytes(2);

/**
* Maximum size of the HTTP message header.
*/
Expand Down Expand Up @@ -403,14 +398,13 @@ public void setMinSpareThreads(int minSpareThreads) {
this.minSpareThreads = minSpareThreads;
}

@DeprecatedConfigurationProperty(reason = "replaced because it was misleadingly named",
replacement = "server.tomcat.max-http-form-post-size")
@DeprecatedConfigurationProperty(replacement = "server.tomcat.max-http-form-post-size")
public DataSize getMaxHttpPostSize() {
return this.maxHttpPostSize;
return this.maxHttpFormPostSize;
}

public void setMaxHttpPostSize(DataSize maxHttpPostSize) {
this.maxHttpPostSize = maxHttpPostSize;
this.maxHttpFormPostSize = maxHttpPostSize;
}

public DataSize getMaxHttpFormPostSize() {
Expand Down Expand Up @@ -737,11 +731,6 @@ public static class Jetty {
*/
private final Accesslog accesslog = new Accesslog();

/**
* Maximum size of the HTTP post or put content.
*/
private DataSize maxHttpPostSize = DataSize.ofBytes(200000);

/**
* Maximum size of the form content in any HTTP post request.
*/
Expand All @@ -763,14 +752,13 @@ public Accesslog getAccesslog() {
return this.accesslog;
}

@DeprecatedConfigurationProperty(reason = "replaced because it was misleadingly named",
replacement = "server.jetty.max-http-form-post-size")
@DeprecatedConfigurationProperty(replacement = "server.jetty.max-http-form-post-size")
public DataSize getMaxHttpPostSize() {
return this.maxHttpPostSize;
return this.maxHttpFormPostSize;
}

public void setMaxHttpPostSize(DataSize maxHttpPostSize) {
this.maxHttpPostSize = maxHttpPostSize;
this.maxHttpFormPostSize = maxHttpPostSize;
}

public DataSize getMaxHttpFormPostSize() {
Expand Down
27 changes: 0 additions & 27 deletions .../org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java
View file
Expand Up @@ -75,8 +75,6 @@ public void customize(ConfigurableJettyWebServerFactory factory) {
propertyMapper.from(properties::getMaxHttpHeaderSize).whenNonNull().asInt(DataSize::toBytes)
.when(this::isPositive).to((maxHttpHeaderSize) -> factory
.addServerCustomizers(new MaxHttpHeaderSizeCustomizer(maxHttpHeaderSize)));
propertyMapper.from(jettyProperties::getMaxHttpPostSize).asInt(DataSize::toBytes).when(this::isPositive)
.to((maxHttpPostSize) -> customizeMaxHttpPostSize(factory, maxHttpPostSize));
propertyMapper.from(jettyProperties::getMaxHttpFormPostSize).asInt(DataSize::toBytes).when(this::isPositive)
.to((maxHttpFormPostSize) -> customizeMaxHttpFormPostSize(factory, maxHttpFormPostSize));
propertyMapper.from(properties::getConnectionTimeout).whenNonNull()
Expand Down Expand Up @@ -107,31 +105,6 @@ private void customizeConnectionTimeout(ConfigurableJettyWebServerFactory factor
});
}

private void customizeMaxHttpPostSize(ConfigurableJettyWebServerFactory factory, int maxHttpPostSize) {
factory.addServerCustomizers(new JettyServerCustomizer() {

@Override
public void customize(Server server) {
setHandlerMaxHttpPostSize(maxHttpPostSize, server.getHandlers());
}

private void setHandlerMaxHttpPostSize(int maxHttpPostSize, Handler... handlers) {
for (Handler handler : handlers) {
if (handler instanceof ContextHandler) {
((ContextHandler) handler).setMaxFormContentSize(maxHttpPostSize);
}
else if (handler instanceof HandlerWrapper) {
setHandlerMaxHttpPostSize(maxHttpPostSize, ((HandlerWrapper) handler).getHandler());
}
else if (handler instanceof HandlerCollection) {
setHandlerMaxHttpPostSize(maxHttpPostSize, ((HandlerCollection) handler).getHandlers());
}
}
}

});
}

private void customizeMaxHttpFormPostSize(ConfigurableJettyWebServerFactory factory, int maxHttpFormPostSize) {
factory.addServerCustomizers(new JettyServerCustomizer() {

Expand Down
7 changes: 0 additions & 7 deletions ...org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java
View file
Expand Up @@ -87,9 +87,6 @@ public void customize(ConfigurableTomcatWebServerFactory factory) {
.to((maxHttpHeaderSize) -> customizeMaxHttpHeaderSize(factory, maxHttpHeaderSize));
propertyMapper.from(tomcatProperties::getMaxSwallowSize).whenNonNull().asInt(DataSize::toBytes)
.to((maxSwallowSize) -> customizeMaxSwallowSize(factory, maxSwallowSize));
propertyMapper.from(tomcatProperties::getMaxHttpPostSize).asInt(DataSize::toBytes)
.when((maxHttpPostSize) -> maxHttpPostSize != 0)
.to((maxHttpPostSize) -> customizeMaxHttpPostSize(factory, maxHttpPostSize));
propertyMapper.from(tomcatProperties::getMaxHttpFormPostSize).asInt(DataSize::toBytes)
.when((maxHttpFormPostSize) -> maxHttpFormPostSize != 0)
.to((maxHttpFormPostSize) -> customizeMaxHttpFormPostSize(factory, maxHttpFormPostSize));
Expand Down Expand Up @@ -220,10 +217,6 @@ private void customizeMaxSwallowSize(ConfigurableTomcatWebServerFactory factory,
});
}

private void customizeMaxHttpPostSize(ConfigurableTomcatWebServerFactory factory, int maxHttpPostSize) {
factory.addConnectorCustomizers((connector) -> connector.setMaxPostSize(maxHttpPostSize));
}

private void customizeMaxHttpFormPostSize(ConfigurableTomcatWebServerFactory factory, int maxHttpFormPostSize) {
factory.addConnectorCustomizers((connector) -> connector.setMaxPostSize(maxHttpFormPostSize));
}
Expand Down
60 changes: 0 additions & 60 deletions ...igure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java
View file
Expand Up @@ -262,66 +262,6 @@ public void tomcatInternalProxiesMatchesDefault() {
.isEqualTo(new RemoteIpValve().getInternalProxies());
}

@Test
public void jettyMaxHttpPostSizeMatchesDefault() throws Exception {
JettyServletWebServerFactory jettyFactory = new JettyServletWebServerFactory(0);
JettyWebServer jetty = (JettyWebServer) jettyFactory
.getWebServer((ServletContextInitializer) (servletContext) -> servletContext
.addServlet("formPost", new HttpServlet() {

@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
req.getParameterMap();
}

}).addMapping("/form"));
jetty.start();
org.eclipse.jetty.server.Connector connector = jetty.getServer().getConnectors()[0];
final AtomicReference<Throwable> failure = new AtomicReference<>();
connector.addBean(new HttpChannel.Listener() {

@Override
public void onDispatchFailure(Request request, Throwable ex) {
failure.set(ex);
}

});
try {
RestTemplate template = new RestTemplate();
template.setErrorHandler(new ResponseErrorHandler() {

@Override
public boolean hasError(ClientHttpResponse response) throws IOException {
return false;
}

@Override
public void handleError(ClientHttpResponse response) throws IOException {

}

});
HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
MultiValueMap<String, Object> body = new LinkedMultiValueMap<>();
StringBuilder data = new StringBuilder();
for (int i = 0; i < 250000; i++) {
data.append("a");
}
body.add("data", data.toString());
HttpEntity<MultiValueMap<String, Object>> entity = new HttpEntity<>(body, headers);
template.postForEntity(URI.create("http://localhost:" + jetty.getPort() + "/form"), entity, Void.class);
assertThat(failure.get()).isNotNull();
String message = failure.get().getCause().getMessage();
int defaultMaxPostSize = Integer.valueOf(message.substring(message.lastIndexOf(' ')).trim());
assertThat(this.properties.getJetty().getMaxHttpPostSize().toBytes()).isEqualTo(defaultMaxPostSize);
}
finally {
jetty.stop();
}
}

@Test
public void jettyMaxHttpFormPostSizeMatchesDefault() throws Exception {
JettyServletWebServerFactory jettyFactory = new JettyServletWebServerFactory(0);
Expand Down
2 changes: 0 additions & 2 deletions ...pringframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java
View file
Expand Up @@ -93,7 +93,6 @@ public void customBackgroundProcessorDelay() {
@Test
public void customDisableMaxHttpPostSize() {
bind("server.tomcat.max-http-post-size=-1");
bind("server.tomcat.max-http-form-post-size=-1");
customizeAndRunServer((server) -> assertThat(server.getTomcat().getConnector().getMaxPostSize()).isEqualTo(-1));
}

Expand All @@ -114,7 +113,6 @@ public void customMaxConnections() {
@Test
public void customMaxHttpPostSize() {
bind("server.tomcat.max-http-post-size=10000");
bind("server.tomcat.max-http-form-post-size=10000");
customizeAndRunServer(
(server) -> assertThat(server.getTomcat().getConnector().getMaxPostSize()).isEqualTo(10000));
}
Expand Down

0 comments on commit 270a6ef

Please sign in to comment.