Regression: async execution runtimes set the negotiated protocol version in the execution context at the wrong point of request execution

ok2c · ok2c · commit 2f87e4a6b7af · 2022-11-21T22:37:49.000+01:00
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalH2AsyncExecRuntime.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalH2AsyncExecRuntime.java
@@ -102,7 +102,6 @@ public void completed(final IOSession ioSession) {
                             if (log.isDebugEnabled()) {
                                 log.debug("{} acquired endpoint", id);
                             }
-                            context.setProtocolVersion(HttpVersion.HTTP_2);
                             callback.completed(InternalH2AsyncExecRuntime.this);
                         }
 
@@ -237,6 +236,7 @@ public Cancellable execute(
             if (log.isDebugEnabled()) {
                 log.debug("{} start execution {}", ConnPoolSupport.getId(endpoint), id);
             }
+            context.setProtocolVersion(HttpVersion.HTTP_2);
             session.enqueue(
                     new RequestExecutionCommand(exchangeHandler, pushHandlerFactory, complexCancellable, context),
                     Command.Priority.NORMAL);
@@ -254,6 +254,7 @@ public void completed(final IOSession ioSession) {
                     if (log.isDebugEnabled()) {
                         log.debug("{} start execution {}", ConnPoolSupport.getId(endpoint), id);
                     }
+                    context.setProtocolVersion(HttpVersion.HTTP_2);
                     session.enqueue(
                             new RequestExecutionCommand(exchangeHandler, pushHandlerFactory, complexCancellable, context),
                             Command.Priority.NORMAL);
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java
@@ -465,7 +465,6 @@ public void completed(final ManagedAsyncClientConnection connection) {
                                 LOG.debug("{} connected {}", ConnPoolSupport.getId(endpoint), ConnPoolSupport.getId(connection));
                             }
                             final ProtocolVersion protocolVersion = connection.getProtocolVersion();
-                            context.setProtocolVersion(protocolVersion);
                             final Timeout socketTimeout = connectionConfig.getSocketTimeout();
                             if (socketTimeout != null) {
                                 connection.setSocketTimeout(socketTimeout);
@@ -521,7 +520,6 @@ public void completed(final ManagedAsyncClientConnection connection) {
 
                                 @Override
                                 public void completed(final ProtocolIOSession protocolIOSession) {
-                                    context.setProtocolVersion(HttpVersion.HTTP_2);
                                     if (callback != null) {
                                         callback.completed(endpoint);
                                     }
@@ -750,6 +748,7 @@ public void execute(
             if (LOG.isDebugEnabled()) {
                 LOG.debug("{} executing exchange {} over {}", id, exchangeId, ConnPoolSupport.getId(connection));
             }
+            context.setProtocolVersion(connection.getProtocolVersion());
             connection.submitCommand(
                     new RequestExecutionCommand(exchangeHandler, pushHandlerFactory, context),
                     Command.Priority.NORMAL);

Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,6 @@ public void completed(final IOSession ioSession) {`
`102`	`102`	`if (log.isDebugEnabled()) {`
`103`	`103`	`log.debug("{} acquired endpoint", id);`
`104`	`104`	`}`
`105`		`- context.setProtocolVersion(HttpVersion.HTTP_2);`
`106`	`105`	`callback.completed(InternalH2AsyncExecRuntime.this);`
`107`	`106`	`}`
`108`	`107`
`@@ -237,6 +236,7 @@ public Cancellable execute(`
`237`	`236`	`if (log.isDebugEnabled()) {`
`238`	`237`	`log.debug("{} start execution {}", ConnPoolSupport.getId(endpoint), id);`
`239`	`238`	`}`
	`239`	`+ context.setProtocolVersion(HttpVersion.HTTP_2);`
`240`	`240`	`session.enqueue(`
`241`	`241`	`new RequestExecutionCommand(exchangeHandler, pushHandlerFactory, complexCancellable, context),`
`242`	`242`	`Command.Priority.NORMAL);`
`@@ -254,6 +254,7 @@ public void completed(final IOSession ioSession) {`
`254`	`254`	`if (log.isDebugEnabled()) {`
`255`	`255`	`log.debug("{} start execution {}", ConnPoolSupport.getId(endpoint), id);`
`256`	`256`	`}`
	`257`	`+ context.setProtocolVersion(HttpVersion.HTTP_2);`
`257`	`258`	`session.enqueue(`
`258`	`259`	`new RequestExecutionCommand(exchangeHandler, pushHandlerFactory, complexCancellable, context),`
`259`	`260`	`Command.Priority.NORMAL);`