Skip to content
Permalink
Browse files
8274779: HttpURLConnection: HttpClient and HttpsClient incorrectly ch…
…eck request method when set to POST

Backport-of: 45ce06c9f3e9bee7d4bda313c38f0f0e8786a4db
  • Loading branch information
GoeLin committed Oct 28, 2021
1 parent 8a9cda2 commit 9cc661690ceb5822844dccf59412e566e3433a83
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 2 deletions.
@@ -303,7 +303,7 @@ public static HttpClient New(URL url, Proxy p, int to, boolean useCache,
ret = kac.get(url, null);
if (ret != null && httpuc != null &&
httpuc.streaming() &&
httpuc.getRequestMethod() == "POST") {
"POST".equals(httpuc.getRequestMethod())) {
if (!ret.available()) {
ret.inCache = false;
ret.closeServer();
@@ -331,7 +331,7 @@ static HttpClient New(SSLSocketFactory sf, URL url, HostnameVerifier hv,
ret = (HttpsClient) kac.get(url, sf);
if (ret != null && httpuc != null &&
httpuc.streaming() &&
httpuc.getRequestMethod() == "POST") {
"POST".equals(httpuc.getRequestMethod())) {
if (!ret.available())
ret = null;
}
@@ -0,0 +1,140 @@
/*
* Copyright (c) 2021, 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
* @summary This test checks that a broken HttpClient is not returned from the KeepAliveCache
* when the intial HttpURLConnection.setRequest method is passed a 'new String("POST")'
* rather than the "POST" String literal
* @bug 8274779
* @library /test/lib
* @modules java.base/sun.net.www.http
* java.base/sun.net.www.protocol.http
* @build java.base/sun.net.www.http.HttpClientAccess
* @run testng/othervm RequestMethodEquality
*/

import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpHandler;
import com.sun.net.httpserver.HttpServer;
import jdk.test.lib.net.URIBuilder;
import org.testng.Assert;
import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
import sun.net.www.http.HttpClient;
import sun.net.www.http.HttpClientAccess;
import sun.net.www.http.KeepAliveCache;
import sun.net.www.protocol.http.HttpURLConnection;

import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.net.URL;

public class RequestMethodEquality {
private static final String TEST_CONTEXT = "/reqmethodtest";
private HttpServer server;
private CustomHandler handler;
private HttpClientAccess httpClientAccess;

@BeforeTest
public void setup() throws Exception {
handler = new CustomHandler();
server = createServer(handler);
httpClientAccess = new HttpClientAccess();
}

@AfterTest
public void tearDown() throws Exception {
if (server != null) {
server.stop(0);
}
}

@Test
public void testHttpClient() throws Exception {
HttpURLConnection conn = null;
try {
URL url = URIBuilder.newBuilder()
.scheme("http")
.host(server.getAddress().getAddress())
.port(server.getAddress().getPort())
.path(TEST_CONTEXT)
.toURL();

conn = (HttpURLConnection) url.openConnection();
conn.setChunkedStreamingMode(8); // ensures the call to HttpURLConnection.streaming() passes

int firstConnectTimeout = 1234;
HttpClient freshClient = HttpClient.New(url, Proxy.NO_PROXY, firstConnectTimeout, true, conn);
freshClient.closeServer(); // ensures that the call to HttpClient.available() fails

httpClientAccess.setInCache(freshClient, true); // allows the assertion in HttpClient.New to pass

// Injecting a mock KeepAliveCache that the HttpClient can use
KeepAliveCache kac = httpClientAccess.getKeepAliveCache();
kac.put(url, null, freshClient);

// The 'new' keyword is important here as the original code
// used '==' rather than String.equals to compare request methods
conn.setRequestMethod(new String("POST"));

// Before the fix, the value returned to 'cachedClient' would have been the (broken) cached
// 'freshClient' as HttpClient.available() could never be checked
int secondConnectTimeout = 4321;
HttpClient cachedClient = HttpClient.New(url, Proxy.NO_PROXY, secondConnectTimeout, true, conn);
cachedClient.closeServer();

int originalConnectTimeout = freshClient.getConnectTimeout();
int cachedConnectTimeout = cachedClient.getConnectTimeout();

// If both connectTimeout values are equal, it means the test retrieved the same broken
// HttpClient from the cache and is trying to re-use it.
Assert.assertNotEquals(originalConnectTimeout, cachedConnectTimeout, "Both connectTimeout values are equal.\nThis means the test is reusing a broken HttpClient rather than creating a new one.");
} finally {
if (conn != null) {
conn.disconnect();
}
}
}

private static HttpServer createServer(final HttpHandler handler) throws IOException {
final InetSocketAddress serverAddress = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
final int backlog = -1;
final HttpServer server = HttpServer.create(serverAddress, backlog);
server.createContext(TEST_CONTEXT, handler);
server.start();
System.out.println("Server started on " + server.getAddress());
return server;
}

private static class CustomHandler implements HttpHandler {
@Override
public void handle(HttpExchange exchange) throws IOException {
// We'll always send 200 OK - We don't care about the server logic
exchange.sendResponseHeaders(200, 1);
exchange.close();
}
}
}
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2021, 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.
*/

package sun.net.www.http;
// We can use this injected accessor class to get the KeepAliveCache from our HttpClient
public class HttpClientAccess {
public KeepAliveCache getKeepAliveCache () {
// kac is a protected static field in HttpClient
return HttpClient.kac;
}

public void setInCache(HttpClient client, boolean inCache) {
client.inCache = inCache;
}
}

1 comment on commit 9cc6616

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on 9cc6616 Oct 28, 2021

Choose a reason for hiding this comment

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

Please sign in to comment.