test(search): /api/es IT coverage + docs for non-finite _score regression (#36478)

docs/backend/OPENSEARCH_MIGRATION.md

@@ -593,6 +593,51 @@ hand carries genuine semantic intent — and routing by tag is the natural expre
 - Index timestamp is part of the identity — never hardcode index names
 - Working index is always a superset — never write to live without also writing to working
 
+### Non-finite numbers (`NaN` / `Infinity`) in manual JSON serialization — #36478
+
+**Rule: any `float`/`double` you serialize that originates from an underlying search/index/DB/compute
+API can be non-finite. Coerce it to `null` before it reaches the serializer. Never assume a number is
+finite just because it is a "score", "distance", or "average".**
+
+**Where non-finite values come from.** Elasticsearch/OpenSearch set a hit `_score` to **`NaN`**
+whenever the hit is *not relevance-scored* — any query that **sorts by a field** without
+`track_scores: true`, plus `filter` / `constant_score` / aggregation-only (`size: 0`) contexts. The
+same hazard applies to suggester option `score`, aggregation metric values (avg/sum/stats on empty or
+degenerate buckets), pgvector distances, and any Java-computed ratio/division (`0.0/0.0 → NaN`,
+`x/0.0 → Infinity`).
+
+**Both serializers are traps, in different ways:**
+
+| Serializer | Behavior on a non-finite number | Symptom |
+|------------|----------------------------------|---------|
+| dotCMS `com.dotmarketing.util.json.JSONObject` / `JSONArray` (strict) | `testValidity()` throws `JSONException("JSON does not allow non-finite numbers.")` — **twice over**: eagerly inside `.put(key, value)` *and* again at serialization inside `numberToString` (reached from `.toString()`) | **HTTP 500** |
+| Jackson `ObjectMapper` | Does **not** throw by default; writes the bare tokens `NaN` / `Infinity` / `-Infinity`, which are **not valid JSON** | Strict client parsers (`JSON.parse`, most SDKs) **reject the response**; silently non-standard payload. `QUOTE_NON_NUMERIC_NUMBERS` only turns them into `"NaN"` strings — still not a number/null a consumer expects |
+
+**Why the ES cutover exposed this.** The pre-#36398 endpoints returned Elasticsearch's *native*
+serializer output (`SearchResponse.toString()`), and ES XContent serializes non-finite as `null`.
+#36398 rebuilt the same wire shape through the **strict** dotCMS `JSONObject`, which rejects what ES
+tolerated — turning a silently-null field into a 500. Matching ES's native `null` behavior is
+therefore the correct fix, not an arbitrary choice.
+
+**The pattern (see `ESContentResourcePortlet.toLegacyEsJson` / `hitsToLegacyJson`):**
+
+- Guard every **explicit** numeric `.put(...)` — the strict writer validates *eagerly*, so the value
+  must already be finite-or-`null` when inserted:
+  ```java
+  .put("_score", finiteOrNull(hit.getScore()))   // NaN/Infinity -> JSONObject.NULL
+  ```
+- For values that enter a tree **unvalidated** — via `new JSONObject(Map)` / bean-wrapping (e.g. the
+  suggester block, `_source` numerics) — a per-field guard is not enough: they skip the eager check
+  but still throw at `.toString()`. Either sanitize the source map, or run one recursive pass over the
+  built tree that coerces every non-finite `Float`/`Double` to `JSONObject.NULL` before serializing.
+  Only `float`/`double` can be non-finite; integral and `BigDecimal` values are safe.
+
+**Regression coverage.** `ESContentResourcePortletNaNScoreTest` (fast unit test) drives the adapter
+with `NaN`/`Infinity` scores and asserts `_score: null` instead of a throw. Note the integration
+tests in `ESContentResourcePortletTest` use relevance-scored `bool`/`term` queries, so they do **not**
+reproduce the trigger — an end-to-end guard needs a *field-sorted* (or `constant_score`/`size:0`)
+query asserting HTTP 200.
+
 ### Index name divergence between providers
 
 **Fresh install** (Phases 1–3 from day zero): ES and OS indices are created in the same bootstrap

dotcms-integration/src/test/java/com/dotcms/rest/elasticsearch/ESContentResourcePortletTest.java

@@ -41,9 +41,15 @@
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
+import javax.servlet.ReadListener;
+import javax.servlet.ServletInputStream;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.ws.rs.core.MediaType;
@@ -338,6 +344,163 @@ public void test_search_esresponse_preservesLegacyEsWireShape() throws Exception
                 typesAggFound);
     }
 
