Skip to content

Commit ebf908d

Browse files
committed
8347373: HTTP/2 flow control checks may count unprocessed data twice
Backport-of: 06126361db1edb1d4c181a82952c1ac133a839f9
1 parent a6deb5b commit ebf908d

File tree

5 files changed

+112
-67
lines changed

5 files changed

+112
-67
lines changed

src/java.net.http/share/classes/jdk/internal/net/http/WindowUpdateSender.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -117,18 +117,24 @@ abstract class WindowUpdateSender {
117117
* the caller wants to buffer.
118118
*/
119119
boolean canBufferUnprocessedBytes(int len) {
120-
return !checkWindowSizeExceeded(unprocessed.addAndGet(len));
120+
long buffered, processed;
121+
// get received before unprocessed in order to avoid counting
122+
// unprocessed bytes that might get unbuffered asynchronously
123+
// twice.
124+
processed = received.get();
125+
buffered = unprocessed.addAndGet(len);
126+
return !checkWindowSizeExceeded(processed, buffered);
121127
}
122128

123129
// adds the provided amount to the amount of already
124-
// received and processed bytes and checks whether the
130+
// processed and processed bytes and checks whether the
125131
// flow control window is exceeded. If so, take
126132
// corrective actions and return true.
127-
private boolean checkWindowSizeExceeded(long len) {
133+
private boolean checkWindowSizeExceeded(long processed, long len) {
128134
// because windowSize is bound by Integer.MAX_VALUE
129135
// we will never reach the point where received.get() + len
130136
// could overflow
131-
long rcv = received.get() + len;
137+
long rcv = processed + len;
132138
return rcv > windowSize && windowSizeExceeded(rcv);
133139
}
134140

@@ -143,6 +149,7 @@ private boolean checkWindowSizeExceeded(long len) {
143149
* @param delta the amount of processed bytes to release
144150
*/
145151
void processed(int delta) {
152+
assert delta >= 0 : delta;
146153
long rest = unprocessed.addAndGet(-delta);
147154
assert rest >= 0;
148155
update(delta);
@@ -166,6 +173,7 @@ void processed(int delta) {
166173
* @return the amount of remaining unprocessed bytes
167174
*/
168175
long released(int delta) {
176+
assert delta >= 0 : delta;
169177
long rest = unprocessed.addAndGet(-delta);
170178
assert rest >= 0;
171179
return rest;
@@ -195,7 +203,7 @@ void update(int delta) {
195203
try {
196204
int tosend = (int)Math.min(received.get(), Integer.MAX_VALUE);
197205
if (tosend > limit) {
198-
received.getAndAdd(-tosend);
206+
received.addAndGet(-tosend);
199207
sendWindowUpdate(tosend);
200208
}
201209
} finally {

test/jdk/java/net/httpclient/http2/StreamFlowControlTest.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -34,6 +34,7 @@
3434

3535
import java.io.IOException;
3636
import java.io.InputStream;
37+
import java.io.InterruptedIOException;
3738
import java.io.OutputStream;
3839
import java.net.ProtocolException;
3940
import java.net.URI;
@@ -335,7 +336,9 @@ public void handle(Http2TestExchange t) throws IOException {
335336
// to wait for the connection window
336337
fct.conn.obtainConnectionWindow(resp.length);
337338
} catch (InterruptedException ie) {
338-
// ignore and continue...
339+
var ioe = new InterruptedIOException(ie.toString());
340+
ioe.initCause(ie);
341+
throw ioe;
339342
}
340343
}
341344
try {
@@ -344,6 +347,7 @@ public void handle(Http2TestExchange t) throws IOException {
344347
if (t instanceof FCHttp2TestExchange fct) {
345348
fct.conn.updateConnectionWindow(resp.length);
346349
}
350+
throw x;
347351
}
348352
}
349353
} finally {

test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/common/HttpServerAdapters.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -239,7 +239,7 @@ public static abstract class HttpTestExchange implements AutoCloseable {
239239
public abstract OutputStream getResponseBody();
240240
public abstract HttpTestRequestHeaders getRequestHeaders();
241241
public abstract HttpTestResponseHeaders getResponseHeaders();
242-
public abstract void sendResponseHeaders(int code, int contentLength) throws IOException;
242+
public abstract void sendResponseHeaders(int code, long contentLength) throws IOException;
243243
public abstract URI getRequestURI();
244244
public abstract String getRequestMethod();
245245
public abstract void close();
@@ -292,7 +292,7 @@ public HttpTestResponseHeaders getResponseHeaders() {
292292
return HttpTestResponseHeaders.of(exchange.getResponseHeaders());
293293
}
294294
@Override
295-
public void sendResponseHeaders(int code, int contentLength) throws IOException {
295+
public void sendResponseHeaders(int code, long contentLength) throws IOException {
296296
if (contentLength == 0) contentLength = -1;
297297
else if (contentLength < 0) contentLength = 0;
298298
exchange.sendResponseHeaders(code, contentLength);
@@ -355,7 +355,7 @@ public HttpTestResponseHeaders getResponseHeaders() {
355355
return HttpTestResponseHeaders.of(exchange.getResponseHeaders());
356356
}
357357
@Override
358-
public void sendResponseHeaders(int code, int contentLength) throws IOException {
358+
public void sendResponseHeaders(int code, long contentLength) throws IOException {
359359
if (contentLength == 0) contentLength = -1;
360360
else if (contentLength < 0) contentLength = 0;
361361
exchange.sendResponseHeaders(code, contentLength);

test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http2/BodyOutputStream.java

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -63,22 +63,28 @@ synchronized void updateWindow(int update) {
6363
}
6464

6565
void waitForWindow(int demand) throws InterruptedException {
66-
// first wait for the connection window
67-
conn.obtainConnectionWindow(demand);
68-
// now wait for the stream window
66+
// first wait for the stream window
6967
waitForStreamWindow(demand);
68+
// now wait for the connection window
69+
conn.obtainConnectionWindow(demand);
7070
}
7171

72-
public void waitForStreamWindow(int demand) throws InterruptedException {
73-
synchronized (this) {
74-
while (demand > 0) {
75-
int n = Math.min(demand, window);
76-
demand -= n;
77-
window -= n;
78-
if (demand > 0) {
79-
wait();
72+
public void waitForStreamWindow(int amount) throws InterruptedException {
73+
int demand = amount;
74+
try {
75+
synchronized (this) {
76+
while (amount > 0) {
77+
int n = Math.min(amount, window);
78+
amount -= n;
79+
window -= n;
80+
if (amount > 0) {
81+
wait();
82+
}
8083
}
8184
}
85+
} catch (Throwable t) {
86+
window += (demand - amount);
87+
throw t;
8288
}
8389
}
8490

0 commit comments

Comments
 (0)