Skip to content

Commit

Permalink
8170305: URLConnection doesn't handle HTTP/1.1 1xx (informational) me…
Browse files Browse the repository at this point in the history
…ssages

Reviewed-by: dfuchs, michaelm
  • Loading branch information
jaikiran committed Sep 13, 2022
1 parent 9cd3e35 commit 8bd79d3
Show file tree
Hide file tree
Showing 2 changed files with 243 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/java.base/share/classes/sun/net/www/http/HttpClient.java
Expand Up @@ -968,7 +968,21 @@ private boolean parseHTTPHeader(MessageHeader responses, ProgressSource pi, Http
code = Integer.parseInt(resp, ind, ind + 3, 10);
} catch (Exception e) {}

if (code == HTTP_CONTINUE && ignoreContinue) {
if (code == 101) {
// We don't support protocol upgrade through the "Upgrade:" request header, so if a
// server still unexpectedly sends a 101 response, we consider that a protocol violation
// and close the connection.
closeServer();
logFinest("Closed connection due to unexpected 101 response");
// clear off the response headers so that they don't get propagated
// to the application
responses.reset();
throw new ProtocolException("Unexpected 101 response from server");
}
// ignore interim informational responses and continue to wait for final response.
if ((code == HTTP_CONTINUE && ignoreContinue)
|| (code >= 102 && code <= 199)) {
logFinest("Ignoring interim informational 1xx response: " + code);
responses.reset();
return parseHTTPHeader(responses, pi, httpuc);
}
Expand Down
228 changes: 228 additions & 0 deletions test/jdk/java/net/HttpURLConnection/Response1xxTest.java
@@ -0,0 +1,228 @@
/*
* 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.
*/

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.HttpURLConnection;
import java.net.InetAddress;
import java.net.ProtocolException;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.URI;
import java.nio.charset.StandardCharsets;

import jdk.test.lib.net.URIBuilder;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

/**
* @test
* @bug 8170305
* @summary Tests behaviour of HttpURLConnection when server responds with 1xx interim response status codes
* @library /test/lib
* @run testng Response1xxTest
*/
public class Response1xxTest {
private static final String EXPECTED_RSP_BODY = "Hello World";

private ServerSocket serverSocket;
private Http11Server server;
private String requestURIBase;


@BeforeClass
public void setup() throws Exception {
serverSocket = new ServerSocket(0, 0, InetAddress.getLoopbackAddress());
server = new Http11Server(serverSocket);
new Thread(server).start();
requestURIBase = URIBuilder.newBuilder().scheme("http").loopback()
.port(serverSocket.getLocalPort()).build().toString();
}

@AfterClass
public void teardown() throws Exception {
if (server != null) {
server.stop = true;
System.out.println("(HTTP 1.1) Server stop requested");
}
if (serverSocket != null) {
serverSocket.close();
System.out.println("Closed (HTTP 1.1) server socket");
}
}

private static final class Http11Server implements Runnable {
private static final int CONTENT_LENGTH = EXPECTED_RSP_BODY.getBytes(StandardCharsets.UTF_8).length;

private static final String HTTP_1_1_RSP_200 = "HTTP/1.1 200 OK\r\n" +
"Content-Length: " + CONTENT_LENGTH + "\r\n\r\n" +
EXPECTED_RSP_BODY;

private static final String REQ_LINE_FOO = "GET /test/foo HTTP/1.1\r\n";
private static final String REQ_LINE_BAR = "GET /test/bar HTTP/1.1\r\n";
private static final String REQ_LINE_HELLO = "GET /test/hello HTTP/1.1\r\n";
private static final String REQ_LINE_BYE = "GET /test/bye HTTP/1.1\r\n";


private final ServerSocket serverSocket;
private volatile boolean stop;

private Http11Server(final ServerSocket serverSocket) {
this.serverSocket = serverSocket;
}

@Override
public void run() {
System.out.println("Server running at " + serverSocket);
while (!stop) {
Socket socket = null;
try {
// accept a connection
socket = serverSocket.accept();
System.out.println("Accepted connection from client " + socket);
// read request
final String requestLine;
try {
requestLine = readRequestLine(socket);
} catch (Throwable t) {
// ignore connections from potential rogue client
System.err.println("Ignoring connection/request from client " + socket
+ " due to exception:");
t.printStackTrace();
// close the socket
safeClose(socket);
continue;
}
System.out.println("Received following request line from client " + socket
+ " :\n" + requestLine);
final int informationalResponseCode;
if (requestLine.startsWith(REQ_LINE_FOO)) {
// we will send intermediate/informational 102 response
informationalResponseCode = 102;
} else if (requestLine.startsWith(REQ_LINE_BAR)) {
// we will send intermediate/informational 103 response
informationalResponseCode = 103;
} else if (requestLine.startsWith(REQ_LINE_HELLO)) {
// we will send intermediate/informational 100 response
informationalResponseCode = 100;
} else if (requestLine.startsWith(REQ_LINE_BYE)) {
// we will send intermediate/informational 101 response
informationalResponseCode = 101;
} else {
// unexpected client. ignore and close the client
System.err.println("Ignoring unexpected request from client " + socket);
safeClose(socket);
continue;
}
try (final OutputStream os = socket.getOutputStream()) {
// send informational response headers a few times (spec allows them to
// be sent multiple times)
for (int i = 0; i < 3; i++) {
// send 1xx response header
os.write(("HTTP/1.1 " + informationalResponseCode + "\r\n\r\n")
.getBytes(StandardCharsets.UTF_8));
os.flush();
System.out.println("Sent response code " + informationalResponseCode
+ " to client " + socket);
}
// now send a final response
System.out.println("Now sending 200 response code to client " + socket);
os.write(HTTP_1_1_RSP_200.getBytes(StandardCharsets.UTF_8));
os.flush();
System.out.println("Sent 200 response code to client " + socket);
}
} catch (Throwable t) {
// close the client connection
safeClose(socket);
// continue accepting any other client connections until we are asked to stop
System.err.println("Ignoring exception in server:");
t.printStackTrace();
}
}
}

static String readRequestLine(final Socket sock) throws IOException {
final InputStream is = sock.getInputStream();
final StringBuilder sb = new StringBuilder("");
byte[] buf = new byte[1024];
while (!sb.toString().endsWith("\r\n\r\n")) {
final int numRead = is.read(buf);
if (numRead == -1) {
return sb.toString();
}
final String part = new String(buf, 0, numRead, StandardCharsets.ISO_8859_1);
sb.append(part);
}
return sb.toString();
}

private static void safeClose(final Socket socket) {
try {
socket.close();
} catch (Throwable t) {
// ignore
}
}
}

/**
* Tests that when a HTTP/1.1 server sends intermediate 1xx response codes and then the final
* response, the client (internally) will ignore those intermediate informational response codes
* and only return the final response to the application
*/
@Test
public void test1xx() throws Exception {
final URI[] requestURIs = new URI[]{
new URI(requestURIBase + "/test/foo"),
new URI(requestURIBase + "/test/bar"),
new URI(requestURIBase + "/test/hello")};
for (final URI requestURI : requestURIs) {
System.out.println("Issuing request to " + requestURI);
final HttpURLConnection urlConnection = (HttpURLConnection) requestURI.toURL().openConnection();
final int responseCode = urlConnection.getResponseCode();
Assert.assertEquals(responseCode, 200, "Unexpected response code");
final String body;
try (final InputStream is = urlConnection.getInputStream()) {
final byte[] bytes = is.readAllBytes();
body = new String(bytes, StandardCharsets.UTF_8);
}
Assert.assertEquals(body, EXPECTED_RSP_BODY, "Unexpected response body");
}
}

/**
* Tests that when a HTTP/1.1 server sends 101 response code, when the client
* didn't ask for a connection upgrade, then the request fails with an exception.
*/
@Test
public void test101CausesRequestFailure() throws Exception {
final URI requestURI = new URI(requestURIBase + "/test/bye");
System.out.println("Issuing request to " + requestURI);
final HttpURLConnection urlConnection = (HttpURLConnection) requestURI.toURL().openConnection();
// we expect the request to fail because the server unexpectedly sends a 101 response
Assert.assertThrows(ProtocolException.class, () -> urlConnection.getResponseCode());
}
}

3 comments on commit 8bd79d3

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@VladimirKempik
Copy link

Choose a reason for hiding this comment

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

/backport jdk19u

@openjdk
Copy link

@openjdk openjdk bot commented on 8bd79d3 Sep 18, 2022

Choose a reason for hiding this comment

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

@VladimirKempik the backport was successfully created on the branch VladimirKempik-backport-8bd79d3e in my personal fork of openjdk/jdk19u. To create a pull request with this backport targeting openjdk/jdk19u: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 8bd79d3e from the openjdk/jdk repository.

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

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/jdk19u:

$ git fetch https://github.com/openjdk-bots/jdk19u VladimirKempik-backport-8bd79d3e:VladimirKempik-backport-8bd79d3e
$ git checkout VladimirKempik-backport-8bd79d3e
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk19u VladimirKempik-backport-8bd79d3e

Please sign in to comment.