+    /**
+     * Method to test: {@link ESContentResourcePortlet#search(HttpServletRequest, HttpServletResponse, String, String, boolean, String, boolean)}
+     * Given scenario: a query that <b>sorts by a field</b> and does not set {@code track_scores}.
+     *          Elasticsearch/OpenSearch then return a non-finite ({@code NaN}) {@code _score} for
+     *          every hit (the hits are not relevance-scored).
+     * Expected result: HTTP 200 and each hit's {@code _score} serialized as {@code null} (the
+     *          Elasticsearch-native wire format), <b>not</b> HTTP 500
+     *          {@code "JSON does not allow non-finite numbers."}. Regression guard for
+     *          <a href="https://github.com/dotCMS/core/issues/36478">#36478</a>.
+     */
+    @Test
+    public void test_search_fieldSortedQuery_nonFiniteScore_returnsOkWithNullScore() throws Exception {
+        final ContentType contentType = createContentTypeWithPublishedContent();
+
+        final String jsonQuery = "{\n"
+                + "  \"query\": { \"bool\": { \"must\": { \"term\": { \"contenttype\": \""
+                + contentType.variable() + "\" } } } },\n"
+                + "  \"sort\": [ { \"moddate\": \"desc\" } ]\n"
+                + "}";
+
+        final Response response = new ESContentResourcePortlet()
+                .search(createHttpRequest(false), new MockHttpResponse(), jsonQuery, "0", true, null, false);
+
+        assertEquals(Status.OK.getStatusCode(), response.getStatus());
+        assertHitScoresAreNull(response.getEntity().toString());
+    }
+
+    /**
+     * Method to test: {@link ESContentResourcePortlet#searchRaw(HttpServletRequest)}
+     * Given scenario: the same field-sorted (non-relevance-scored) query posted to
+     *          {@code /api/es/raw}, whose body is read from the request input stream.
+     * Expected result: HTTP 200 and {@code _score: null} per hit — {@code /api/es/raw} shares the
+     *          {@code toLegacyEsJson} adapter with {@code /api/es/search}, so it is subject to the
+     *          same #36478 regression and must be guarded too.
+     */
+    @Test
+    public void test_searchRaw_fieldSortedQuery_nonFiniteScore_returnsOkWithNullScore() throws Exception {
+        final ContentType contentType = createContentTypeWithPublishedContent();
+
+        final String jsonQuery = "{\n"
+                + "  \"query\": { \"bool\": { \"must\": { \"term\": { \"contenttype\": \""
+                + contentType.variable() + "\" } } } },\n"
+                + "  \"sort\": [ { \"moddate\": \"desc\" } ]\n"
+                + "}";
+
+        final Response response = new ESContentResourcePortlet()
+                .searchRaw(createHttpRequestWithBody(jsonQuery));
+
+        assertEquals(Status.OK.getStatusCode(), response.getStatus());
+        // searchRaw returns the legacy ES-wire object directly (no "esresponse" wrapper array).
+        assertHitScoresAreNull(response.getEntity().toString(), false);
+    }
+
+    /**
+     * Method to test: {@link ESContentResourcePortlet#search(HttpServletRequest, HttpServletResponse, String, String, boolean, String, boolean)}
+     * Given scenario: a field-sorted query that <b>does</b> set {@code track_scores: true}, forcing
+     *          Elasticsearch to compute a finite relevance score.
+     * Expected result: HTTP 200 and a finite (non-null) {@code _score} — the non-finite guard must
+     *          not alter legitimate finite scores.
+     */
+    @Test
+    public void test_search_fieldSortedQuery_withTrackScores_preservesFiniteScore() throws Exception {
+        final ContentType contentType = createContentTypeWithPublishedContent();
+
+        final String jsonQuery = "{\n"
+                + "  \"track_scores\": true,\n"
+                + "  \"query\": { \"bool\": { \"must\": { \"term\": { \"contenttype\": \""
+                + contentType.variable() + "\" } } } },\n"
+                + "  \"sort\": [ { \"moddate\": \"desc\" } ]\n"
+                + "}";
+
+        final Response response = new ESContentResourcePortlet()
+                .search(createHttpRequest(false), new MockHttpResponse(), jsonQuery, "0", true, null, false);
+
+        assertEquals(Status.OK.getStatusCode(), response.getStatus());
+
+        final JSONObject esresponse = new JSONObject(response.getEntity().toString())
+                .getJSONArray("esresponse").getJSONObject(0);
+        final JSONArray hits = esresponse.getJSONObject("hits").getJSONArray("hits");
+        assertTrue("query must return at least one hit", hits.length() > 0);
+        assertTrue("a track_scores query must keep a finite (non-null) _score",
+                !hits.getJSONObject(0).isNull("_score"));
+    }
+
+    private ContentType createContentTypeWithPublishedContent() throws Exception {
+        final long now = System.currentTimeMillis();
+        final ContentType contentType = new ContentTypeDataGen()
+                .field(new FieldDataGen().name("Title").velocityVarName("title").next())
+                .nextPersisted();
+        final Contentlet contentlet = new ContentletDataGen(contentType.id())
+                .setProperty("title", "nan-score-" + now)
+                .nextPersisted();
+        ContentletDataGen.publish(contentlet);
+        APILocator.getContentletAPI().isInodeIndexed(contentlet.getInode(), true);
+        return contentType;
+    }
+
+    /** Asserts every hit's {@code _score} is {@code null}, reading the {@code /api/es/search} "esresponse" wrapper. */
+    private void assertHitScoresAreNull(final String entity) throws JSONException {
+        assertHitScoresAreNull(entity, true);
+    }
+
+    private void assertHitScoresAreNull(final String entity, final boolean wrappedInEsResponse)
+            throws JSONException {
+        final JSONObject esresponse = wrappedInEsResponse
+                ? new JSONObject(entity).getJSONArray("esresponse").getJSONObject(0)
+                : new JSONObject(entity);
+        final JSONArray hits = esresponse.getJSONObject("hits").getJSONArray("hits");
+        assertTrue("field-sorted query must return at least one hit", hits.length() > 0);
+        for (int i = 0; i < hits.length(); i++) {
+            assertTrue("_score of a non-relevance-scored (field-sorted) hit must serialize as null",
+                    hits.getJSONObject(i).isNull("_score"));
+        }
+    }
+
+    private HttpServletRequest createHttpRequestWithBody(final String body) throws Exception {
+        final HttpServletRequest request = createHttpRequest(false);
+        when(request.getInputStream())
+                .thenReturn(new MockServletInputStream(
+                        new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8))));
+        return request;
+    }
+
+    /** Minimal {@link ServletInputStream} over a byte array, to feed a request body to {@code searchRaw}. */
+    private static class MockServletInputStream extends ServletInputStream {
+
+        private final InputStream sourceStream;
+
+        MockServletInputStream(final InputStream sourceStream) {
+            this.sourceStream = sourceStream;
+        }
+
+        @Override
+        public int read() throws IOException {
+            return sourceStream.read();
+        }
+
+        @Override
+        public boolean isFinished() {
+            try {
+                return sourceStream.available() == 0;
+            } catch (final IOException e) {
+                return true;
+            }
+        }
+
+        @Override
+        public boolean isReady() {
+            return true;
+        }
+
+        @Override
+        public void setReadListener(final ReadListener readListener) {
+            // no-op: synchronous read is sufficient for the test
+        }
+    }
+
     private HttpServletRequest createHttpRequest(final boolean anonymous) throws Exception{
         final MockHeaderRequest request = new MockHeaderRequest(new MockSessionRequest(
                 new MockAttributeRequest(new MockHttpRequestIntegrationTest("localhost", "/").request())