Skip to content

Commit 6073c21

Browse files
arturobernalgfitekonea
arturobernalg
authored and
fitekonea
committed
Validate Connection: Upgrade on protocol switch responses
ProtocolSwitchStrategy now rejects 101 Switching Protocols responses that advertise Upgrade without the required Connection: Upgrade token.
1 parent e005c48 commit 6073c21

2 files changed

Lines changed: 110 additions & 0 deletions

File tree

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/ProtocolSwitchStrategy.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ public InternalProtocolException(final ProtocolException cause) {
7070
}
7171

7272
public ProtocolVersion switchProtocol(final HttpMessage response) throws ProtocolException {
73+
if (!containsConnectionUpgrade(response)) {
74+
throw new ProtocolException("Invalid protocol switch response: missing Connection: Upgrade");
75+
}
76+
7377
final AtomicReference<ProtocolVersion> tlsUpgrade = new AtomicReference<>();
7478

7579
parseHeaders(response, HttpHeaders.UPGRADE, (buffer, cursor) -> {
@@ -91,6 +95,49 @@ public ProtocolVersion switchProtocol(final HttpMessage response) throws Protoco
9195
}
9296
}
9397

98+
private boolean containsConnectionUpgrade(final HttpMessage message) throws ProtocolException {
99+
final Iterator<Header> it = message.headerIterator(HttpHeaders.CONNECTION);
100+
while (it.hasNext()) {
101+
final Header header = it.next();
102+
if (header instanceof FormattedHeader) {
103+
final CharArrayBuffer buf = ((FormattedHeader) header).getBuffer();
104+
final ParserCursor cursor = new ParserCursor(0, buf.length());
105+
cursor.updatePos(((FormattedHeader) header).getValuePos());
106+
if (containsUpgradeToken(buf, cursor)) {
107+
return true;
108+
}
109+
} else {
110+
final String value = header.getValue();
111+
if (value == null) {
112+
continue;
113+
}
114+
final ParserCursor cursor = new ParserCursor(0, value.length());
115+
if (containsUpgradeToken(value, cursor)) {
116+
return true;
117+
}
118+
}
119+
}
120+
return false;
121+
}
122+
123+
private boolean containsUpgradeToken(final CharSequence buffer, final ParserCursor cursor) throws ProtocolException {
124+
while (!cursor.atEnd()) {
125+
TOKENIZER.skipWhiteSpace(buffer, cursor);
126+
final String token = TOKENIZER.parseToken(buffer, cursor, UPGRADE_TOKEN_DELIMITER);
127+
if (token.equalsIgnoreCase("upgrade")) {
128+
return true;
129+
}
130+
TOKENIZER.skipWhiteSpace(buffer, cursor);
131+
if (!cursor.atEnd()) {
132+
final char ch = buffer.charAt(cursor.getPos());
133+
if (ch == ',') {
134+
cursor.updatePos(cursor.getPos() + 1);
135+
}
136+
}
137+
}
138+
return false;
139+
}
140+
94141
private ProtocolVersion parseProtocolVersion(final CharSequence buffer, final ParserCursor cursor) throws ProtocolException {
95142
TOKENIZER.skipWhiteSpace(buffer, cursor);
96143
final String proto = TOKENIZER.parseToken(buffer, cursor, LAX_PROTO_DELIMITER);

‎httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestProtocolSwitchStrategy.java‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,30 +52,36 @@ void setUp() {
5252
@Test
5353
void testSwitchToTLS() throws Exception {
5454
final HttpResponse response1 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
55+
response1.addHeader(HttpHeaders.CONNECTION, "Upgrade");
5556
response1.addHeader(HttpHeaders.UPGRADE, "TLS");
5657
Assertions.assertEquals(TLS.V_1_2.getVersion(), switchStrategy.switchProtocol(response1));
5758

5859
final HttpResponse response2 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
60+
response2.addHeader(HttpHeaders.CONNECTION, "Upgrade");
5961
response2.addHeader(HttpHeaders.UPGRADE, "TLS/1.3");
6062
Assertions.assertEquals(TLS.V_1_3.getVersion(), switchStrategy.switchProtocol(response2));
6163
}
6264

6365
@Test
6466
void testSwitchToHTTP11AndTLS() throws Exception {
6567
final HttpResponse response1 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
68+
response1.addHeader(HttpHeaders.CONNECTION, "Upgrade");
6669
response1.addHeader(HttpHeaders.UPGRADE, "TLS, HTTP/1.1");
6770
Assertions.assertEquals(TLS.V_1_2.getVersion(), switchStrategy.switchProtocol(response1));
6871

6972
final HttpResponse response2 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
73+
response2.addHeader(HttpHeaders.CONNECTION, "Upgrade");
7074
response2.addHeader(HttpHeaders.UPGRADE, ",, HTTP/1.1, TLS, ");
7175
Assertions.assertEquals(TLS.V_1_2.getVersion(), switchStrategy.switchProtocol(response2));
7276

7377
final HttpResponse response3 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
78+
response3.addHeader(HttpHeaders.CONNECTION, "Upgrade");
7479
response3.addHeader(HttpHeaders.UPGRADE, "HTTP/1.1");
7580
response3.addHeader(HttpHeaders.UPGRADE, "TLS/1.2");
7681
Assertions.assertEquals(TLS.V_1_2.getVersion(), switchStrategy.switchProtocol(response3));
7782

7883
final HttpResponse response4 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
84+
response4.addHeader(HttpHeaders.CONNECTION, "Upgrade");
7985
response4.addHeader(HttpHeaders.UPGRADE, "HTTP/1.1");
8086
response4.addHeader(HttpHeaders.UPGRADE, "TLS/1.2, TLS/1.3");
8187
Assertions.assertEquals(TLS.V_1_3.getVersion(), switchStrategy.switchProtocol(response4));
@@ -84,42 +90,58 @@ void testSwitchToHTTP11AndTLS() throws Exception {
8490
@Test
8591
void testSwitchInvalid() {
8692
final HttpResponse response1 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
93+
response1.addHeader(HttpHeaders.CONNECTION, "Upgrade");
8794
response1.addHeader(HttpHeaders.UPGRADE, "Crap");
8895
Assertions.assertThrows(ProtocolException.class, () -> switchStrategy.switchProtocol(response1));
8996

9097
final HttpResponse response2 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
98+
response2.addHeader(HttpHeaders.CONNECTION, "Upgrade");
9199
response2.addHeader(HttpHeaders.UPGRADE, "TLS, huh?");
92100
Assertions.assertThrows(ProtocolException.class, () -> switchStrategy.switchProtocol(response2));
93101

94102
final HttpResponse response3 = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
103+
response3.addHeader(HttpHeaders.CONNECTION, "Upgrade");
95104
response3.addHeader(HttpHeaders.UPGRADE, ",,,");
96105
Assertions.assertThrows(ProtocolException.class, () -> switchStrategy.switchProtocol(response3));
97106
}
98107

108+
@Test
109+
void testNullToken() throws ProtocolException {
110+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
111+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
112+
response.addHeader(HttpHeaders.UPGRADE, "TLS,");
113+
response.addHeader(HttpHeaders.UPGRADE, null);
114+
Assertions.assertEquals(TLS.V_1_2.getVersion(), switchStrategy.switchProtocol(response));
115+
}
116+
99117
@Test
100118
void testWhitespaceOnlyToken() throws ProtocolException {
101119
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
120+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
102121
response.addHeader(HttpHeaders.UPGRADE, " , TLS");
103122
Assertions.assertEquals(TLS.V_1_2.getVersion(), switchStrategy.switchProtocol(response));
104123
}
105124

106125
@Test
107126
void testUnsupportedTlsVersion() throws Exception {
108127
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
128+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
109129
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.4");
110130
Assertions.assertEquals(new ProtocolVersion("TLS", 1, 4), switchStrategy.switchProtocol(response));
111131
}
112132

113133
@Test
114134
void testUnsupportedTlsMajorVersion() throws Exception {
115135
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
136+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
116137
response.addHeader(HttpHeaders.UPGRADE, "TLS/2.0");
117138
Assertions.assertEquals(new ProtocolVersion("TLS", 2, 0), switchStrategy.switchProtocol(response));
118139
}
119140

120141
@Test
121142
void testUnsupportedHttpVersion() {
122143
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
144+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
123145
response.addHeader(HttpHeaders.UPGRADE, "HTTP/2.0");
124146
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
125147
switchStrategy.switchProtocol(response));
@@ -129,6 +151,7 @@ void testUnsupportedHttpVersion() {
129151
@Test
130152
void testInvalidTlsFormat() {
131153
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
154+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
132155
response.addHeader(HttpHeaders.UPGRADE, "TLS/abc");
133156
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
134157
switchStrategy.switchProtocol(response));
@@ -138,6 +161,7 @@ void testInvalidTlsFormat() {
138161
@Test
139162
void testHttp11Only() {
140163
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
164+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
141165
response.addHeader(HttpHeaders.UPGRADE, "HTTP/1.1");
142166
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
143167
switchStrategy.switchProtocol(response));
@@ -147,6 +171,7 @@ void testHttp11Only() {
147171
@Test
148172
void testSwitchToTlsValid_TLS_1_2() throws Exception {
149173
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
174+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
150175
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.2");
151176
final ProtocolVersion result = switchStrategy.switchProtocol(response);
152177
Assertions.assertEquals(TLS.V_1_2.getVersion(), result);
@@ -155,6 +180,7 @@ void testSwitchToTlsValid_TLS_1_2() throws Exception {
155180
@Test
156181
void testSwitchToTlsValid_TLS_1_0() throws Exception {
157182
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
183+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
158184
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.0");
159185
final ProtocolVersion result = switchStrategy.switchProtocol(response);
160186
Assertions.assertEquals(TLS.V_1_0.getVersion(), result);
@@ -163,6 +189,7 @@ void testSwitchToTlsValid_TLS_1_0() throws Exception {
163189
@Test
164190
void testSwitchToTlsValid_TLS_1_1() throws Exception {
165191
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
192+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
166193
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.1");
167194
final ProtocolVersion result = switchStrategy.switchProtocol(response);
168195
Assertions.assertEquals(TLS.V_1_1.getVersion(), result);
@@ -171,6 +198,7 @@ void testSwitchToTlsValid_TLS_1_1() throws Exception {
171198
@Test
172199
void testInvalidTlsFormat_NoSlash() {
173200
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
201+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
174202
response.addHeader(HttpHeaders.UPGRADE, "TLSv1");
175203
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
176204
switchStrategy.switchProtocol(response));
@@ -180,6 +208,7 @@ void testInvalidTlsFormat_NoSlash() {
180208
@Test
181209
void testSwitchToTlsValid_TLS_1() throws Exception {
182210
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
211+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
183212
response.addHeader(HttpHeaders.UPGRADE, "TLS/1");
184213
final ProtocolVersion result = switchStrategy.switchProtocol(response);
185214
Assertions.assertEquals(TLS.V_1_0.getVersion(), result);
@@ -188,6 +217,7 @@ void testSwitchToTlsValid_TLS_1() throws Exception {
188217
@Test
189218
void testInvalidTlsFormat_MissingMajor() {
190219
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
220+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
191221
response.addHeader(HttpHeaders.UPGRADE, "TLS/.1");
192222
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
193223
switchStrategy.switchProtocol(response));
@@ -197,6 +227,7 @@ void testInvalidTlsFormat_MissingMajor() {
197227
@Test
198228
void testMultipleHttp11Tokens() {
199229
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
230+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
200231
response.addHeader(HttpHeaders.UPGRADE, "HTTP/1.1, HTTP/1.1");
201232
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
202233
switchStrategy.switchProtocol(response));
@@ -206,6 +237,7 @@ void testMultipleHttp11Tokens() {
206237
@Test
207238
void testMixedInvalidAndValidTokens() {
208239
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
240+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
209241
response.addHeader(HttpHeaders.UPGRADE, "Crap, TLS/1.2, Invalid");
210242
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
211243
switchStrategy.switchProtocol(response));
@@ -215,10 +247,41 @@ void testMixedInvalidAndValidTokens() {
215247
@Test
216248
void testInvalidTlsFormat_NoProtocolName() {
217249
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
250+
response.addHeader(HttpHeaders.CONNECTION, "Upgrade");
218251
response.addHeader(HttpHeaders.UPGRADE, ",,/1.1");
219252
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
220253
switchStrategy.switchProtocol(response));
221254
Assertions.assertEquals("Invalid protocol; error at offset 2: <,,/1.1>", ex.getMessage());
222255
}
223256

257+
@Test
258+
void testMissingConnectionUpgradeRejected() {
259+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
260+
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.2");
261+
262+
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
263+
switchStrategy.switchProtocol(response));
264+
Assertions.assertEquals("Invalid protocol switch response: missing Connection: Upgrade", ex.getMessage());
265+
}
266+
267+
@Test
268+
void testConnectionWithoutUpgradeTokenRejected() {
269+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
270+
response.addHeader(HttpHeaders.CONNECTION, "keep-alive");
271+
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.2");
272+
273+
final ProtocolException ex = Assertions.assertThrows(ProtocolException.class, () ->
274+
switchStrategy.switchProtocol(response));
275+
Assertions.assertEquals("Invalid protocol switch response: missing Connection: Upgrade", ex.getMessage());
276+
}
277+
278+
@Test
279+
void testConnectionWithUpgradeTokenInListAccepted() throws Exception {
280+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
281+
response.addHeader(HttpHeaders.CONNECTION, "keep-alive, Upgrade");
282+
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.2");
283+
284+
Assertions.assertEquals(TLS.V_1_2.getVersion(), switchStrategy.switchProtocol(response));
285+
}
286+
224287
}

0 commit comments

Comments
 (0)