Skip to content

Commit fdaaa3b

Browse files
carterkozakfl4via
carterkozak
authored and
fl4via
committed
UNDERTOW-2207: UndertowOutputStream allows flush after writes
Previously, setting Content-Length would auto-close the stream in such a way that a flush after the final write would throw an IOException due to internal self-closure.
1 parent 036ac6f commit fdaaa3b

File tree

3 files changed

+239
-3
lines changed

3 files changed

+239
-3
lines changed

Diff for: ‎core/src/main/java/io/undertow/io/UndertowOutputStream.java

+10-3
Original file line numberDiff line numberDiff line change
@@ -272,11 +272,18 @@ void updateWritten(final long len) throws IOException {
272272
* {@inheritDoc}
273273
*/
274274
public void flush() throws IOException {
275-
if (anyAreSet(state, FLAG_CLOSED)) {
276-
throw UndertowMessages.MESSAGES.streamIsClosed();
277-
}
275+
boolean closed = anyAreSet(state, FLAG_CLOSED);
278276
if (buffer != null && buffer.position() != 0) {
277+
if (closed) {
278+
throw UndertowMessages.MESSAGES.streamIsClosed();
279+
}
279280
writeBufferBlocking(false);
281+
} else if (closed) {
282+
// No-op if flush is called with no buffered data on a closed channel.
283+
// This stream closes itself when a Content-Length is provided, once sufficient
284+
// bytes have been written -- a flush following the last write is entirely
285+
// reasonable.
286+
return;
280287
}
281288
if (channel == null) {
282289
channel = exchange.getResponseChannel();

Diff for: ‎core/src/test/java/io/undertow/server/handlers/blocking/BlockingServerStreamClosureTestCase.java

+119
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
* JBoss, Home of Professional Open Source.
3+
* Copyright 2022 Red Hat, Inc., and individual contributors
4+
* as indicated by the @author tags.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package io.undertow.server.handlers.blocking;
20+
21+
import io.undertow.server.handlers.BlockingHandler;
22+
import io.undertow.testutils.DefaultServer;
23+
import io.undertow.testutils.HttpClientUtils;
24+
import io.undertow.testutils.TestHttpClient;
25+
import io.undertow.util.Headers;
26+
import io.undertow.util.StatusCodes;
27+
import org.apache.http.HttpResponse;
28+
import org.apache.http.client.methods.HttpGet;
29+
import org.junit.Assert;
30+
import org.junit.Test;
31+
import org.junit.runner.RunWith;
32+
33+
import java.io.IOException;
34+
import java.io.OutputStream;
35+
import java.util.concurrent.atomic.AtomicReference;
36+
37+
/**
38+
* @author Carter Kozak
39+
*/
40+
@RunWith(DefaultServer.class)
41+
public class BlockingServerStreamClosureTestCase {
42+
43+
@Test
44+
public void testFlushAfterContentLengthReached() throws IOException {
45+
AtomicReference<Throwable> thrown = new AtomicReference<>();
46+
DefaultServer.setRootHandler(new BlockingHandler(exchange -> {
47+
exchange.getResponseHeaders().put(Headers.CONTENT_LENGTH, "1");
48+
try {
49+
OutputStream out = exchange.getOutputStream();
50+
out.write(65);
51+
out.flush();
52+
out.close();
53+
} catch (Throwable t) {
54+
thrown.set(t);
55+
throw t;
56+
}
57+
}));
58+
makeSuccessfulRequest("A");
59+
Throwable maybeFailure = thrown.get();
60+
if (maybeFailure != null) {
61+
throw new AssertionError("Unexpected failure", maybeFailure);
62+
}
63+
}
64+
65+
@Test
66+
public void testEmptyWriteAfterContentLength() throws IOException {
67+
AtomicReference<Throwable> thrown = new AtomicReference<>();
68+
DefaultServer.setRootHandler(new BlockingHandler(exchange -> {
69+
exchange.getResponseHeaders().put(Headers.CONTENT_LENGTH, "1");
70+
try {
71+
OutputStream out = exchange.getOutputStream();
72+
out.write(65);
73+
out.write(new byte[10], 2, 0);
74+
out.close();
75+
} catch (Throwable t) {
76+
thrown.set(t);
77+
throw t;
78+
}
79+
}));
80+
makeSuccessfulRequest("A");
81+
Throwable maybeFailure = thrown.get();
82+
if (maybeFailure != null) {
83+
throw new AssertionError("Unexpected failure", maybeFailure);
84+
}
85+
}
86+
87+
@Test
88+
public void testFlushAfterClose() throws IOException {
89+
AtomicReference<Throwable> thrown = new AtomicReference<>();
90+
DefaultServer.setRootHandler(new BlockingHandler(exchange -> {
91+
try {
92+
OutputStream out = exchange.getOutputStream();
93+
out.write(65);
94+
out.close();
95+
out.flush();
96+
} catch (Throwable t) {
97+
thrown.set(t);
98+
throw t;
99+
}
100+
}));
101+
makeSuccessfulRequest("A");
102+
Throwable maybeFailure = thrown.get();
103+
if (maybeFailure != null) {
104+
throw new AssertionError("Unexpected failure", maybeFailure);
105+
}
106+
}
107+
108+
private static void makeSuccessfulRequest(String expectedContent) throws IOException {
109+
TestHttpClient client = new TestHttpClient();
110+
try {
111+
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL());
112+
HttpResponse result = client.execute(get);
113+
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
114+
Assert.assertEquals(expectedContent, HttpClientUtils.readResponse(result));
115+
} finally {
116+
client.getConnectionManager().shutdown();
117+
}
118+
}
119+
}

Diff for: ‎servlet/src/test/java/io/undertow/servlet/test/streams/ServletOutputStreamClosureTestCase.java

+110
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* JBoss, Home of Professional Open Source.
3+
* Copyright 2022 Red Hat, Inc., and individual contributors
4+
* as indicated by the @author tags.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package io.undertow.servlet.test.streams;
20+
21+
import io.undertow.servlet.api.ServletInfo;
22+
import io.undertow.servlet.test.util.DeploymentUtils;
23+
import io.undertow.servlet.util.ImmediateInstanceFactory;
24+
import io.undertow.testutils.DefaultServer;
25+
import io.undertow.testutils.HttpClientUtils;
26+
import io.undertow.testutils.TestHttpClient;
27+
import io.undertow.util.Headers;
28+
import io.undertow.util.StatusCodes;
29+
import org.apache.http.HttpResponse;
30+
import org.apache.http.client.methods.HttpGet;
31+
import org.junit.Assert;
32+
import org.junit.Test;
33+
import org.junit.runner.RunWith;
34+
35+
import javax.servlet.ServletOutputStream;
36+
import javax.servlet.http.HttpServlet;
37+
import javax.servlet.http.HttpServletRequest;
38+
import javax.servlet.http.HttpServletResponse;
39+
import java.io.IOException;
40+
import java.util.concurrent.atomic.AtomicReference;
41+
42+
/**
43+
* @author Carter Kozak
44+
*/
45+
@RunWith(DefaultServer.class)
46+
public class ServletOutputStreamClosureTestCase {
47+
48+
@Test
49+
public void testFlushAfterContentLengthReached() throws IOException {
50+
AtomicReference<Throwable> thrown = new AtomicReference<>();
51+
DeploymentUtils.setupServlet(
52+
new ServletInfo("servlet", HttpServlet.class, new ImmediateInstanceFactory<HttpServlet>(new HttpServlet() {
53+
@Override
54+
protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
55+
resp.setHeader(Headers.CONTENT_LENGTH_STRING, "1");
56+
try {
57+
ServletOutputStream out = resp.getOutputStream();
58+
out.write(65);
59+
out.flush();
60+
out.close();
61+
} catch (Throwable t) {
62+
thrown.set(t);
63+
throw t;
64+
}
65+
}
66+
})).addMapping("/*"));
67+
makeSuccessfulRequest("A");
68+
Throwable maybeFailure = thrown.get();
69+
if (maybeFailure != null) {
70+
throw new AssertionError("Unexpected failure", maybeFailure);
71+
}
72+
}
73+
74+
@Test
75+
public void testFlushAfterClose() throws IOException {
76+
AtomicReference<Throwable> thrown = new AtomicReference<>();
77+
DeploymentUtils.setupServlet(
78+
new ServletInfo("servlet", HttpServlet.class, new ImmediateInstanceFactory<HttpServlet>(new HttpServlet() {
79+
@Override
80+
protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
81+
try {
82+
ServletOutputStream out = resp.getOutputStream();
83+
out.write(65);
84+
out.close();
85+
out.flush();
86+
} catch (Throwable t) {
87+
thrown.set(t);
88+
throw t;
89+
}
90+
}
91+
})).addMapping("/*"));
92+
makeSuccessfulRequest("A");
93+
Throwable maybeFailure = thrown.get();
94+
if (maybeFailure != null) {
95+
throw new AssertionError("Unexpected failure", maybeFailure);
96+
}
97+
}
98+
99+
private static void makeSuccessfulRequest(String expectedContent) throws IOException {
100+
TestHttpClient client = new TestHttpClient();
101+
try {
102+
HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/servletContext");
103+
HttpResponse result = client.execute(get);
104+
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
105+
Assert.assertEquals(expectedContent, HttpClientUtils.readResponse(result));
106+
} finally {
107+
client.getConnectionManager().shutdown();
108+
}
109+
}
110+
}

0 commit comments

Comments
 (0)