ok2c commented on code in PR #627:
URL: 
https://github.com/apache/httpcomponents-client/pull/627#discussion_r2014948461


##########
httpclient5/src/main/java/org/apache/hc/client5/http/impl/ProtocolSwitchStrategy.java:
##########
@@ -45,31 +49,96 @@
 @Internal
 public final class ProtocolSwitchStrategy {
 
-    enum ProtocolSwitch { FAILURE, TLS }
+    private static final ProtocolVersionParser PARSER = 
ProtocolVersionParser.INSTANCE;
+    private static final Tokenizer TOKENIZER = Tokenizer.INSTANCE;
 
     public ProtocolVersion switchProtocol(final HttpMessage response) throws 
ProtocolException {
         final Iterator<String> it = MessageSupport.iterateTokens(response, 
HttpHeaders.UPGRADE);
-
         ProtocolVersion tlsUpgrade = null;
+        ProtocolVersion httpUpgrade = null;
+
         while (it.hasNext()) {
-            final String token = it.next();
+            final String token = it.next().trim();
             if (token.startsWith("TLS")) {
-                // TODO: Improve handling of HTTP protocol token once 
HttpVersion has a #parse method
                 try {
-                    tlsUpgrade = token.length() == 3 ? TLS.V_1_2.getVersion() 
: TLS.parse(token.replace("TLS/", "TLSv"));
+                    tlsUpgrade = parseTlsToken(token);
+                } catch (final ParseException ex) {
+                    throw new ProtocolException("Invalid TLS protocol: " + 
token, ex);
+                }
+            } else if (token.startsWith("HTTP/")) {
+                try {
+                    httpUpgrade = parseHttpToken(token);
                 } catch (final ParseException ex) {
-                    throw new ProtocolException("Invalid protocol: " + token);
+                    throw new ProtocolException("Invalid HTTP protocol: " + 
token, ex);
                 }
-            } else if (token.equals("HTTP/1.1")) {
-                // TODO: Improve handling of HTTP protocol token once 
HttpVersion has a #parse method
             } else {
                 throw new ProtocolException("Unsupported protocol: " + token);
             }
         }
-        if (tlsUpgrade == null) {
+
+        if (tlsUpgrade != null) {
+            return tlsUpgrade;
+        } else if (httpUpgrade != null) {
+            return httpUpgrade;
+        } else {
             throw new ProtocolException("Invalid protocol switch response");
         }
-        return tlsUpgrade;
     }
 
-}
+    private ProtocolVersion parseTlsToken(final String token) throws 
ParseException {

Review Comment:
   @arturobernalg Am I missing something? There is a `parse`method in 
`ProtocolVersionParser` that should be able to do all this with one line of 
code.
   
   ```
   MatcherAssert.assertThat(ProtocolVersion.parse(" PROTO/1.2,0000", cursor, 
Tokenizer.delimiters(',')), CoreMatchers.equalTo(new ProtocolVersion("PROTO", 
1, 2)));
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org

Reply via email to