jdyer1 commented on code in PR #2259:
URL: https://github.com/apache/solr/pull/2259#discussion_r1521448629
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrJdkClient.java:
##########
@@ -278,64 +285,74 @@ private HttpResponse<InputStream> doPutOrPost(
return response;
}
- private volatile boolean headRequested;
- private volatile boolean headSucceeded;
-
/**
- * TODO: we should only try this if not set to Http/1.1 and not TLS
- *
- *
- * This is a workaround for the case where the first request using
- * this client:
+ * This is a workaround for the case where:
*
- * (1) no SSL/TLS (2) using POST with stream and (3) using Http/2
+ * <p>(1) no SSL/TLS (2) using POST with stream and (3) using Http/2
*
- * The JDK Http Client will send an upgrade request over Http/1
- * along with request content in the same request. However,
- * the Jetty Server underpinning Solr does not accept this.
+ * <p>The JDK Http Client will send an upgrade request over Http/1 along
with request content in
+ * the same request. However, the Jetty Server underpinning Solr does not
accept this.
*
- * By sending a ping before any real requests occur, the client
- * knows if Solr can accept Http/2, and no additional
- * upgrade requests will be sent.
+ * <p>We send a HEAD request first, then the client knows if Solr can accept
Http/2, and no
+ * additional upgrade requests will be sent.
*
- * See https://bugs.openjdk.org/browse/JDK-8287589
- * See
https://github.com/jetty/jetty.project/issues/9998#issuecomment-1614216870
+ * <p>See https://bugs.openjdk.org/browse/JDK-8287589 See
+ * https://github.com/jetty/jetty.project/issues/9998#issuecomment-1614216870
*
- * We only try once, and if it fails, we downgrade to Http/1
+ * <p>We only try once, and if it fails, we downgrade to Http/1
*
* @param url the url with no request parameters
* @return true if success
*/
- private synchronized boolean maybeTryHeadRequest(String url) {
- if(headRequested) {
+ private boolean maybeTryHeadRequest(String url) {
+ if (forceHttp11 || url == null ||
url.toLowerCase(Locale.ROOT).startsWith("https://";)) {
+ return true;
+ }
+ return maybeTryHeadRequestSync(url);
+ }
+
+ protected volatile boolean headRequested; // must be threadsafe
+ private boolean headSucceeded; // must be threadsafe
+
+ private synchronized boolean maybeTryHeadRequestSync(String url) {
+ if (headRequested) {
return headSucceeded;
}
+
URI uriNoQueryParams;
try {
uriNoQueryParams = new URI(url);
- } catch(URISyntaxException e) {
- headSucceeded = false;
+ } catch (URISyntaxException e) {
+
+ // If the url is invalid, let a subsequent request try again.
return false;
}
- HttpRequest.Builder headReqB =
HttpRequest.newBuilder(uriNoQueryParams).method("HEAD",
HttpRequest.BodyPublishers.noBody());
+ HttpRequest.Builder headReqB =
+ HttpRequest.newBuilder(uriNoQueryParams)
+ .method("HEAD", HttpRequest.BodyPublishers.noBody());
decorateRequest(headReqB, new QueryRequest());
try {
httpClient.send(headReqB.build(),
HttpResponse.BodyHandlers.discarding());
headSucceeded = true;
- } catch(IOException ioe) {
+ } catch (IOException ioe) {
log.warn("Could not issue HEAD request to {} ", url, ioe);
headSucceeded = false;
- } catch(InterruptedException ie) {
+ } catch (InterruptedException ie) {
Thread.currentThread().interrupt();
headSucceeded = false;
} finally {
+
+ // The HEAD request is tried only once. All future requests will skip
this check.
headRequested = true;
+
+ if (!headSucceeded) {
+ log.info("All insecure POST requests with a chunked body will use
http/1.1");
Review Comment:
I can change this to _unencrypted_. Let's try not to offend our users, eh?
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
