Skip to content

Commit 966cafc

Browse files
RealCLangerRealCLanger
committedAug 7, 2024
7026262: HttpServer: improve handling of finished HTTP exchanges
Backport-of: a5ffa079a0d6107be652bc026f5c91b7dcd791f8
·
jdk-17.0.16+0jdk-17.0.13-ga
1 parent 396e209 commit 966cafc

File tree

9 files changed

+306
-37
lines changed

9 files changed

+306
-37
lines changed
 

‎src/jdk.httpserver/share/classes/sun/net/httpserver/ChunkedOutputStream.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2008, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2023, 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
@@ -27,6 +27,8 @@
2727

2828
import java.io.*;
2929
import java.net.*;
30+
import java.util.Objects;
31+
3032
import com.sun.net.httpserver.*;
3133
import com.sun.net.httpserver.spi.*;
3234

@@ -77,6 +79,10 @@ public void write (int b) throws IOException {
7779
}
7880

7981
public void write (byte[]b, int off, int len) throws IOException {
82+
Objects.checkFromIndexSize(off, len, b.length);
83+
if (len == 0) {
84+
return;
85+
}
8086
if (closed) {
8187
throw new StreamClosedException ();
8288
}

‎src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2023, 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
@@ -282,9 +282,7 @@ public void sendResponseHeaders (int rCode, long contentLen)
282282
sentHeaders = true;
283283
logger.log(Level.TRACE, "Sent headers: noContentToSend=" + noContentToSend);
284284
if (noContentToSend) {
285-
WriteFinishedEvent e = new WriteFinishedEvent (this);
286-
server.addEvent (e);
287-
closed = true;
285+
close();
288286
}
289287
server.logReply (rCode, req.requestLine(), null);
290288
}

‎src/jdk.httpserver/share/classes/sun/net/httpserver/FixedLengthOutputStream.java

+8-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2006, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2023, 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
@@ -27,6 +27,8 @@
2727

2828
import java.io.*;
2929
import java.net.*;
30+
import java.util.Objects;
31+
3032
import com.sun.net.httpserver.*;
3133
import com.sun.net.httpserver.spi.*;
3234

