Skip to content

Commit

Permalink
8291638: Keep-Alive timeout of 0 should close connection immediately
Browse files Browse the repository at this point in the history
Reviewed-by: dfuchs, jpai
  • Loading branch information
Michael-Mc-Mahon committed Oct 13, 2022
1 parent 6ae7e4d commit 26ac836
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 11 deletions.
16 changes: 15 additions & 1 deletion src/java.base/share/classes/sun/net/www/HeaderParser.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1996, 2011, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1996, 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
Expand All @@ -26,6 +26,7 @@
package sun.net.www;

import java.util.Iterator;
import java.util.OptionalInt;

/* This is useful for the nightmare of parsing multi-part HTTP/RFC822 headers
* sensibly:
Expand Down Expand Up @@ -246,6 +247,19 @@ public int findInt(String k, int Default) {
return Default;
}
}

public OptionalInt findInt(String k) {
try {
String s = findValue(k);
if (s == null) {
return OptionalInt.empty();
}
return OptionalInt.of(Integer.parseInt(s));
} catch (Throwable t) {
return OptionalInt.empty();
}
}

/*
public static void main(String[] a) throws Exception {
System.out.print("enter line to parse> ");
Expand Down
18 changes: 14 additions & 4 deletions src/java.base/share/classes/sun/net/www/http/HttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.net.*;
import java.util.Locale;
import java.util.Objects;
import java.util.OptionalInt;
import java.util.Properties;
import java.util.concurrent.locks.ReentrantLock;

Expand Down Expand Up @@ -127,6 +128,7 @@ private static int getDefaultPort(String proto) {
* 0: the server specified no keep alive headers
* -1: the server provided "Connection: keep-alive" but did not specify a
* a particular time in a "Keep-Alive:" headers
* -2: the server provided "Connection: keep-alive" and timeout=0
* Positive values are the number of seconds specified by the server
* in a "Keep-Alive" header
*/
Expand Down Expand Up @@ -903,11 +905,19 @@ private boolean parseHTTPHeader(MessageHeader responses, ProgressSource pi, Http
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
OptionalInt timeout = p.findInt("timeout");
if (timeout.isEmpty()) {
keepAliveTimeout = -1;
} else {
keepAliveTimeout = timeout.getAsInt();
if (keepAliveTimeout < 0) {
// if the server specified a negative (invalid) value
// then we set to -1, which is equivalent to no value
keepAliveTimeout = -1;
} else if (keepAliveTimeout == 0) {
// handled specially to mean close connection immediately
keepAliveTimeout = -2;
}
}
}
} else if (b[7] != '0') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ public Void run() {
// different default for server and proxy
keepAliveTimeout = http.getUsingProxy() ? 60 : 5;
}
} else if (keepAliveTimeout == -2) {
keepAliveTimeout = 0;
}
// at this point keepAliveTimeout is the number of seconds to keep
// alive, which could be 0, if the user specified 0 for the property
Expand Down
7 changes: 1 addition & 6 deletions test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
/*
* @test
* @library /test/lib
* @bug 8291226 8291638
* @modules java.base/sun.net:+open
* java.base/sun.net.www.http:+open
* java.base/sun.net.www:+open
Expand Down Expand Up @@ -1015,12 +1016,6 @@ else if (scenarioNumber >= 144 && scenarioNumber <= 159){
}

private void startScenario(int scenarioNumber) throws Exception {
//test scenarios are skipped because of JDK-8291638
if((scenarioNumber >= 112 && scenarioNumber <= 127) || (scenarioNumber >= 144 && scenarioNumber <= 159)) {
System.out.println("Scenario Skipped:"+scenarioNumber);
this.countDownLatch.countDown();
return;
}
System.out.println("serverScenarios[" + scenarioNumber + "]=" + getServerScenario(scenarioNumber));
System.out.println("clientScenarios[" + scenarioNumber + "]=" + clientScenarios[getClientScenarioNumber(scenarioNumber)]);
if(expectedValues[scenarioNumber] == 0) {
Expand Down

5 comments on commit 26ac836

@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 26ac836 Mar 29, 2023

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on 26ac836 Mar 29, 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 26ac836 Mar 29, 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-26ac8366 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 26ac8366 from the openjdk/jdk repository.

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

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-26ac8366:GoeLin-backport-26ac8366
$ git checkout GoeLin-backport-26ac8366
# 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-26ac8366

@openjdk
Copy link

@openjdk openjdk bot commented on 26ac836 Mar 29, 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-26ac8366 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 26ac8366 from the openjdk/jdk repository.

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

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-26ac8366:GoeLin-backport-26ac8366
$ git checkout GoeLin-backport-26ac8366
# 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-26ac8366

Please sign in to comment.