From 7b4fa59382d5fe50aa53d2d7334a42c5ff85fa63 Mon Sep 17 00:00:00 2001 From: paulb Date: Mon, 23 Dec 2024 19:29:07 +0100 Subject: [PATCH 1/9] Add "Content-Type" to HEAD request --- .../org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index c26d13606b6..cbbbecfa45d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -380,7 +380,8 @@ private synchronized boolean maybeTryHeadRequestSync(String url) { } HttpRequest.Builder headReqB = HttpRequest.newBuilder(uriNoQueryParams) - .method("HEAD", HttpRequest.BodyPublishers.noBody()); + .method("HEAD", HttpRequest.BodyPublishers.noBody()) + .header("Content-Type", ClientUtils.TEXT_JSON); decorateRequest(headReqB, new QueryRequest()); try { httpClient.send(headReqB.build(), HttpResponse.BodyHandlers.discarding()); From 9165fe94009b19058791d0d4ce0eded20f987dbb Mon Sep 17 00:00:00 2001 From: paulb Date: Mon, 23 Dec 2024 21:26:03 +0100 Subject: [PATCH 2/9] Fix for spotlessJavaCheck format validation --- .../org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index cbbbecfa45d..561161b1760 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -381,7 +381,7 @@ private synchronized boolean maybeTryHeadRequestSync(String url) { HttpRequest.Builder headReqB = HttpRequest.newBuilder(uriNoQueryParams) .method("HEAD", HttpRequest.BodyPublishers.noBody()) - .header("Content-Type", ClientUtils.TEXT_JSON); + .header("Content-Type", ClientUtils.TEXT_JSON); decorateRequest(headReqB, new QueryRequest()); try { httpClient.send(headReqB.build(), HttpResponse.BodyHandlers.discarding()); From 692c70938c6ed2ef0a553be8ba10d02ccaaeaad4 Mon Sep 17 00:00:00 2001 From: paulb Date: Fri, 27 Dec 2024 15:05:54 +0100 Subject: [PATCH 3/9] Add test for maybeTryHeadRequest (only returns true when HttpStatusCode == 200) --- .../solr/client/solrj/impl/HttpJdkSolrClient.java | 7 +++++-- .../client/solrj/impl/HttpJdkSolrClientTest.java | 13 +++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index 561161b1760..fb480974b3b 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -384,8 +384,11 @@ private synchronized boolean maybeTryHeadRequestSync(String url) { .header("Content-Type", ClientUtils.TEXT_JSON); decorateRequest(headReqB, new QueryRequest()); try { - httpClient.send(headReqB.build(), HttpResponse.BodyHandlers.discarding()); - headSucceeded = true; + headSucceeded = + 200 + == httpClient + .send(headReqB.build(), HttpResponse.BodyHandlers.discarding()) + .statusCode(); } catch (IOException ioe) { log.warn("Could not issue HEAD request to {} ", url, ioe); headSucceeded = false; diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 1dbfc0e998b..6dc66b51d51 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -478,6 +478,19 @@ public void testPing() throws Exception { } } + @Test + public void testMaybeTryHeadRequestIsFalseWhenHttpStatusCodeIsNot200() throws Exception { + String url = getCoreUrl(); + try (HttpJdkSolrClient client = builder(url).build()) { + // valid request: returns 200 (OK) + assertTrue(client.maybeTryHeadRequest(url + "/update")); + } + try (HttpJdkSolrClient client = builder(url).build()) { + // invalid request: returns 404 (Not Found) in this case + assertFalse(client.maybeTryHeadRequest(url + "/undefined-request-handler")); + } + } + /** * This is not required for any test, but there appears to be a bug in the JDK client where it * does not release all threads if the client has not performed any queries, even after a forced From 412ceca61ff1a9b9c3c108ebea60772082ba5bd0 Mon Sep 17 00:00:00 2001 From: paulb Date: Mon, 30 Dec 2024 11:10:10 +0100 Subject: [PATCH 4/9] Revert "Add test for maybeTryHeadRequest (only returns true when HttpStatusCode == 200)" This reverts commit 692c70938c6ed2ef0a553be8ba10d02ccaaeaad4. --- .../solr/client/solrj/impl/HttpJdkSolrClient.java | 7 ++----- .../client/solrj/impl/HttpJdkSolrClientTest.java | 13 ------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index fb480974b3b..561161b1760 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -384,11 +384,8 @@ private synchronized boolean maybeTryHeadRequestSync(String url) { .header("Content-Type", ClientUtils.TEXT_JSON); decorateRequest(headReqB, new QueryRequest()); try { - headSucceeded = - 200 - == httpClient - .send(headReqB.build(), HttpResponse.BodyHandlers.discarding()) - .statusCode(); + httpClient.send(headReqB.build(), HttpResponse.BodyHandlers.discarding()); + headSucceeded = true; } catch (IOException ioe) { log.warn("Could not issue HEAD request to {} ", url, ioe); headSucceeded = false; diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 6dc66b51d51..1dbfc0e998b 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -478,19 +478,6 @@ public void testPing() throws Exception { } } - @Test - public void testMaybeTryHeadRequestIsFalseWhenHttpStatusCodeIsNot200() throws Exception { - String url = getCoreUrl(); - try (HttpJdkSolrClient client = builder(url).build()) { - // valid request: returns 200 (OK) - assertTrue(client.maybeTryHeadRequest(url + "/update")); - } - try (HttpJdkSolrClient client = builder(url).build()) { - // invalid request: returns 404 (Not Found) in this case - assertFalse(client.maybeTryHeadRequest(url + "/undefined-request-handler")); - } - } - /** * This is not required for any test, but there appears to be a bug in the JDK client where it * does not release all threads if the client has not performed any queries, even after a forced From e7ee0e32d3263fd7acfcddcdef126ae1297a9bbd Mon Sep 17 00:00:00 2001 From: paulb Date: Mon, 30 Dec 2024 14:59:44 +0100 Subject: [PATCH 5/9] Add test for maybeTryHeadRequest (check "Content-Type" is provided) --- .../solr/client/solrj/impl/HttpJdkSolrClientTest.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 1dbfc0e998b..6756fd0b052 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -478,6 +478,17 @@ public void testPing() throws Exception { } } + @Test + public void testMaybeTryHeadRequestHasContentType() throws Exception { + DebugServlet.clear(); + String url = getBaseUrl() + DEBUG_SERVLET_PATH; + try (HttpJdkSolrClient client = builder(url).build()) { + assertTrue(client.maybeTryHeadRequest(url)); + } + assertEquals("head", DebugServlet.lastMethod); + assertTrue(DebugServlet.headers.containsKey("content-type")); + } + /** * This is not required for any test, but there appears to be a bug in the JDK client where it * does not release all threads if the client has not performed any queries, even after a forced From a9a9af3a8ed13ecd372a4648cfd394044ef0cad3 Mon Sep 17 00:00:00 2001 From: paulb Date: Fri, 3 Jan 2025 15:10:28 +0100 Subject: [PATCH 6/9] Fix test --- .../apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 6756fd0b052..7b460c4797f 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -484,9 +484,9 @@ public void testMaybeTryHeadRequestHasContentType() throws Exception { String url = getBaseUrl() + DEBUG_SERVLET_PATH; try (HttpJdkSolrClient client = builder(url).build()) { assertTrue(client.maybeTryHeadRequest(url)); + assertEquals("head", DebugServlet.lastMethod); + assertTrue(DebugServlet.headers.containsKey("content-type")); } - assertEquals("head", DebugServlet.lastMethod); - assertTrue(DebugServlet.headers.containsKey("content-type")); } /** From 802b329de92eafd947d1abe17dff722e91cdaf5d Mon Sep 17 00:00:00 2001 From: paulb Date: Mon, 13 Jan 2025 09:52:44 +0100 Subject: [PATCH 7/9] Add CHANGES.txt entry --- solr/CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 6e36e4ca99c..dbc6850aafa 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -183,6 +183,8 @@ Other Changes * GITHUB#2680: Improve reliablity of NpmTasks finding needed files/commands. (Tyler Bertrand via Eric Pugh) +* SOLR-17589: Prevent error log entry on solr server due to initial HEAD request from HttpJdkSolrClient. (Paul Blanchaert via James Dyer) + ================== 9.8.0 ================== New Features --------------------- From 6b7b2f114c700379ffb73bdc953e9d3e2683852d Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Mon, 13 Jan 2025 09:45:36 -0600 Subject: [PATCH 8/9] fix test for case where https is randomly selected (no HEAD request attempted) --- .../solr/client/solrj/impl/HttpJdkSolrClientTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 217a6575161..414155571ab 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -504,8 +504,12 @@ public void testMaybeTryHeadRequestHasContentType() throws Exception { String url = getBaseUrl() + DEBUG_SERVLET_PATH; try (HttpJdkSolrClient client = builder(url).build()) { assertTrue(client.maybeTryHeadRequest(url)); - assertEquals("head", DebugServlet.lastMethod); - assertTrue(DebugServlet.headers.containsKey("content-type")); + + // if https, the client won't attempt a HEAD request + if(client.headRequested) { + assertEquals("head", DebugServlet.lastMethod); + assertTrue(DebugServlet.headers.containsKey("content-type")); + } } } From eca289c0e2c966894e7d0b2411fae6d80fac8665 Mon Sep 17 00:00:00 2001 From: jdyer1 Date: Mon, 13 Jan 2025 09:47:01 -0600 Subject: [PATCH 9/9] tidy --- .../apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 414155571ab..f0880e1ae7c 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -506,7 +506,7 @@ public void testMaybeTryHeadRequestHasContentType() throws Exception { assertTrue(client.maybeTryHeadRequest(url)); // if https, the client won't attempt a HEAD request - if(client.headRequested) { + if (client.headRequested) { assertEquals("head", DebugServlet.lastMethod); assertTrue(DebugServlet.headers.containsKey("content-type")); }