Skip to content

Commit 48c58a3

Browse files
mpkorstanjempkorstanje
committed
[Core] UrlOutputStream uploads all data in one pass
Chunked upload requires a connection to be kept alive for the duration of the test execution. This is practically not feasible. So rather then using chunked upload the output is written to a temporary file and then uploaded all at once.
1 parent ba13e24 commit 48c58a3

File tree

2 files changed

+80
-62
lines changed

2 files changed

+80
-62
lines changed

‎core/src/main/java/io/cucumber/core/plugin/UrlOutputStream.java

+52-47
Original file line numberDiff line numberDiff line change
@@ -9,94 +9,97 @@
99
import java.io.OutputStream;
1010
import java.net.HttpURLConnection;
1111
import java.net.URL;
12+
import java.nio.file.Files;
13+
import java.nio.file.Path;
1214
import java.util.List;
1315
import java.util.Map;
1416
import java.util.Map.Entry;
1517
import java.util.stream.Collectors;
1618

1719
import static java.nio.charset.StandardCharsets.UTF_8;
20+
import static java.nio.file.Files.newOutputStream;
1821

1922
class UrlOutputStream extends OutputStream {
20-
// Allow streaming, using a chunk size that is similar to a typical NDJSON
21-
// message length
22-
public static final int CHUNK_LENGTH = 256;
23-
private final HttpURLConnection urlConnection;
24-
private final OutputStream outputStream;
25-
26-
private String method;
27-
private URL url;
28-
private final Map<String, List<String>> requestHeaders;
23+
private final CurlOption option;
24+
private final Path temp;
25+
private final OutputStream tempOutputStream;
2926

3027
UrlOutputStream(CurlOption option) throws IOException {
31-
this.method = option.getMethod().name();
32-
this.url = option.getUri().toURL();
33-
34-
urlConnection = (HttpURLConnection) this.url.openConnection();
35-
for (Entry<String, String> header : option.getHeaders()) {
36-
urlConnection.setRequestProperty(header.getKey(), header.getValue());
37-
}
38-
urlConnection.setRequestMethod(this.method);
39-
urlConnection.setDoOutput(true);
40-
urlConnection.setChunkedStreamingMode(CHUNK_LENGTH);
41-
urlConnection.setInstanceFollowRedirects(false);
42-
requestHeaders = urlConnection.getRequestProperties();
43-
outputStream = urlConnection.getOutputStream();
28+
this.option = option;
29+
this.temp = Files.createTempFile("cucumber", null);
30+
this.tempOutputStream = newOutputStream(temp);
4431
}
4532

4633
@Override
4734
public void write(byte[] buffer, int offset, int count) throws IOException {
48-
outputStream.write(buffer, offset, count);
35+
tempOutputStream.write(buffer, offset, count);
4936
}
5037

5138
@Override
5239
public void write(byte[] buffer) throws IOException {
53-
outputStream.write(buffer);
40+
tempOutputStream.write(buffer);
5441
}
5542

5643
@Override
5744
public void write(int b) throws IOException {
58-
outputStream.write(b);
45+
tempOutputStream.write(b);
5946
}
6047

6148
@Override
6249
public void flush() throws IOException {
63-
outputStream.flush();
50+
tempOutputStream.flush();
6451
}
6552

6653
@Override
6754
public void close() throws IOException {
68-
outputStream.close();
69-
int httpStatus = urlConnection.getResponseCode();
70-
boolean redirect = httpStatus >= 300 && httpStatus < 400;
71-
boolean error = httpStatus >= 400;
72-
try (InputStream inputStream = error ? urlConnection.getErrorStream() : urlConnection.getInputStream()) {
73-
Map<String, List<String>> responseHeaders = urlConnection.getHeaderFields();
74-
try (BufferedReader br = new BufferedReader(new InputStreamReader(inputStream, UTF_8))) {
75-
String responseBody = br.lines().collect(Collectors.joining(System.lineSeparator()));
76-
if (error || redirect) {
77-
String message = generateCurlLikeMessage(this.method, this.url, this.requestHeaders, responseHeaders, responseBody, redirect);
78-
throw new IOException(message);
79-
}
55+
tempOutputStream.close();
56+
57+
URL url = option.getUri().toURL();
58+
HttpURLConnection urlConnection = (HttpURLConnection) url.openConnection();
59+
for (Entry<String, String> header : option.getHeaders()) {
60+
urlConnection.setRequestProperty(header.getKey(), header.getValue());
61+
}
62+
urlConnection.setInstanceFollowRedirects(true);
63+
urlConnection.setRequestMethod(option.getMethod().name());
64+
urlConnection.setDoOutput(true);
65+
Map<String, List<String>> requestHeaders = urlConnection.getRequestProperties();
66+
try (OutputStream outputStream = urlConnection.getOutputStream()) {
67+
Files.copy(temp, outputStream);
68+
handleResponse(urlConnection, requestHeaders);
69+
}
70+
}
71+
72+
private static void handleResponse(HttpURLConnection urlConnection, Map<String, List<String>> requestHeaders) throws IOException {
73+
Map<String, List<String>> responseHeaders = urlConnection.getHeaderFields();
74+
int responseCode = urlConnection.getResponseCode();
75+
boolean success = 200 <= responseCode && responseCode < 300;
76+
77+
InputStream inputStream = urlConnection.getErrorStream() != null ? urlConnection.getErrorStream() : urlConnection.getInputStream();
78+
try (BufferedReader br = new BufferedReader(new InputStreamReader(inputStream, UTF_8))) {
79+
String responseBody = br.lines().collect(Collectors.joining(System.lineSeparator()));
80+
if (!success) {
81+
String method = urlConnection.getRequestMethod();
82+
URL url = urlConnection.getURL();
83+
throw createCurlLikeException(method, url, requestHeaders, responseHeaders, responseBody);
8084
}
8185
}
8286
}
8387

84-
static String generateCurlLikeMessage(
88+
static IOException createCurlLikeException(
8589
String method,
8690
URL url,
8791
Map<String, List<String>> requestHeaders,
8892
Map<String, List<String>> responseHeaders,
89-
String responseBody,
90-
boolean redirect) {
91-
return String.format(
92-
"%s:\n> %s %s\n%s%s\n%s",
93-
redirect ? "HTTP redirect not supported" : "HTTP request failed",
93+
String responseBody) {
94+
return new IOException(String.format(
95+
"%s:\n> %s %s%s%s%s",
96+
"HTTP request failed",
9497
method,
9598
url,
9699
headersToString("> ", requestHeaders),
97100
headersToString("< ", responseHeaders),
98101
responseBody
99-
);
102+
));
100103
}
101104

102105
private static String headersToString(String prefix, Map<String, List<String>> headers) {
@@ -109,10 +112,12 @@ private static String headersToString(String prefix, Map<String, List<String>> h
109112
.map(value -> {
110113
if (header.getKey() == null) {
111114
return prefix + value;
115+
} else if (header.getValue() == null) {
116+
return prefix + header.getKey();
112117
} else {
113-
return prefix + (header.getKey() + ": ") + value;
118+
return prefix + header.getKey() + ": " + value;
114119
}
115120
})
116-
).collect(Collectors.joining("\n"));
121+
).collect(Collectors.joining("\n", "", "\n"));
117122
}
118123
}

‎core/src/test/java/io/cucumber/core/plugin/UrlOutputStreamTest.java

+28-15
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
@ExtendWith({VertxExtension.class})
3232
public class UrlOutputStreamTest {
3333
private int port;
34-
private IOException exception;
34+
private Exception exception;
3535

3636
@BeforeEach
3737
void randomPort() throws IOException {
@@ -56,18 +56,28 @@ void throws_exception_for_500_status(Vertx vertx, VertxTestContext testContext)
5656
}
5757

5858
@Test
59-
void throws_exception_for_redirects(Vertx vertx, VertxTestContext testContext) throws InterruptedException {
59+
void follows_307_temporary_redirects(Vertx vertx, VertxTestContext testContext) throws InterruptedException {
6060
String requestBody = "hello";
6161
TestServer testServer = new TestServer(port, testContext, requestBody, HttpMethod.POST, null, "application/x-www-form-urlencoded", 200, "");
6262
CurlOption url = CurlOption.parse(format("http://localhost:%d/redirect", port));
6363
verifyRequest(url, testServer, vertx, testContext, requestBody);
6464

6565
assertThat(testContext.awaitCompletion(5, TimeUnit.SECONDS), is(true));
66-
assertThat(exception.getMessage(), is(equalTo("HTTP redirect not supported:\n" +
67-
"> POST http://localhost:" + port + "/redirect\n" +
68-
"< HTTP/1.1 301 Moved Permanently\n" +
69-
"< content-length: 0\n" +
70-
"< Location: /\n")));
66+
}
67+
68+
@Test
69+
void follows_307_temporary_redirect_without_location(Vertx vertx, VertxTestContext testContext) throws InterruptedException {
70+
String requestBody = "hello";
71+
TestServer testServer = new TestServer(port, testContext, requestBody, HttpMethod.POST, null, "application/x-www-form-urlencoded", 200, "");
72+
CurlOption url = CurlOption.parse(format("http://localhost:%d/redirect-no-location", port));
73+
verifyRequest(url, testServer, vertx, testContext, requestBody);
74+
75+
assertThat(testContext.awaitCompletion(5, TimeUnit.SECONDS), is(true));
76+
assertThat(exception.getMessage(), is(equalTo("HTTP request failed:\n" +
77+
"> POST http://localhost:" + port + "/redirect-no-location\n" +
78+
"< HTTP/1.1 307 Temporary Redirect\n" +
79+
"< content-length: 0\n"
80+
)));
7181
}
7282

7383
@Test
@@ -103,7 +113,7 @@ private void verifyRequest(CurlOption url, TestServer testServer, Vertx vertx, V
103113
w.flush();
104114
w.close();
105115
testContext.completeNow();
106-
} catch (IOException e) {
116+
} catch (Exception e) {
107117
exception = e;
108118
testContext.completeNow();
109119
}
@@ -147,15 +157,18 @@ public TestServer(
147157
@Override
148158
public void start(Promise<Void> startPromise) {
149159
Router router = Router.router(vertx);
160+
router.route("/redirect").handler(ctx -> {
161+
ctx.response().setStatusCode(307);
162+
ctx.response().headers().add("Location", "http://localhost:" + port);
163+
ctx.response().end();
164+
});
165+
router.route("/redirect-no-location").handler(ctx -> {
166+
ctx.response().setStatusCode(307);
167+
ctx.response().end();
168+
});
169+
150170
router.route().handler(ctx -> {
151-
if (ctx.request().uri().equals("/redirect")) {
152-
ctx.response().setStatusCode(301);
153-
ctx.response().headers().add("Location", "/");
154-
ctx.response().end();
155-
return;
156-
}
157171
ctx.response().setStatusCode(statusCode);
158-
159172
testContext.verify(() -> {
160173
assertThat(ctx.request().method(), is(equalTo(expectedMethod)));
161174
assertThat(ctx.request().query(), is(equalTo(expectedQuery)));

0 commit comments

Comments
 (0)