Skip to content

Commit 17dafc8

Browse files
arturobernalgarturobernalg
committed
Simplify ProtocolSwitchStrategy by Leveraging ProtocolVersionParser
Unify HTTP and TLS token parsing in the Upgrade header by replacing custom version parsing with ProtocolVersionParser. This change removes redundant code and ensures that only supported protocols (HTTP/ and TLS tokens) are accepted, while all other upgrade protocols are rejected as unsupported.
1 parent 5e62dac commit 17dafc8

2 files changed

Lines changed: 140 additions & 15 deletions

File tree

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

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@
3434
import org.apache.hc.core5.http.ParseException;
3535
import org.apache.hc.core5.http.ProtocolException;
3636
import org.apache.hc.core5.http.ProtocolVersion;
37+
import org.apache.hc.core5.http.ProtocolVersionParser;
3738
import org.apache.hc.core5.http.message.MessageSupport;
3839
import org.apache.hc.core5.http.ssl.TLS;
40+
import org.apache.hc.core5.util.CharArrayBuffer;
41+
import org.apache.hc.core5.util.Tokenizer;
3942

4043
/**
4144
* Protocol switch handler.
@@ -45,31 +48,53 @@
4548
@Internal
4649
public final class ProtocolSwitchStrategy {
4750

48-
enum ProtocolSwitch { FAILURE, TLS }
51+
private static final ProtocolVersionParser PARSER = ProtocolVersionParser.INSTANCE;
4952

5053
public ProtocolVersion switchProtocol(final HttpMessage response) throws ProtocolException {
5154
final Iterator<String> it = MessageSupport.iterateTokens(response, HttpHeaders.UPGRADE);
52-
5355
ProtocolVersion tlsUpgrade = null;
56+
ProtocolVersion httpUpgrade = null;
57+
5458
while (it.hasNext()) {
55-
final String token = it.next();
56-
if (token.startsWith("TLS")) {
57-
// TODO: Improve handling of HTTP protocol token once HttpVersion has a #parse method
58-
try {
59-
tlsUpgrade = token.length() == 3 ? TLS.V_1_2.getVersion() : TLS.parse(token.replace("TLS/", "TLSv"));
60-
} catch (final ParseException ex) {
61-
throw new ProtocolException("Invalid protocol: " + token);
59+
final String token = it.next().trim();
60+
try {
61+
if (token.startsWith("TLS") || token.startsWith("HTTP/")) {
62+
final ProtocolVersion version = parseToken(token);
63+
if (token.startsWith("TLS")) {
64+
tlsUpgrade = version;
65+
} else {
66+
httpUpgrade = version;
67+
}
68+
} else {
69+
throw new ProtocolException("Unsupported protocol: " + token);
70+
}
71+
} catch (final ParseException ex) {
72+
if (token.startsWith("TLS")) {
73+
throw new ProtocolException("Invalid TLS protocol: " + token, ex);
74+
} else if (token.startsWith("HTTP/")) {
75+
throw new ProtocolException("Invalid HTTP protocol: " + token, ex);
76+
} else {
77+
throw new ProtocolException("Unsupported protocol: " + token, ex);
6278
}
63-
} else if (token.equals("HTTP/1.1")) {
64-
// TODO: Improve handling of HTTP protocol token once HttpVersion has a #parse method
65-
} else {
66-
throw new ProtocolException("Unsupported protocol: " + token);
6779
}
6880
}
69-
if (tlsUpgrade == null) {
81+
82+
if (tlsUpgrade != null) {
83+
return tlsUpgrade;
84+
} else if (httpUpgrade != null) {
85+
return httpUpgrade;
86+
} else {
7087
throw new ProtocolException("Invalid protocol switch response");
7188
}
72-
return tlsUpgrade;
7389
}
7490

91+
private ProtocolVersion parseToken(final String token) throws ParseException {
92+
if (token.equals("TLS")) {
93+
return TLS.V_1_2.getVersion();
94+
}
95+
final CharArrayBuffer buffer = new CharArrayBuffer(token.length());
96+
buffer.append(token);
97+
final Tokenizer.Cursor cursor = new Tokenizer.Cursor(0, buffer.length());
98+
return PARSER.parse(buffer, cursor, null);
99+
}
75100
}

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
import org.apache.hc.core5.http.HttpHeaders;
3030
import org.apache.hc.core5.http.HttpResponse;
3131
import org.apache.hc.core5.http.HttpStatus;
32+
import org.apache.hc.core5.http.HttpVersion;
3233
import org.apache.hc.core5.http.ProtocolException;
34+
import org.apache.hc.core5.http.ProtocolVersion;
3335
import org.apache.hc.core5.http.message.BasicHttpResponse;
3436
import org.apache.hc.core5.http.ssl.TLS;
3537
import org.junit.jupiter.api.Assertions;
@@ -95,4 +97,102 @@ void testSwitchInvalid() {
9597
Assertions.assertThrows(ProtocolException.class, () -> switchStrategy.switchProtocol(response3));
9698
}
9799

100+
@Test
101+
void testSwitchToTlsValid_TLS_2_0() throws ProtocolException {
102+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
103+
response.addHeader(HttpHeaders.UPGRADE, "HTTP/2.0");
104+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
105+
Assertions.assertEquals(HttpVersion.HTTP_2_0, result);
106+
}
107+
108+
@Test
109+
void testSwitchToHttpValid_HTTP_1_1() throws Exception {
110+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
111+
response.addHeader(HttpHeaders.UPGRADE, "HTTP/1.1");
112+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
113+
Assertions.assertEquals(HttpVersion.HTTP_1_1, result);
114+
}
115+
116+
@Test
117+
void testUnsupportedHttpVersion() throws ProtocolException {
118+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
119+
response.addHeader(HttpHeaders.UPGRADE, "HTTP/11.22");
120+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
121+
Assertions.assertEquals(11, result.getMajor());
122+
Assertions.assertEquals(22, result.getMinor());
123+
}
124+
125+
@Test
126+
void testSwitchToTlsValid_TLS_1_2() throws Exception {
127+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
128+
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.2");
129+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
130+
Assertions.assertEquals(TLS.V_1_2.getVersion(), result);
131+
}
132+
133+
// New Tests for parseTlsToken Casuistics
134+
@Test
135+
void testSwitchToTlsValid_TLS_1_0() throws Exception {
136+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
137+
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.0");
138+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
139+
Assertions.assertEquals(TLS.V_1_0.getVersion(), result);
140+
}
141+
142+
@Test
143+
void testSwitchToTlsValid_TLS_1_1() throws Exception {
144+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
145+
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.1");
146+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
147+
Assertions.assertEquals(TLS.V_1_1.getVersion(), result);
148+
}
149+
150+
@Test
151+
void testUnsupportedTlsVersion_TLS_1_4() throws ProtocolException {
152+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
153+
response.addHeader(HttpHeaders.UPGRADE, "TLS/1.4");
154+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
155+
Assertions.assertEquals(1, result.getMajor());
156+
Assertions.assertEquals(4, result.getMinor());
157+
}
158+
159+
@Test
160+
void testTlsVersion_TLS_2_0() throws ProtocolException {
161+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
162+
response.addHeader(HttpHeaders.UPGRADE, "TLS/2.0");
163+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
164+
Assertions.assertEquals(2, result.getMajor());
165+
}
166+
167+
@Test
168+
void testInvalidTlsFormat_NoSlash() {
169+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
170+
response.addHeader(HttpHeaders.UPGRADE, "TLSv1");
171+
Assertions.assertThrows(ProtocolException.class, () -> switchStrategy.switchProtocol(response),
172+
"Invalid TLS protocol format: TLSv1");
173+
}
174+
175+
@Test
176+
void testInvalidTlsFormat_NonNumeric() {
177+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
178+
response.addHeader(HttpHeaders.UPGRADE, "TLS/abc");
179+
Assertions.assertThrows(ProtocolException.class, () -> switchStrategy.switchProtocol(response),
180+
"Invalid TLS version: abc");
181+
}
182+
183+
@Test
184+
void testSwitchToTlsValid_TLS_1() throws ProtocolException {
185+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
186+
response.addHeader(HttpHeaders.UPGRADE, "TLS/1");
187+
final ProtocolVersion result = switchStrategy.switchProtocol(response);
188+
Assertions.assertEquals(TLS.V_1_0.getVersion(), result);
189+
}
190+
191+
@Test
192+
void testInvalidTlsFormat_MissingMajor() {
193+
final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_SWITCHING_PROTOCOLS);
194+
response.addHeader(HttpHeaders.UPGRADE, "TLS/.1");
195+
Assertions.assertThrows(ProtocolException.class, () -> switchStrategy.switchProtocol(response),
196+
"Invalid TLS version: .1");
197+
}
98198
}

0 commit comments

Comments
 (0)