This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch branch_10_1 in repository https://gitbox.apache.org/repos/asf/solr.git
commit 16c89f928e22c90f18c66edfc5ac8675d600f0e0 Author: Nick <[email protected]> AuthorDate: Wed Oct 7 09:03:48 2026 -0600 SOLR-13202: Return 400 instead of 500 for join queries missing from/to parameters (#5012) (cherry picked from commit 93e357bcf6bc9b3b6195043529ef5e1fa2001fde) (cherry picked from commit a9d489d2035d6ee920f2068f025c6217a934603a) --- changelog/unreleased/SOLR-13202.yml | 7 +++++++ .../org/apache/solr/search/JoinQParserPlugin.java | 2 ++ .../search/join/AuxIndexJoinQParserPlugin.java | 4 +--- .../search/join/CrossCollectionJoinQParser.java | 1 + .../solr/search/join/ScoreJoinQParserPlugin.java | 13 ++++++++++++ solr/core/src/test/org/apache/solr/TestJoin.java | 15 ++++++++++++++ .../search/join/CrossCollectionJoinQueryTest.java | 23 ++++++++++++++++++++++ .../search/join/TestAuxIndexJoinQParserPlugin.java | 11 +++++++---- .../search/join/TestGlobalOrdinalsJoinQParser.java | 13 ++++++++++++ 9 files changed, 82 insertions(+), 7 deletions(-) diff --git a/changelog/unreleased/SOLR-13202.yml b/changelog/unreleased/SOLR-13202.yml new file mode 100644 index 00000000000..1d4ea052cec --- /dev/null +++ b/changelog/unreleased/SOLR-13202.yml @@ -0,0 +1,7 @@ +title: Return 400 instead of 500 for join queries, including cross-collection joins, that are missing the from or to parameter +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-13202 + url: https://issues.apache.org/jira/browse/SOLR-13202 diff --git a/solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java index b403c0c92a2..343210cde7f 100644 --- a/solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java @@ -146,6 +146,8 @@ public class JoinQParserPlugin extends QParserPlugin { final String v = qparser.localParams.get(QueryParsing.V); final String coreName; + ScoreJoinQParserPlugin.requireFromAndTo(fromField, toField); + Query fromQuery; long fromCoreOpenTime = 0; diff --git a/solr/core/src/java/org/apache/solr/search/join/AuxIndexJoinQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/join/AuxIndexJoinQParserPlugin.java index 9535dcf5226..0f7cfee6317 100644 --- a/solr/core/src/java/org/apache/solr/search/join/AuxIndexJoinQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/join/AuxIndexJoinQParserPlugin.java @@ -408,9 +408,7 @@ public class AuxIndexJoinQParserPlugin extends QParserPlugin } final String fromField = getParam("from"); final String toField = getParam("to"); - if (fromField == null || toField == null) { - throw new SyntaxError("auxIndexJoin query parser requires 'from' and 'to' local params"); - } + ScoreJoinQParserPlugin.requireFromAndTo(fromField, toField); final String fromIndex = localParams.get("fromIndex"); final String v = localParams.get(CommonParams.VALUE); final String myCore = req.getCore().getCoreDescriptor().getName(); diff --git a/solr/core/src/java/org/apache/solr/search/join/CrossCollectionJoinQParser.java b/solr/core/src/java/org/apache/solr/search/join/CrossCollectionJoinQParser.java index 28b08154f57..4581facf15d 100644 --- a/solr/core/src/java/org/apache/solr/search/join/CrossCollectionJoinQParser.java +++ b/solr/core/src/java/org/apache/solr/search/join/CrossCollectionJoinQParser.java @@ -106,6 +106,7 @@ public class CrossCollectionJoinQParser extends QParser { String collection = localParams.get(FROM_INDEX); String fromField = localParams.get(FROM); String toField = localParams.get(TO); + ScoreJoinQParserPlugin.requireFromAndTo(fromField, toField); boolean routedByJoinKey = localParams.getBool(ROUTED_BY_JOIN_KEY, toField.equals(routerField)); int ttl = localParams.getInt(TTL, TTL_DEFAULT); diff --git a/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java index 29b4a44f432..08beafdc190 100644 --- a/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java @@ -355,6 +355,18 @@ public class ScoreJoinQParserPlugin extends QParserPlugin { } } + /** Fails with a bad request if a join has no {@code from} or no {@code to} field. */ + public static void requireFromAndTo(String fromField, String toField) { + if (fromField == null) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, "Join query missing required 'from' parameter"); + } + if (toField == null) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, "Join query missing required 'to' parameter"); + } + } + @Override public QParser createParser( String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) { @@ -364,6 +376,7 @@ public class ScoreJoinQParserPlugin extends QParserPlugin { final String fromField = localParams.get("from"); final String fromIndex = localParams.get("fromIndex"); final String toField = localParams.get("to"); + requireFromAndTo(fromField, toField); final ScoreMode scoreMode = ScoreModeParser.parse(getParam(SCORE)); final String v = localParams.get(CommonParams.VALUE); diff --git a/solr/core/src/test/org/apache/solr/TestJoin.java b/solr/core/src/test/org/apache/solr/TestJoin.java index 14f776610ba..85e4b4f128b 100644 --- a/solr/core/src/test/org/apache/solr/TestJoin.java +++ b/solr/core/src/test/org/apache/solr/TestJoin.java @@ -490,4 +490,19 @@ public class TestJoin extends SolrTestCaseJ4 { return "{!join " + allProvidedParams + " method=topLevelDV}"; } } + + @Test + public void testJoinMissingFromToReturns400() { + for (String method : + List.of("", "method=dvWithScore score=none", "method=topLevelDV", "score=none")) { + assertMissingJoinParam("{!join to=id " + method + "}*:*", "from"); + assertMissingJoinParam("{!join from=id " + method + "}*:*", "to"); + } + } + + private void assertMissingJoinParam(String query, String missing) { + SolrException e = expectThrows(SolrException.class, () -> h.query(req("q", query, "fl", "id"))); + assertEquals(query, SolrException.ErrorCode.BAD_REQUEST.code, e.code()); + assertTrue(e.getMessage(), e.getMessage().contains("'" + missing + "'")); + } } diff --git a/solr/core/src/test/org/apache/solr/search/join/CrossCollectionJoinQueryTest.java b/solr/core/src/test/org/apache/solr/search/join/CrossCollectionJoinQueryTest.java index 341be4d6223..2b6e50693fd 100644 --- a/solr/core/src/test/org/apache/solr/search/join/CrossCollectionJoinQueryTest.java +++ b/solr/core/src/test/org/apache/solr/search/join/CrossCollectionJoinQueryTest.java @@ -30,6 +30,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.Replica; @@ -469,6 +470,28 @@ public class CrossCollectionJoinQueryTest extends SolrCloudTestCase { } } + @Test + public void testMissingFromOrToAtParser() throws Exception { + for (String missing : List.of(CrossCollectionJoinQParser.FROM, CrossCollectionJoinQParser.TO)) { + ModifiableSolrParams localParams = new ModifiableSolrParams(); + localParams.set(QueryParsing.V, "*:*"); + localParams.set(CrossCollectionJoinQParser.FROM_INDEX, "products"); + localParams.set(CrossCollectionJoinQParser.FROM, "product_id_s"); + localParams.set(CrossCollectionJoinQParser.TO, "product_id_s"); + localParams.remove(missing); + + ModifiableSolrParams requestParams = new ModifiableSolrParams(); + try (SolrQueryRequest req = new SolrQueryRequestBase(null, requestParams) {}) { + CrossCollectionJoinQParser parser = + new CrossCollectionJoinQParser( + null, localParams, requestParams, req, "product_id_s", null); + SolrException e = expectThrows(SolrException.class, parser::parse); + assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code()); + assertTrue(e.getMessage(), e.getMessage().contains("'" + missing + "'")); + } + } + } + @Test public void testShardsPreferenceRequestParamPropagation() throws Exception { // shards.preference set as a request-level param (not in localParams) should be diff --git a/solr/core/src/test/org/apache/solr/search/join/TestAuxIndexJoinQParserPlugin.java b/solr/core/src/test/org/apache/solr/search/join/TestAuxIndexJoinQParserPlugin.java index 4c1871f8d36..9685ed74bac 100644 --- a/solr/core/src/test/org/apache/solr/search/join/TestAuxIndexJoinQParserPlugin.java +++ b/solr/core/src/test/org/apache/solr/search/join/TestAuxIndexJoinQParserPlugin.java @@ -119,12 +119,15 @@ public class TestAuxIndexJoinQParserPlugin extends SolrTestCase { @Test public void testMissingLocalParams() { + assertMissingJoinParam("{!auxIndexJoin to=dept_id_s}*:*", "from"); + assertMissingJoinParam("{!auxIndexJoin from=dept_s}*:*", "to"); + } + + private void assertMissingJoinParam(String query, String missing) { SolrException e = - expectThrows( - SolrException.class, - () -> solrRule.getSolrClient().query(params("q", "{!auxIndexJoin to=dept_id_s}*:*"))); + expectThrows(SolrException.class, () -> solrRule.getSolrClient().query(params("q", query))); assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code()); - assertTrue(e.getMessage(), e.getMessage().contains("requires")); + assertTrue(e.getMessage(), e.getMessage().contains("missing required '" + missing + "'")); } @Test diff --git a/solr/core/src/test/org/apache/solr/search/join/TestGlobalOrdinalsJoinQParser.java b/solr/core/src/test/org/apache/solr/search/join/TestGlobalOrdinalsJoinQParser.java index 7cd72a52516..04eebca7f54 100644 --- a/solr/core/src/test/org/apache/solr/search/join/TestGlobalOrdinalsJoinQParser.java +++ b/solr/core/src/test/org/apache/solr/search/join/TestGlobalOrdinalsJoinQParser.java @@ -442,6 +442,19 @@ public class TestGlobalOrdinalsJoinQParser extends SolrTestCaseJ4 { assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, ex.code()); assertTrue(ex.getMessage().contains("which")); + // Missing from query (no query body and no 'v' local param) + ex = + expectThrows( + SolrException.class, + () -> { + h.query( + req( + "q", + "{!globalOrdinalsJoin joinField=join_s_dv which=\\\"type_s:parent\\\"}")); + }); + assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, ex.code()); + assertTrue(ex.getMessage().contains("from query is required")); + // Invalid score mode ex = expectThrows(
