-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Potential connection leak if Zuul "post" filter throws exception befo…
…re SendResponseFilter being executed.
- Loading branch information
Denys Kurylenko
committed
Jul 21, 2016
1 parent
afb43c5
commit ddd57b3
Showing
4 changed files
with
251 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
68 changes: 68 additions & 0 deletions
68
...ava/org/springframework/cloud/netflix/zuul/filters/error/ReleaseResourcesErrorFilter.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* Copyright 2013-2015 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. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package org.springframework.cloud.netflix.zuul.filters.error; | ||
|
||
import com.netflix.zuul.ZuulFilter; | ||
import com.netflix.zuul.context.RequestContext; | ||
|
||
import java.io.IOException; | ||
import java.io.InputStream; | ||
|
||
/** | ||
* Releases underneath connections created by "route" filters, but not released by "post" filters. | ||
* | ||
* In most exceptional cases "error" filters are getting executed before "post" filters, therefore closing input stream blindly might lead to | ||
* unpredictable behaviour of "post" filters. | ||
* | ||
* This filter coordinates work with {@link org.springframework.cloud.netflix.zuul.filters.post.PrePostFilter} to give "post" filters chance to be executed first | ||
* | ||
* TODO: Better fix would to close input stream in {@link com.netflix.zuul.http.ZuulServlet} or {@link org.springframework.cloud.netflix.zuul.web.ZuulController} | ||
* | ||
* @author Denys Kurylenko | ||
*/ | ||
public class ReleaseResourcesErrorFilter extends ZuulFilter { | ||
|
||
@Override | ||
public String filterType() { | ||
return "error"; | ||
} | ||
|
||
@Override | ||
public int filterOrder() { | ||
return 0; | ||
} | ||
|
||
@Override | ||
public boolean shouldFilter() { | ||
// don't close connection unless "post" filters attempted to read the data | ||
final RequestContext ctx = RequestContext.getCurrentContext(); | ||
return ctx.getResponseDataStream() != null && (ctx.getResponse().isCommitted() || ctx.getBoolean("post.filters.started")); | ||
} | ||
|
||
@Override | ||
public Object run() { | ||
final InputStream is = RequestContext.getCurrentContext().getResponseDataStream(); | ||
try { | ||
if (is != null) { | ||
is.close(); | ||
} | ||
} | ||
catch (IOException ex) { | ||
} | ||
return null; | ||
} | ||
} |
49 changes: 49 additions & 0 deletions
49
...core/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/PrePostFilter.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* Copyright 2013-2015 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. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package org.springframework.cloud.netflix.zuul.filters.post; | ||
|
||
import com.netflix.zuul.ZuulFilter; | ||
import com.netflix.zuul.context.RequestContext; | ||
|
||
/** | ||
* Indicate that "post" filter phase started. | ||
* | ||
* @author Denys Kurylenko | ||
*/ | ||
public class PrePostFilter extends ZuulFilter { | ||
@Override | ||
public String filterType() { | ||
return "post"; | ||
} | ||
|
||
@Override | ||
public int filterOrder() { | ||
return -1000; | ||
} | ||
|
||
@Override | ||
public boolean shouldFilter() { | ||
return true; | ||
} | ||
|
||
@Override | ||
public Object run() { | ||
final RequestContext ctx = RequestContext.getCurrentContext(); | ||
ctx.set("post.filters.started", Boolean.TRUE); | ||
return null; | ||
} | ||
} |
122 changes: 122 additions & 0 deletions
122
.../org/springframework/cloud/netflix/zuul/filters/post/SendErrorFilterIntegrationTests.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
package org.springframework.cloud.netflix.zuul.filters.post; | ||
|
||
import com.netflix.zuul.ZuulFilter; | ||
import com.netflix.zuul.context.RequestContext; | ||
import org.apache.commons.io.input.ProxyInputStream; | ||
import org.apache.commons.lang3.RandomStringUtils; | ||
import org.junit.Test; | ||
import org.junit.runner.RunWith; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.boot.autoconfigure.EnableAutoConfiguration; | ||
import org.springframework.boot.test.IntegrationTest; | ||
import org.springframework.boot.test.SpringApplicationConfiguration; | ||
import org.springframework.boot.test.TestRestTemplate; | ||
import org.springframework.cloud.netflix.ribbon.RibbonClient; | ||
import org.springframework.cloud.netflix.ribbon.RibbonClients; | ||
import org.springframework.cloud.netflix.zuul.EnableZuulProxy; | ||
import org.springframework.cloud.netflix.zuul.filters.RouteLocator; | ||
import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; | ||
import org.springframework.context.annotation.Bean; | ||
import org.springframework.context.annotation.Configuration; | ||
import org.springframework.http.HttpEntity; | ||
import org.springframework.http.HttpMethod; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.test.annotation.DirtiesContext; | ||
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; | ||
import org.springframework.test.context.web.WebAppConfiguration; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
import java.io.IOException; | ||
import java.io.InputStream; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
|
||
/** | ||
* @author Denys Kurylenko | ||
*/ | ||
@RunWith(SpringJUnit4ClassRunner.class) | ||
@SpringApplicationConfiguration(classes = SendErrorFilterIntegrationTests.TestConfig.class) | ||
@WebAppConfiguration | ||
@IntegrationTest({"server.port: 0", | ||
"zuul.routes.simple: /simple/**"}) | ||
@DirtiesContext | ||
public class SendErrorFilterIntegrationTests { | ||
|
||
@Value("${local.server.port}") | ||
protected int port; | ||
|
||
@Autowired | ||
RouteLocator routeLocator; | ||
|
||
@Test | ||
public void responseDataStreamGettingClosed() { | ||
ResponseEntity<String> result = new TestRestTemplate().exchange( | ||
"http://localhost:" + this.port + "/simple", | ||
HttpMethod.GET, new HttpEntity<>((Void) null), String.class); | ||
assertEquals("true", result.getHeaders().getFirst("x-response-stream-closed")); | ||
} | ||
|
||
// Don't use @SpringBootApplication because we don't want to component scan | ||
@Configuration | ||
@EnableAutoConfiguration | ||
@RestController | ||
@EnableZuulProxy | ||
@RibbonClients({ | ||
@RibbonClient(name = "simple", configuration = ZuulProxyTestBase.SimpleRibbonClientConfiguration.class) }) | ||
static class TestConfig extends ZuulProxyTestBase.AbstractZuulProxyApplication | ||
{ | ||
@Bean | ||
public ZuulFilter bogusPostFilter(final SendErrorFilter sendErrorFilter) | ||
{ | ||
return new ZuulFilter() | ||
{ | ||
@Override | ||
public boolean shouldFilter() | ||
{ | ||
return true; | ||
} | ||
|
||
@Override | ||
public Object run() | ||
{ | ||
final RequestContext ctx = RequestContext.getCurrentContext(); | ||
final InputStream responseDataStream = ctx.getResponseDataStream(); | ||
if (responseDataStream != null) | ||
{ | ||
ctx.setResponseDataStream(new CloseAwareInputStream(responseDataStream)); | ||
} | ||
throw new IllegalStateException("Internal exception"); | ||
} | ||
|
||
@Override | ||
public String filterType() | ||
{ | ||
return "post"; | ||
} | ||
|
||
@Override | ||
public int filterOrder() | ||
{ | ||
return sendErrorFilter.filterOrder() - 10; | ||
} | ||
}; | ||
} | ||
} | ||
|
||
static class CloseAwareInputStream extends ProxyInputStream | ||
{ | ||
public CloseAwareInputStream(InputStream proxy) | ||
{ | ||
super(proxy); | ||
} | ||
|
||
@Override | ||
public void close() throws IOException | ||
{ | ||
RequestContext.getCurrentContext().getResponse().setHeader("x-response-stream-closed", "true"); | ||
super.close(); | ||
} | ||
} | ||
} |