mirror of
https://github.com/square/okhttp.git
synced 2026-01-25 16:01:38 +03:00
Merge pull request #410 from adriancole/spdy-prohibited-headers
portably handle prohibited headers across SPDY variants.
This commit is contained in:
@@ -31,6 +31,11 @@ import java.util.List;
|
||||
* http://tools.ietf.org/html/draft-ietf-httpbis-http2-09
|
||||
*/
|
||||
public final class Http20Draft09 implements Variant {
|
||||
|
||||
@Override public String getProtocol() {
|
||||
return "HTTP-draft-09/2.0";
|
||||
}
|
||||
|
||||
private static final byte[] CONNECTION_HEADER;
|
||||
static {
|
||||
try {
|
||||
@@ -158,7 +163,6 @@ public final class Http20Draft09 implements Variant {
|
||||
|
||||
if ((flags & FLAG_END_HEADERS) != 0) {
|
||||
hpackReader.emitReferenceSet();
|
||||
// not filtering out illegal headers on read.
|
||||
List<String> nameValueBlock = hpackReader.getAndReset();
|
||||
// TODO: Concat multi-value headers with 0x0, except COOKIE, which uses 0x3B, 0x20.
|
||||
// http://tools.ietf.org/html/draft-ietf-httpbis-http2-09#section-8.1.3
|
||||
@@ -322,20 +326,6 @@ public final class Http20Draft09 implements Variant {
|
||||
private void headers(boolean outFinished, int streamId, int priority,
|
||||
List<String> nameValueBlock) throws IOException {
|
||||
hpackBuffer.reset();
|
||||
for (int i = 0, size = nameValueBlock.size(); i < size; i += 2) {
|
||||
String name = nameValueBlock.get(i);
|
||||
// our SpdyTransport.writeNameValueBlock hard-codes :host
|
||||
// TODO: is :authority literally the same value as :host?
|
||||
// https://github.com/http2/http2-spec/issues/334
|
||||
if (":host".equals(name)) {
|
||||
nameValueBlock.set(i, ":authority");
|
||||
} else if (shouldDropHeader(name)) {
|
||||
//TODO: Avoid creating headers like these.
|
||||
nameValueBlock.remove(i);
|
||||
nameValueBlock.remove(i);
|
||||
i -= 2;
|
||||
}
|
||||
}
|
||||
hpackWriter.writeHeaders(nameValueBlock);
|
||||
int type = TYPE_HEADERS;
|
||||
// TODO: implement CONTINUATION
|
||||
@@ -423,19 +413,4 @@ public final class Http20Draft09 implements Variant {
|
||||
private static IOException ioException(String message, Object... args) throws IOException {
|
||||
throw new IOException(String.format(message, args));
|
||||
}
|
||||
|
||||
/**
|
||||
* Leniently drop as opposed to throwing malformed.
|
||||
* http://tools.ietf.org/html/draft-ietf-httpbis-http2-09#section-8.1.3
|
||||
*/
|
||||
private static boolean shouldDropHeader(String name) {
|
||||
return name.equals("connection")
|
||||
|| name.equals("host") // host is not supported in http/2
|
||||
|| name.equals("keep-alive")
|
||||
|| name.equals("proxy-connection")
|
||||
|| name.equals("te")
|
||||
|| name.equals("transfer-encoding")
|
||||
|| name.equals("encoding")
|
||||
|| name.equals("upgrade");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,6 +29,11 @@ import java.util.List;
|
||||
import java.util.zip.Deflater;
|
||||
|
||||
final class Spdy3 implements Variant {
|
||||
|
||||
@Override public String getProtocol() {
|
||||
return "spdy/3";
|
||||
}
|
||||
|
||||
static final int TYPE_DATA = 0x0;
|
||||
static final int TYPE_SYN_STREAM = 0x1;
|
||||
static final int TYPE_SYN_REPLY = 0x2;
|
||||
|
||||
@@ -103,6 +103,15 @@ public final class SpdyConnection implements Closeable {
|
||||
new Thread(new Reader(), "Spdy Reader " + hostName).start();
|
||||
}
|
||||
|
||||
/**
|
||||
* The protocol name, like {@code spdy/3} or {@code HTTP-draft-09/2.0}.
|
||||
*
|
||||
* @see com.squareup.okhttp.internal.spdy.Variant#getProtocol()
|
||||
*/
|
||||
public String getProtocol() {
|
||||
return variant.getProtocol();
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the number of {@link SpdyStream#isOpen() open streams} on this
|
||||
* connection.
|
||||
|
||||
@@ -23,6 +23,9 @@ interface Variant {
|
||||
Variant SPDY3 = new Spdy3();
|
||||
Variant HTTP_20_DRAFT_09 = new Http20Draft09();
|
||||
|
||||
/** The protocol name, like {@code spdy/3} or {@code HTTP-draft-09/2.0}. */
|
||||
String getProtocol();
|
||||
|
||||
/**
|
||||
* @param client true if this is the HTTP client's reader, reading frames from
|
||||
* a peer SPDY or HTTP/2 server.
|
||||
|
||||
@@ -57,7 +57,8 @@ public final class SpdyTransport implements Transport {
|
||||
boolean hasResponseBody = true;
|
||||
String version = RequestLine.version(httpEngine.connection.getHttpMinorVersion());
|
||||
stream = spdyConnection.newStream(
|
||||
writeNameValueBlock(request, version), hasRequestBody, hasResponseBody);
|
||||
writeNameValueBlock(request, spdyConnection.getProtocol(), version), hasRequestBody,
|
||||
hasResponseBody);
|
||||
stream.setReadTimeout(httpEngine.client.getReadTimeout());
|
||||
}
|
||||
|
||||
@@ -70,7 +71,7 @@ public final class SpdyTransport implements Transport {
|
||||
}
|
||||
|
||||
@Override public Response.Builder readResponseHeaders() throws IOException {
|
||||
return readNameValueBlock(stream.getResponseHeaders());
|
||||
return readNameValueBlock(stream.getResponseHeaders(), spdyConnection.getProtocol());
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -78,7 +79,7 @@ public final class SpdyTransport implements Transport {
|
||||
* Names are all lower case. No names are repeated. If any name has multiple
|
||||
* values, they are concatenated using "\0" as a delimiter.
|
||||
*/
|
||||
public static List<String> writeNameValueBlock(Request request, String version) {
|
||||
public static List<String> writeNameValueBlock(Request request, String protocol, String version) {
|
||||
Headers headers = request.headers();
|
||||
List<String> result = new ArrayList<String>(headers.size() + 10);
|
||||
result.add(":method");
|
||||
@@ -87,7 +88,13 @@ public final class SpdyTransport implements Transport {
|
||||
result.add(RequestLine.requestPath(request.url()));
|
||||
result.add(":version");
|
||||
result.add(version);
|
||||
result.add(":host");
|
||||
if (protocol.equals("spdy/3")) {
|
||||
result.add(":host");
|
||||
} else if (protocol.equals("HTTP-draft-09/2.0")) {
|
||||
result.add(":authority");
|
||||
} else {
|
||||
throw new AssertionError();
|
||||
}
|
||||
result.add(HttpEngine.hostHeader(request.url()));
|
||||
result.add(":scheme");
|
||||
result.add(request.url().getProtocol());
|
||||
@@ -98,19 +105,14 @@ public final class SpdyTransport implements Transport {
|
||||
String value = headers.value(i);
|
||||
|
||||
// Drop headers that are forbidden when layering HTTP over SPDY.
|
||||
if (name.equals("connection")
|
||||
|| name.equals("host")
|
||||
|| name.equals("keep-alive")
|
||||
|| name.equals("proxy-connection")
|
||||
|| name.equals("transfer-encoding")) {
|
||||
continue;
|
||||
}
|
||||
if (isProhibitedHeader(protocol, name)) continue;
|
||||
|
||||
// They shouldn't be set, but if they are, drop them. We've already written them!
|
||||
if (name.equals(":method")
|
||||
|| name.equals(":path")
|
||||
|| name.equals(":version")
|
||||
|| name.equals(":host")
|
||||
|| name.equals(":authority")
|
||||
|| name.equals(":scheme")) {
|
||||
continue;
|
||||
}
|
||||
@@ -134,7 +136,7 @@ public final class SpdyTransport implements Transport {
|
||||
}
|
||||
|
||||
/** Returns headers for a name value block containing a SPDY response. */
|
||||
public static Response.Builder readNameValueBlock(List<String> nameValueBlock)
|
||||
public static Response.Builder readNameValueBlock(List<String> nameValueBlock, String protocol)
|
||||
throws IOException {
|
||||
if (nameValueBlock.size() % 2 != 0) {
|
||||
throw new IllegalArgumentException("Unexpected name value block: " + nameValueBlock);
|
||||
@@ -143,7 +145,7 @@ public final class SpdyTransport implements Transport {
|
||||
String version = null;
|
||||
|
||||
Headers.Builder headersBuilder = new Headers.Builder();
|
||||
headersBuilder.set(OkHeaders.SELECTED_TRANSPORT, "spdy/3");
|
||||
headersBuilder.set(OkHeaders.SELECTED_TRANSPORT, protocol);
|
||||
for (int i = 0; i < nameValueBlock.size(); i += 2) {
|
||||
String name = nameValueBlock.get(i);
|
||||
String values = nameValueBlock.get(i + 1);
|
||||
@@ -157,7 +159,7 @@ public final class SpdyTransport implements Transport {
|
||||
status = value;
|
||||
} else if (":version".equals(name)) {
|
||||
version = value;
|
||||
} else {
|
||||
} else if (!isProhibitedHeader(protocol, name)) { // Don't write forbidden headers!
|
||||
headersBuilder.add(name, value);
|
||||
}
|
||||
start = end + 1;
|
||||
@@ -190,4 +192,34 @@ public final class SpdyTransport implements Transport {
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/** When true, this header should not be emitted or consumed. */
|
||||
private static boolean isProhibitedHeader(String protocol, String name) {
|
||||
boolean prohibited = false;
|
||||
if (protocol.equals("spdy/3")) {
|
||||
// http://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft3#TOC-3.2.1-Request
|
||||
if (name.equals("connection")
|
||||
|| name.equals("host")
|
||||
|| name.equals("keep-alive")
|
||||
|| name.equals("proxy-connection")
|
||||
|| name.equals("transfer-encoding")) {
|
||||
prohibited = true;
|
||||
}
|
||||
} else if (protocol.equals("HTTP-draft-09/2.0")) {
|
||||
// http://tools.ietf.org/html/draft-ietf-httpbis-http2-09#section-8.1.3
|
||||
if (name.equals("connection")
|
||||
|| name.equals("host")
|
||||
|| name.equals("keep-alive")
|
||||
|| name.equals("proxy-connection")
|
||||
|| name.equals("te")
|
||||
|| name.equals("transfer-encoding")
|
||||
|| name.equals("encoding")
|
||||
|| name.equals("upgrade")) {
|
||||
prohibited = true;
|
||||
}
|
||||
} else {
|
||||
throw new AssertionError();
|
||||
}
|
||||
return prohibited;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,7 +34,8 @@ public final class HeadersTest {
|
||||
":status", "200 OK",
|
||||
":version", "HTTP/1.1");
|
||||
Request request = new Request.Builder().url("http://square.com/").build();
|
||||
Response response = SpdyTransport.readNameValueBlock(nameValueBlock).request(request).build();
|
||||
Response response =
|
||||
SpdyTransport.readNameValueBlock(nameValueBlock, "spdy/3").request(request).build();
|
||||
Headers headers = response.headers();
|
||||
assertEquals(4, headers.size());
|
||||
assertEquals("HTTP/1.1 200 OK", response.statusLine());
|
||||
@@ -53,6 +54,34 @@ public final class HeadersTest {
|
||||
assertNull(headers.get(":version"));
|
||||
}
|
||||
|
||||
@Test public void readNameValueBlockDropsForbiddenHeadersSpdy3() throws IOException {
|
||||
List<String> nameValueBlock = Arrays.asList(
|
||||
":status", "200 OK",
|
||||
":version", "HTTP/1.1",
|
||||
"connection", "close");
|
||||
Request request = new Request.Builder().url("http://square.com/").build();
|
||||
Response response =
|
||||
SpdyTransport.readNameValueBlock(nameValueBlock, "spdy/3").request(request).build();
|
||||
Headers headers = response.headers();
|
||||
assertEquals(1, headers.size());
|
||||
assertEquals(OkHeaders.SELECTED_TRANSPORT, headers.name(0));
|
||||
assertEquals("spdy/3", headers.value(0));;
|
||||
}
|
||||
|
||||
@Test public void readNameValueBlockDropsForbiddenHeadersHttp2() throws IOException {
|
||||
List<String> nameValueBlock = Arrays.asList(
|
||||
":status", "200 OK",
|
||||
":version", "HTTP/1.1",
|
||||
"connection", "close");
|
||||
Request request = new Request.Builder().url("http://square.com/").build();
|
||||
Response response =
|
||||
SpdyTransport.readNameValueBlock(nameValueBlock, "HTTP-draft-09/2.0").request(request).build();
|
||||
Headers headers = response.headers();
|
||||
assertEquals(1, headers.size());
|
||||
assertEquals(OkHeaders.SELECTED_TRANSPORT, headers.name(0));
|
||||
assertEquals("HTTP-draft-09/2.0", headers.value(0));;
|
||||
}
|
||||
|
||||
@Test public void toNameValueBlock() {
|
||||
Request request = new Request.Builder()
|
||||
.url("http://square.com/")
|
||||
@@ -61,7 +90,7 @@ public final class HeadersTest {
|
||||
.addHeader("set-cookie", "Cookie2")
|
||||
.header(":status", "200 OK")
|
||||
.build();
|
||||
List<String> nameValueBlock = SpdyTransport.writeNameValueBlock(request, "HTTP/1.1");
|
||||
List<String> nameValueBlock = SpdyTransport.writeNameValueBlock(request, "spdy/3", "HTTP/1.1");
|
||||
List<String> expected = Arrays.asList(
|
||||
":method", "GET",
|
||||
":path", "/",
|
||||
@@ -74,7 +103,7 @@ public final class HeadersTest {
|
||||
assertEquals(expected, nameValueBlock);
|
||||
}
|
||||
|
||||
@Test public void toNameValueBlockDropsForbiddenHeaders() {
|
||||
@Test public void toNameValueBlockDropsForbiddenHeadersSpdy3() {
|
||||
Request request = new Request.Builder()
|
||||
.url("http://square.com/")
|
||||
.header("Connection", "close")
|
||||
@@ -86,6 +115,22 @@ public final class HeadersTest {
|
||||
":version", "HTTP/1.1",
|
||||
":host", "square.com",
|
||||
":scheme", "http");
|
||||
assertEquals(expected, SpdyTransport.writeNameValueBlock(request, "HTTP/1.1"));
|
||||
assertEquals(expected, SpdyTransport.writeNameValueBlock(request, "spdy/3", "HTTP/1.1"));
|
||||
}
|
||||
|
||||
@Test public void toNameValueBlockDropsForbiddenHeadersHttp2() {
|
||||
Request request = new Request.Builder()
|
||||
.url("http://square.com/")
|
||||
.header("Connection", "upgrade")
|
||||
.header("Upgrade", "websocket")
|
||||
.build();
|
||||
List<String> expected = Arrays.asList(
|
||||
":method", "GET",
|
||||
":path", "/",
|
||||
":version", "HTTP/1.1",
|
||||
":authority", "square.com",
|
||||
":scheme", "http");
|
||||
assertEquals(expected,
|
||||
SpdyTransport.writeNameValueBlock(request, "HTTP-draft-09/2.0", "HTTP/1.1"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user