@@ -41,7 +43,6 @@
4143
class FixedLengthOutputStream extends FilterOutputStream
4244
{
4345
private long remaining;
44-
private boolean eof = false;
4546
private boolean closed = false;
4647
ExchangeImpl t;
4748

@@ -58,22 +59,21 @@ public void write (int b) throws IOException {
5859
if (closed) {
5960
throw new IOException ("stream closed");
6061
}
61-
eof = (remaining == 0);
62-
if (eof) {
62+
if (remaining == 0) {
6363
throw new StreamClosedException();
6464
}
6565
out.write(b);
6666
remaining --;
6767
}
6868

6969
public void write (byte[]b, int off, int len) throws IOException {
70+
Objects.checkFromIndexSize(off, len, b.length);
71+
if (len == 0) {
72+
return;
73+
}
7074
if (closed) {
7175
throw new IOException ("stream closed");
7276
}
73-
eof = (remaining == 0);
74-
if (eof) {
75-
throw new StreamClosedException();
76-
}
7777
if (len > remaining) {
7878
// stream is still open, caller can retry
7979
throw new IOException ("too many bytes to write to stream");
@@ -92,7 +92,6 @@ public void close () throws IOException {
9292
throw new IOException ("insufficient bytes written to stream");
9393
}
9494
flush();
95-
eof = true;
9695
LeftOverInputStream is = t.getOriginalInputStream();
9796
if (!is.isClosed()) {
9897
try {

‎src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java

+21-18
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,14 @@ public void run () {
696696
return;
697697
}
698698
String uriStr = requestLine.substring (start, space);
699-
URI uri = new URI (uriStr);
699+
URI uri;
700+
try {
701+
uri = new URI (uriStr);
702+
} catch (URISyntaxException e3) {
703+
reject(Code.HTTP_BAD_REQUEST,
704+
requestLine, "URISyntaxException thrown");
705+
return;
706+
}
700707
start = space+1;
701708
String version = requestLine.substring (start);
702709
Headers headers = req.headers();
@@ -732,7 +739,13 @@ public void run () {
732739
} else {
733740
headerValue = headers.getFirst("Content-Length");
734741
if (headerValue != null) {
735-
clen = Long.parseLong(headerValue);
742+
try {
743+
clen = Long.parseLong(headerValue);
744+
} catch (NumberFormatException e2) {
745+
reject(Code.HTTP_BAD_REQUEST,
746+
requestLine, "NumberFormatException thrown");
747+
return;
748+
}
736749
if (clen < 0) {
737750
reject(Code.HTTP_BAD_REQUEST, requestLine,
738751
"Illegal Content-Length value");
@@ -818,20 +831,11 @@ public void run () {
818831
uc.doFilter (new HttpExchangeImpl (tx));
819832
}
820833

821-
} catch (IOException e1) {
822-
logger.log (Level.TRACE, "ServerImpl.Exchange (1)", e1);
823-
closeConnection(connection);
824-
} catch (NumberFormatException e2) {
825-
logger.log (Level.TRACE, "ServerImpl.Exchange (2)", e2);
826-
reject (Code.HTTP_BAD_REQUEST,
827-
requestLine, "NumberFormatException thrown");
828-
} catch (URISyntaxException e3) {
829-
logger.log (Level.TRACE, "ServerImpl.Exchange (3)", e3);
830-
reject (Code.HTTP_BAD_REQUEST,
831-
requestLine, "URISyntaxException thrown");
832-
} catch (Exception e4) {
833-
logger.log (Level.TRACE, "ServerImpl.Exchange (4)", e4);
834-
closeConnection(connection);
834+
} catch (Exception e) {
835+
logger.log (Level.TRACE, "ServerImpl.Exchange", e);
836+
if (tx == null || !tx.writefinished) {
837+
closeConnection(connection);
838+
}
835839
} catch (Throwable t) {
836840
logger.log(Level.TRACE, "ServerImpl.Exchange (5)", t);
837841
throw t;
@@ -856,9 +860,8 @@ void reject (int code, String requestStr, String message) {
856860
rejected = true;
857861
logReply (code, requestStr, message);
858862
sendReply (
859-
code, false, "<h1>"+code+Code.msg(code)+"</h1>"+message
863+
code, true, "<h1>"+code+Code.msg(code)+"</h1>"+message
860864
);
861-
closeConnection(connection);
862865
}
863866

864867
void sendReply (

‎src/jdk.httpserver/share/classes/sun/net/httpserver/UndefLengthOutputStream.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2007, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2007, 2023, 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
@@ -27,6 +27,8 @@
2727

2828
import java.io.*;
2929
import java.net.*;
30+
import java.util.Objects;
31+
3032
import com.sun.net.httpserver.*;
3133
import com.sun.net.httpserver.spi.*;
3234

@@ -55,6 +57,10 @@ public void write (int b) throws IOException {
5557
}
5658

5759
public void write (byte[]b, int off, int len) throws IOException {
60+
Objects.checkFromIndexSize(off, len, b.length);
61+
if (len == 0) {
62+
return;
63+
}
5864
if (closed) {
5965
throw new IOException ("stream closed");
6066
}

‎test/jdk/com/sun/net/httpserver/bugs/ExceptionKeepAlive.java

+131
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/**
25+
* @test
26+
* @bug 8219083
27+
* @summary Exceptions thrown from HttpHandler.handle should not close connection
28+
* if response is completed
29+
* @library /test/lib
30+
* @run junit ExceptionKeepAlive
31+
*/
32+
33+
import com.sun.net.httpserver.HttpExchange;
34+
import com.sun.net.httpserver.HttpHandler;
35+
import com.sun.net.httpserver.HttpServer;
36+
import jdk.test.lib.net.URIBuilder;
37+
import org.junit.jupiter.api.Test;
38+
39+
import java.io.IOException;
40+
import java.net.HttpURLConnection;
41+
import java.net.InetAddress;
42+
import java.net.InetSocketAddress;
43+
import java.net.Proxy;
44+
import java.net.URL;
45+
import java.util.logging.Handler;
46+
import java.util.logging.Level;
47+
import java.util.logging.Logger;
48+
import java.util.logging.SimpleFormatter;
49+
import java.util.logging.StreamHandler;
50+
51+
import static org.junit.jupiter.api.Assertions.*;
52+
53+
public class ExceptionKeepAlive
54+
{
55+
56+
public static final Logger LOGGER = Logger.getLogger("com.sun.net.httpserver");
57+
58+
@Test
59+
void test() throws IOException, InterruptedException {
60+
HttpServer httpServer = startHttpServer();
61+
int port = httpServer.getAddress().getPort();
62+
try {
63+
URL url = URIBuilder.newBuilder()
64+
.scheme("http")
65+
.loopback()
66+
.port(port)
67+
.path("/firstCall")
68+
.toURLUnchecked();
69+
HttpURLConnection uc = (HttpURLConnection)url.openConnection(Proxy.NO_PROXY);
70+
int responseCode = uc.getResponseCode();
71+
assertEquals(200, responseCode, "First request should succeed");
72+
73+
URL url2 = URIBuilder.newBuilder()
74+
.scheme("http")
75+
.loopback()
76+
.port(port)
77+
.path("/secondCall")
78+
.toURLUnchecked();
79+
HttpURLConnection uc2 = (HttpURLConnection)url2.openConnection(Proxy.NO_PROXY);
80+
responseCode = uc2.getResponseCode();
81+
assertEquals(200, responseCode, "Second request should reuse connection");
82+
} finally {
83+
httpServer.stop(0);
84+
}
85+
}
86+
87+
/**
88+
* Http Server
89+
*/
90+
HttpServer startHttpServer() throws IOException {
91+
Handler outHandler = new StreamHandler(System.out,
92+
new SimpleFormatter());
93+
outHandler.setLevel(Level.FINEST);
94+
LOGGER.setLevel(Level.FINEST);
95+
LOGGER.addHandler(outHandler);
96+
InetAddress loopback = InetAddress.getLoopbackAddress();
97+
HttpServer httpServer = HttpServer.create(new InetSocketAddress(loopback, 0), 0);
98+
httpServer.createContext("/", new MyHandler());
99+
httpServer.start();
100+
return httpServer;
101+
}
102+
103+
class MyHandler implements HttpHandler {
104+
105+
volatile int port1;
106+
@Override
107+
public void handle(HttpExchange t) throws IOException {
108+
String path = t.getRequestURI().getPath();
109+
if (path.equals("/firstCall")) {
110+
port1 = t.getRemoteAddress().getPort();
111+
System.out.println("First connection on client port = " + port1);
112+
113+
// send response
114+
t.sendResponseHeaders(200, -1);
115+
// response is completed now; throw exception
116+
throw new NumberFormatException();
117+
// the connection should still be reusable
118+
} else if (path.equals("/secondCall")) {
119+
int port2 = t.getRemoteAddress().getPort();
120+
System.out.println("Second connection on client port = " + port2);
121+
122+
if (port1 == port2) {
123+
t.sendResponseHeaders(200, -1);
124+
} else {
125+
t.sendResponseHeaders(500, -1);
126+
}
127+
}
128+
t.close();
129+
}
130+
}
131+
}

‎test/jdk/com/sun/net/httpserver/bugs/ZeroLengthOutputStream.java

+114
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*
2+
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/**
25+
* @test
26+
* @bug 8219083
27+
* @summary HttpExchange.getResponseBody write and close should not throw
28+
* even when response length is zero
29+
* @library /test/lib
30+
* @run junit ZeroLengthOutputStream
31+
*/
32+
33+
import com.sun.net.httpserver.HttpExchange;
34+
import com.sun.net.httpserver.HttpHandler;
35+
import com.sun.net.httpserver.HttpServer;
36+
import jdk.test.lib.net.URIBuilder;
37+
import org.junit.jupiter.api.Test;
38+
39+
import java.io.IOException;
40+
import java.io.OutputStream;
41+
import java.net.HttpURLConnection;
42+
import java.net.InetAddress;
43+
import java.net.InetSocketAddress;
44+
import java.net.Proxy;
45+
import java.net.URL;
46+
import java.util.concurrent.CountDownLatch;
47+
import java.util.logging.Handler;
48+
import java.util.logging.Level;
49+
import java.util.logging.Logger;
50+
import java.util.logging.SimpleFormatter;
51+
import java.util.logging.StreamHandler;
52+
53+
import static org.junit.jupiter.api.Assertions.*;
54+
55+
public class ZeroLengthOutputStream
56+
{
57+
58+
public static final Logger LOGGER = Logger.getLogger("com.sun.net.httpserver");
59+
public volatile boolean closed;
60+
public CountDownLatch cdl = new CountDownLatch(1);
61+
62+
@Test
63+
void test() throws IOException, InterruptedException {
64+
HttpServer httpServer = startHttpServer();
65+
int port = httpServer.getAddress().getPort();
66+
try {
67+
URL url = URIBuilder.newBuilder()
68+
.scheme("http")
69+
.loopback()
70+
.port(port)
71+
.path("/flis/")
72+
.toURLUnchecked();
73+
HttpURLConnection uc = (HttpURLConnection)url.openConnection(Proxy.NO_PROXY);
74+
uc.getResponseCode();
75+
cdl.await();
76+
assertTrue(closed, "OutputStream close did not complete");
77+
} finally {
78+
httpServer.stop(0);
79+
}
80+
}
81+
82+
/**
83+
* Http Server
84+
*/
85+
HttpServer startHttpServer() throws IOException {
86+
Handler outHandler = new StreamHandler(System.out,
87+
new SimpleFormatter());
88+
outHandler.setLevel(Level.FINEST);
89+
LOGGER.setLevel(Level.FINEST);
90+
LOGGER.addHandler(outHandler);
91+
InetAddress loopback = InetAddress.getLoopbackAddress();
92+
HttpServer httpServer = HttpServer.create(new InetSocketAddress(loopback, 0), 0);
93+
httpServer.createContext("/flis/", new MyHandler());
94+
httpServer.start();
95+
return httpServer;
96+
}
97+
98+
class MyHandler implements HttpHandler {
99+
100+
@Override
101+
public void handle(HttpExchange t) throws IOException {
102+
try {
103+
OutputStream os = t.getResponseBody();
104+
t.sendResponseHeaders(200, -1);
105+
os.write(new byte[0]);
106+
os.close();
107+
System.out.println("Output stream closed");
108+
closed = true;
109+
} finally {
110+
cdl.countDown();
111+
}
112+
}
113+
}
114+
}

‎test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2023, 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
@@ -170,7 +170,7 @@ public void handle(HttpExchange trans) {
170170
try {
171171
String path = trans.getRequestURI().getPath();
172172
if (path.equals("/firstCall")) {
173-
port1 = trans.getLocalAddress().getPort();
173+
port1 = trans.getRemoteAddress().getPort();
174174
System.out.println("First connection on client port = " + port1);
175175

176176
byte[] responseBody = new byte[RESPONSE_DATA_LENGTH];
@@ -181,7 +181,7 @@ public void handle(HttpExchange trans) {
181181
pw.print(responseBody);
182182
}
183183
} else if (path.equals("/secondCall")) {
184-
int port2 = trans.getLocalAddress().getPort();
184+
int port2 = trans.getRemoteAddress().getPort();
185185
System.out.println("Second connection on client port = " + port2);
186186

187187
if (port1 != port2)

‎test/jdk/sun/net/www/http/KeepAliveCache/B8293562.java

+13-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2022, 2023, 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
@@ -29,6 +29,8 @@
2929
* @summary Http keep-alive thread should close sockets without holding a lock
3030
*/
3131

32+
import com.sun.net.httpserver.HttpExchange;
33+
import com.sun.net.httpserver.HttpHandler;
3234
import com.sun.net.httpserver.HttpServer;
3335

3436
import javax.net.ssl.HandshakeCompletedListener;
@@ -45,6 +47,7 @@
4547
import java.net.Socket;
4648
import java.net.URL;
4749
import java.net.UnknownHostException;
50+
import java.nio.charset.StandardCharsets;
4851
import java.util.concurrent.CompletableFuture;
4952
import java.util.concurrent.CountDownLatch;
5053
import java.util.concurrent.Executors;
@@ -64,6 +67,7 @@ public static void main(String[] args) throws Exception {
6467
public static void startHttpServer() throws Exception {
6568
server = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 10);
6669
server.setExecutor(Executors.newCachedThreadPool());
70+
server.createContext("/", new NotFoundHandler());
6771
server.start();
6872
}
6973

@@ -234,5 +238,13 @@ public Socket createSocket(Socket s, String host, int port, boolean autoClose) t
234238
throw new UnsupportedOperationException();
235239
}
236240
}
241+
242+
static class NotFoundHandler implements HttpHandler {
243+
@Override
244+
public void handle(HttpExchange t) throws IOException {
245+
t.sendResponseHeaders(404, 3);
246+
t.getResponseBody().write("abc".getBytes(StandardCharsets.UTF_8));
247+
}
248+
}
237249
}
238250

1 commit comments

Comments
 (1)

openjdk-notifier[bot] commented on Aug 7, 2024

@openjdk-notifier[bot]
openjdk-notifier[bot]
on Aug 7, 2024

Review

Issues

Please sign in to comment.