Skip to content

Commit

Permalink
[SOLR-10059] Formalities check
Browse files Browse the repository at this point in the history
  • Loading branch information
tboeghk committed Feb 14, 2024
1 parent 94e90cb commit 99516e7
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@
import org.apache.solr.api.Api;
import org.apache.solr.api.ApiBag;
import org.apache.solr.api.ApiSupport;
import org.apache.solr.cluster.Shard;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ShardParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.List;
import java.util.Map;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.MultiMapSolrParams;
import org.apache.solr.common.params.SolrParams;
Expand Down Expand Up @@ -166,8 +167,9 @@ public static void processParams(
}

if (isShard) {
// sanitize all macros from parameters
newMap = MacroSanitizer.sanitize(newMap);
// sanitize all macros from fq parameters as they
// might corrupt handler local params
newMap = MacroSanitizer.sanitize(CommonParams.FQ, newMap);
} else { // Don't expand macros in shard requests
String[] doMacrosStr = newMap.get("expandMacros");
boolean doMacros = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,42 +1,55 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.solr.request.macro;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

import org.apache.solr.common.util.CollectionUtil;

public class MacroSanitizer {

public static Map<String, String[]> sanitize(Map<String, String[]> params) {
Map<String, String[]> sanitized = params;

// quick peek into the values to check for macros
final boolean needsSanitizing = params.values().stream()
.flatMap(v -> Arrays.stream(v))
.anyMatch(s -> s.contains(MacroExpander.MACRO_START));

if (needsSanitizing) {
sanitized = CollectionUtil.newHashMap(params.size());

for (Entry<String, String[]> param : params.entrySet()) {
final String paramName = param.getKey();
final String[] paramValues = param.getValue();
final List<String> sanitizedParamValues = new ArrayList<>(paramValues.length);

for (int i = 0; i < paramValues.length; i++) {
if (!paramValues[i].contains(MacroExpander.MACRO_START)) {
sanitizedParamValues.add(paramValues[i]);
}
}

sanitized.put(paramName, sanitizedParamValues.toArray(new String[] {}));
}
/**
* Sanitizes macros for the given parameter in the given params set if present
*
* @param param the parameter whose values we should sanitzize
* @param params the parameter set
* @return the sanitized parameter set
*/
public static Map<String, String[]> sanitize(String param, Map<String, String[]> params) {
// quick peek into the values to check for macros
final boolean needsSanitizing =
params.containsKey(param)
&& Arrays.stream(params.get(param))
.anyMatch(s -> s.contains(MacroExpander.MACRO_START));

if (needsSanitizing) {
final String[] fqs = params.get(param);
final List<String> sanitizedFqs = new ArrayList<>(fqs.length);

for (int i = 0; i < fqs.length; i++) {
if (!fqs[i].contains(MacroExpander.MACRO_START)) {
sanitizedFqs.add(fqs[i]);
}

return sanitized;
}

params.put(param, sanitizedFqs.toArray(new String[] {}));
}

return params;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,13 @@ public void test() throws Exception {
new ConfigRequest(
"{\n"
+ " 'add-requesthandler': {\n"
+ " 'name' : '" + handlerName + "',\n"
+ " 'name' : '"
+ handlerName
+ "',\n"
+ " 'class' : 'org.apache.solr.handler.component.SearchHandler',\n"
+ " 'appends' : { 'fq' : '{!collapse tag=collapsing field=" + bee_si + " sort=\"${collapseSort}\" }' }, \n"
+ " 'appends' : { 'fq' : '{!collapse tag=collapsing field="
+ bee_si
+ " sort=\"${collapseSort}\" }' }, \n"
+ " }\n"
+ "}"),
COLLECTION);
Expand All @@ -88,8 +92,6 @@ public void test() throws Exception {
.commit(cluster.getSolrClient(), COLLECTION);
}



// compose the query
final SolrQuery solrQuery = new SolrQuery(bee_si + ":bee");
solrQuery.setParam(CommonParams.QT, handlerName);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,42 +1,62 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.solr.request.macro;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame;

import java.util.HashMap;
import java.util.Map;

import org.apache.solr.common.params.CommonParams;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.Test;

public class MacroSanitizerTest {

@Test
public void shouldReturnSameInstanceWhenNotSanitizing() {
// given
Map<String, String[]> params = new HashMap<>();

// when
Map<String, String[]> sanitized = MacroSanitizer.sanitize(params);

// then
assertSame(params, sanitized);
}

@Test
public void shouldNotSanitizeNonMacros() {
// given
Map<String, String[]> params = new HashMap<>();
params.put("fq", new String[] { "bee:up", "look:left", "{!collapse tag=collapsing field=bee sort=${collapseSort}}" });
params.put("q", new String[] { "bee:honey" });

// when
Map<String, String[]> sanitized = MacroSanitizer.sanitize(params);

// then
assertEquals(2, sanitized.size());
assertEquals(2, sanitized.get("fq").length);
MatcherAssert.assertThat(sanitized.get("fq"), Matchers.arrayContaining("bee:up", "look:left"));
}
@Test
public void shouldReturnSameInstanceWhenNotSanitizing() {
// given
Map<String, String[]> params = new HashMap<>();

// when
Map<String, String[]> sanitized = MacroSanitizer.sanitize(CommonParams.FQ, params);

// then
assertSame(params, sanitized);
}

@Test
public void shouldNotSanitizeNonMacros() {
// given
Map<String, String[]> params = new HashMap<>();
params.put(
CommonParams.FQ,
new String[] {
"bee:up", "look:left", "{!collapse tag=collapsing field=bee sort=${collapseSort}}"
});
params.put("q", new String[] {"bee:honey"});

// when
Map<String, String[]> sanitized = MacroSanitizer.sanitize(CommonParams.FQ, params);

// then
assertEquals(2, sanitized.size());
assertEquals(2, sanitized.get("fq").length);
MatcherAssert.assertThat(sanitized.get("fq"), Matchers.arrayContaining("bee:up", "look:left"));
}
}

0 comments on commit 99516e7

Please sign in to comment.