Skip to content

Commit

Permalink
8291637: HttpClient default keep alive timeout not followed if server…
Browse files Browse the repository at this point in the history
… sends invalid value

Reviewed-by: jpai, dfuchs
  • Loading branch information
Michael-Mc-Mahon committed Aug 5, 2022
1 parent 0da4314 commit b17a745
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/java.base/share/classes/sun/net/www/http/HttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,15 @@ private boolean parseHTTPHeader(MessageHeader responses, ProgressSource pi, Http
responses.findValue("Keep-Alive"));
/* default should be larger in case of proxy */
keepAliveConnections = p.findInt("max", usingProxy?50:5);
if (keepAliveConnections < 0) {
keepAliveConnections = usingProxy?50:5;
}
keepAliveTimeout = p.findInt("timeout", -1);
if (keepAliveTimeout < -1) {
// if the server specified a negative (invalid) value
// then we set to -1, which is equivalent to no value
keepAliveTimeout = -1;
}
}
} else if (b[7] != '0') {
/*
Expand Down
139 changes: 139 additions & 0 deletions test/jdk/sun/net/www/http/KeepAliveCache/B8291637.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8291637
* @run main/othervm -Dhttp.keepAlive.time.server=20 -esa -ea B8291637 timeout
* @run main/othervm -Dhttp.keepAlive.time.server=20 -esa -ea B8291637 max
*/

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.*;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.CompletableFuture;

public class B8291637 {
static CompletableFuture<Boolean> passed = new CompletableFuture<>();

static class Server extends Thread {
final ServerSocket serverSocket;
final int port;
final String param; // the parameter to test "max" or "timeout"
volatile Socket s;

public Server(String param) throws IOException {
serverSocket = new ServerSocket(0);
port = serverSocket.getLocalPort();
setDaemon(true);
this.param = param;
}

public int getPort() {
return port;
}

public void close() {
try {
serverSocket.close();
if (s != null)
s.close();
} catch (IOException e) {}
}

static void readRequest(Socket s) throws IOException {
InputStream is = s.getInputStream();
is.read();
while (is.available() > 0)
is.read();
}

public void run() {
try {
while (true) {
s = serverSocket.accept();
readRequest(s);
OutputStream os = s.getOutputStream();
String resp = "" +
"HTTP/1.1 200 OK\r\n" +
"Content-Length: 11\r\n" +
"Connection: Keep-Alive\r\n" +
"Keep-Alive: " + param + "=-10\r\n" + // invalid negative value
"\r\n" +
"Hello World";
os.write(resp.getBytes(StandardCharsets.ISO_8859_1));
os.flush();
InputStream is = s.getInputStream();
long l1 = System.currentTimeMillis();
is.read();
long l2 = System.currentTimeMillis();
long diff = (l2 - l1) / 1000;
/*
* timeout is set to 20 seconds. If bug is still present
* then the timeout will occur the first time the keep alive
* thread wakes up which is after 5 seconds. This allows
* very large leeway with slow running hardware.
*
* Same behavior should occur in case of max=-1 with the bug
*/
if (diff < 19) {
passed.complete(false);
} else {
passed.complete(true);
}
System.out.println("Time diff = " + diff);
}
} catch (Throwable t) {
System.err.println("Server exception terminating: " + t);
passed.completeExceptionally(t);
}
}
}

public static void main(String[] args) throws Exception {
Server server = new Server(args[0]);
int port = server.getPort();
server.start();
URL url = new URL("http://127.0.0.1:" + Integer.toString(port) + "/");
HttpURLConnection urlc = (HttpURLConnection) url.openConnection();
InputStream i = urlc.getInputStream();
int c,count=0;
byte[] buf = new byte[256];
while ((c=i.read(buf)) != -1) {
count+=c;
}
i.close();
System.out.println("Read " + count );
try {
if (!passed.get()) {
throw new RuntimeException("Test failed");
} else {
System.out.println("Test passed");
}
} finally {
server.close();
}
}
}

5 comments on commit b17a745

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on b17a745 Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on b17a745 Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin the backport was successfully created on the branch GoeLin-backport-b17a745d in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit b17a745d from the openjdk/jdk repository.

The commit being backported was authored by Michael McMahon on 5 Aug 2022 and was reviewed by Jaikiran Pai and Daniel Fuchs.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev.git GoeLin-backport-b17a745d:GoeLin-backport-b17a745d
$ git checkout GoeLin-backport-b17a745d
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev.git GoeLin-backport-b17a745d

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on b17a745 Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk11u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on b17a745 Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin the backport was successfully created on the branch GoeLin-backport-b17a745d in my personal fork of openjdk/jdk11u-dev. To create a pull request with this backport targeting openjdk/jdk11u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit b17a745d from the openjdk/jdk repository.

The commit being backported was authored by Michael McMahon on 5 Aug 2022 and was reviewed by Jaikiran Pai and Daniel Fuchs.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk11u-dev:

$ git fetch https://github.com/openjdk-bots/jdk11u-dev.git GoeLin-backport-b17a745d:GoeLin-backport-b17a745d
$ git checkout GoeLin-backport-b17a745d
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk11u-dev.git GoeLin-backport-b17a745d

Please sign in to comment.