Apache
/
lucene
mirror of https://github.com/apache/lucene.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SOLR-13824: reject prematurely closed curly bracket in JSON.

This commit is contained in:
Mikhail Khludnev 2019-10-13 23:03:44 +03:00
parent cfa4940167
commit afdb80069c
17 changed files with 145 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
solr/CHANGES.txt
View File

@ -126,6 +126,7 @@ Other Changes
* SOLR-13841: Add jackson databind annotations to SolrJ classpath (noble)
* SOLR-13824: Strictly reject anything after JSON in most APIs (Mikhail Khludnev, Munendra S N)
================== 8.3.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

2
solr/contrib/ltr/src/java/org/apache/solr/ltr/model/DefaultWrapperModel.java
View File

@ -94,7 +94,7 @@ public class DefaultWrapperModel extends WrapperModel {
@SuppressWarnings("unchecked")
protected Map<String, Object> parseInputStream(InputStream in) throws IOException {
try (Reader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) {
return (Map<String, Object>) new ObjectBuilder(new JSONParser(reader)).getVal();
return (Map<String, Object>) new ObjectBuilder(new JSONParser(reader)).getValStrict();
}
}

4
solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java
View File

@ -115,7 +115,7 @@ public class DistribPackageStore implements PackageStore {
if (!parent.exists()) {
parent.mkdirs();
}
Map m = (Map) Utils.fromJSON(meta.array());
Map m = (Map) Utils.fromJSON(meta.array(), meta.arrayOffset(), meta.limit());
if (m == null || m.isEmpty()) {
throw new SolrException(SERVER_ERROR, "invalid metadata , discarding : " + path);
}
@ -187,7 +187,7 @@ public class DistribPackageStore implements PackageStore {
metadata = Utils.executeGET(coreContainer.getUpdateShardHandler().getDefaultHttpClient(),
baseUrl + "/node/files" + getMetaPath(),
Utils.newBytesConsumer((int) MAX_PKG_SIZE));
m = (Map) Utils.fromJSON(metadata.array());
m = (Map) Utils.fromJSON(metadata.array(), metadata.arrayOffset(), metadata.limit());
} catch (SolrException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error fetching metadata", e);
}

2
solr/core/src/java/org/apache/solr/request/json/RequestUtil.java
View File

@ -264,7 +264,7 @@ public class RequestUtil {
List<String> path = StrUtils.splitSmart(queryParamName, ".", true);
path = path.subList(1, path.size());
for (String jsonStr : vals) {
Object o = ObjectBuilder.fromJSON(jsonStr);
Object o = ObjectBuilder.fromJSONStrict(jsonStr);
// zero-length strings or comments can cause this to be null (and a zero-length string can result from a json content-type w/o a body)
if (o != null) {
ObjectUtil.mergeObjects(json, path, o, handler);

2
solr/core/src/java/org/apache/solr/schema/JsonPreAnalyzedParser.java
View File

@ -81,7 +81,7 @@ public class JsonPreAnalyzedParser implements PreAnalyzedParser {
if (val.length() == 0) {
return res;
}
Object o = ObjectBuilder.fromJSON(val);
Object o = ObjectBuilder.fromJSONStrict(val);
if (!(o instanceof Map)) {
throw new IOException("Invalid JSON type " + o.getClass().getName() + ", expected Map");
}

2
solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoAddReplicasIntegrationTest.java
View File

@ -70,7 +70,7 @@ public class AutoAddReplicasIntegrationTest extends SolrCloudTestCase {
new V2Request.Builder("/cluster")
.withMethod(SolrRequest.METHOD.POST)
.withPayload("{set-obj-property:{defaults : {cluster: {useLegacyReplicaAssignment:true}}}}}")
.withPayload("{set-obj-property:{defaults : {cluster: {useLegacyReplicaAssignment:true}}}}")
.build()
.process(cluster.getSolrClient());
}

2
solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoAddReplicasPlanActionTest.java
View File

@ -65,7 +65,7 @@ public class AutoAddReplicasPlanActionTest extends SolrCloudTestCase{
new V2Request.Builder("/cluster")
.withMethod(SolrRequest.METHOD.POST)
.withPayload("{set-obj-property:{defaults : {cluster: {useLegacyReplicaAssignment:true}}}}}")
.withPayload("{set-obj-property:{defaults : {cluster: {useLegacyReplicaAssignment:true}}}}")
.build()
.process(cluster.getSolrClient());
}

2
solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
View File

@ -139,7 +139,7 @@ public class TestSolrConfigHandler extends RestTestBase {
assertEquals("10", m._getStr("config/updateHandler/autoCommit/maxTime",null));
assertEquals("true", m._getStr("config/requestDispatcher/requestParsers/addHttpRequestToContext",null));
payload = "{\n" +
" 'unset-property' : 'updateHandler.autoCommit.maxDocs'} \n" +
" 'unset-property' : 'updateHandler.autoCommit.maxDocs' \n" +
" }";
runConfigCommand(harness, "/config", payload);

7
solr/core/src/test/org/apache/solr/filestore/TestDistribPackageStore.java
View File

@ -17,6 +17,9 @@
package org.apache.solr.filestore;
import static org.apache.solr.common.util.Utils.JAVABINCONSUMER;
import static org.apache.solr.core.TestDynamicLoading.getFileContent;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
@ -27,7 +30,6 @@ import java.util.Set;
import java.util.concurrent.Callable;
import java.util.function.Predicate;
import com.google.common.collect.ImmutableSet;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
@ -48,8 +50,7 @@ import org.apache.solr.util.LogLevel;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.server.ByteBufferInputStream;
import static org.apache.solr.common.util.Utils.JAVABINCONSUMER;
import static org.apache.solr.core.TestDynamicLoading.getFileContent;
import com.google.common.collect.ImmutableSet;
@LogLevel("org.apache.solr.filestore.PackageStoreAPI=DEBUG;org.apache.solr.filestore.DistribPackageStore=DEBUG")
public class TestDistribPackageStore extends SolrCloudTestCase {

4
solr/core/src/test/org/apache/solr/search/facet/TestJsonFacetRefinement.java
View File

@ -277,7 +277,7 @@ public class TestJsonFacetRefinement extends SolrTestCaseHS {
" between:{ x:{_l : [x1]} }" +
"} } ");
// a range face w/o any sub facets shouldn't require any refinement
doTestRefine("{top:{type:range, other:all, field:R, start:0, end:3, gap:2 } }" +
doTestRefine("{top:{type:range, other:all, field:R, start:0, end:3, gap:2 } }" ,
// phase #1
"{top: {buckets:[{val:0, count:2}, {val:2, count:2}]," +
" before:{count:3},after:{count:47}," +
@ -314,7 +314,7 @@ public class TestJsonFacetRefinement extends SolrTestCaseHS {
// a range facet w/o any sub facets shouldn't require any refinement
// other and include ignored for ranges
doTestRefine("{top:{type:range, other:all, field:R, ranges:[{from:0, to:2},{from:2, to:3}] } }" +
doTestRefine("{top:{type:range, other:all, field:R, ranges:[{from:0, to:2},{from:2, to:3}] } }",
// phase #1
"{top: {buckets:[{val:\"[0,2)\", count:2}, {val:\"[2,3)\", count:2}]," +
" } }",

9
solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
View File

@ -201,7 +201,8 @@ public class TestJsonFacets extends SolrTestCaseHS {
// straight query facets
client.testJQ(params(p, "q", "*:*", "rows","0", "fq","+${make_s}:honda +cost_f:[${price_low} TO ${price_high}]"
, "json.facet", "{makes:{terms:{field:${make_s}, facet:{models:{terms:{field:${model_s}, limit:2, mincount:0}}}}}}}"
, "json.facet", "{makes:{terms:{field:${make_s}, facet:{models:{terms:{field:${model_s}, limit:2, mincount:0}}}}"
+ "}}"
, "facet","true", "facet.pivot","make_s,model_s", "facet.limit", "2"
)
, "facets=={count:" + nHonda + ", makes:{buckets:[{val:honda, count:" + nHonda + ", models:{buckets:["
@ -1405,7 +1406,7 @@ public class TestJsonFacets extends SolrTestCaseHS {
// terms facet with nested query facet
client.testJQ(params(p, "q", "*:*"
, "json.facet", "{cat:{terms:{${terms} field:'${cat_s}', facet:{nj:{query:'${where_s}:NJ'}} } }} }"
, "json.facet", "{cat:{terms:{${terms} field:'${cat_s}', facet:{nj:{query:'${where_s}:NJ'}} } }}"
)
, "facets=={ 'count':6, " +
"'cat':{ 'buckets':[{ 'val':'B', 'count':3, 'nj':{ 'count':2}}, { 'val':'A', 'count':2, 'nj':{ 'count':1}}]} }"
@ -1413,7 +1414,7 @@ public class TestJsonFacets extends SolrTestCaseHS {
// terms facet with nested query facet on subset
client.testJQ(params(p, "q", "id:(2 5 4)"
, "json.facet", "{cat:{terms:{${terms} field:'${cat_s}', facet:{nj:{query:'${where_s}:NJ'}} } }} }"
, "json.facet", "{cat:{terms:{${terms} field:'${cat_s}', facet:{nj:{query:'${where_s}:NJ'}} } }}"
)
, "facets=={ 'count':3, " +
"'cat':{ 'buckets':[{ 'val':'B', 'count':2, 'nj':{ 'count':2}}, { 'val':'A', 'count':1, 'nj':{ 'count':1}}]} }"
@ -1753,7 +1754,7 @@ public class TestJsonFacets extends SolrTestCaseHS {
//
client.testJQ(params(p, "q", "*:*"
, "json.facet", "{cat:{terms:{${terms} field:'${multi_ss}', facet:{nj:{query:'${where_s}:NJ'}} } }} }"
, "json.facet", "{cat:{terms:{${terms} field:'${multi_ss}', facet:{nj:{query:'${where_s}:NJ'}} } }}"
)
, "facets=={ 'count':6, " +
"'cat':{ 'buckets':[{ 'val':'a', 'count':3, 'nj':{ 'count':2}}, { 'val':'b', 'count':3, 'nj':{ 'count':2}}]} }"

5
solr/solrj/src/java/org/apache/solr/common/util/CommandOperation.java
View File

@ -280,7 +280,10 @@ public class CommandOperation {
List<CommandOperation> operations = new ArrayList<>();
for (; ; ) {
int ev = parser.nextEvent();
if (ev == JSONParser.OBJECT_END) return operations;
if (ev == JSONParser.OBJECT_END) {
ObjectBuilder.checkEOF(parser);
return operations;
}
Object key = ob.getKey();
ev = parser.nextEvent();
Object val = ob.getVal();

14
solr/solrj/src/java/org/apache/solr/common/util/Utils.java
View File

@ -246,17 +246,21 @@ public class Utils {
}
public static Object fromJSON(byte[] utf8) {
return fromJSON(utf8, 0, utf8.length);
}
public static Object fromJSON(byte[] utf8, int offset, int length) {
// convert directly from bytes to chars
// and parse directly from that instead of going through
// intermediate strings or readers
CharArr chars = new CharArr();
ByteUtils.UTF8toUTF16(utf8, 0, utf8.length, chars);
ByteUtils.UTF8toUTF16(utf8, offset, length, chars);
JSONParser parser = new JSONParser(chars.getArray(), chars.getStart(), chars.length());
parser.setFlags(parser.getFlags() |
JSONParser.ALLOW_MISSING_COLON_COMMA_BEFORE_OBJECT |
JSONParser.OPTIONAL_OUTER_BRACES);
try {
return STANDARDOBJBUILDER.apply(parser).getVal(parser);
return STANDARDOBJBUILDER.apply(parser).getValStrict();
} catch (IOException e) {
throw new RuntimeException(e); // should never happen w/o using real IO
}
@ -285,7 +289,7 @@ public class Utils {
public static Object fromJSON(Reader is) {
try {
return STANDARDOBJBUILDER.apply(getJSONParser(is)).getVal();
return STANDARDOBJBUILDER.apply(getJSONParser(is)).getValStrict();
} catch (IOException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parse error", e);
}
@ -334,7 +338,7 @@ public class Utils {
public static Object fromJSON(InputStream is, Function<JSONParser, ObjectBuilder> objBuilderProvider) {
try {
return objBuilderProvider.apply(getJSONParser((new InputStreamReader(is, StandardCharsets.UTF_8)))).getVal();
return objBuilderProvider.apply(getJSONParser((new InputStreamReader(is, StandardCharsets.UTF_8)))).getValStrict();
} catch (IOException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parse error", e);
}
@ -363,7 +367,7 @@ public class Utils {
public static Object fromJSONString(String json) {
try {
return STANDARDOBJBUILDER.apply(getJSONParser(new StringReader(json))).getVal();
return STANDARDOBJBUILDER.apply(getJSONParser(new StringReader(json))).getValStrict();
} catch (Exception e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parse error : " + json, e);
}

43
solr/solrj/src/java/org/noggit/ObjectBuilder.java
View File

@ -20,21 +20,62 @@
package org.noggit;
import java.util.*;
import java.io.IOException;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import org.noggit.JSONParser.ParseException;
public class ObjectBuilder {
/** consider to use {@link #fromJSONStrict(String)}*/
public static Object fromJSON(String json) throws IOException {
JSONParser p = new JSONParser(json);
return getVal(p);
}
/** like {@link #fromJSON(String)}, but also check that there is nothing
* remaining in the given string after closing bracket.
* Throws {@link ParseException} otherwise.*/
public static Object fromJSONStrict(String json) throws IOException {
JSONParser p = new JSONParser(json);
final Object val = getVal(p);
checkEOF(p);
return val;
}
public static void checkEOF(JSONParser p) throws IOException {
if (p.nextEvent()!=JSONParser.EOF) {
throw p.err("Expecting single object only.");
}
}
/** consider to use {@link #getValStrict()}*/
public static Object getVal(JSONParser parser) throws IOException {
return new ObjectBuilder(parser).getVal();
}
/** like {@link #getVal()}, but also check that there is nothing
* remaining in the given stream after closing bracket.
* Throws {@link ParseException} otherwise.*/
public static Object getValStrict(JSONParser parser) throws IOException {
final Object val = getVal(parser);
checkEOF(parser);
return val;
}
/** like {@link #getVal()}, but also check that there is nothing
* remaining in the given stream after closing bracket.
* Throws {@link ParseException} otherwise.*/
public Object getValStrict() throws IOException {
final Object val = getVal();
checkEOF(parser);
return val;
}
final JSONParser parser;

39
solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
View File

@ -18,6 +18,16 @@
package org.apache.solr.client.solrj.cloud.autoscaling;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.solr.client.solrj.cloud.autoscaling.Policy.CLUSTER_PREFERENCES;
import static org.apache.solr.client.solrj.cloud.autoscaling.TestPolicy2.loadFromResource;
import static org.apache.solr.client.solrj.cloud.autoscaling.Variable.Type.CORES;
import static org.apache.solr.client.solrj.cloud.autoscaling.Variable.Type.FREEDISK;
import static org.apache.solr.client.solrj.cloud.autoscaling.Variable.Type.REPLICA;
import static org.apache.solr.common.cloud.ZkStateReader.CLUSTER_STATE;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.ADDREPLICA;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.MOVEREPLICA;
import java.io.IOException;
import java.io.StringWriter;
import java.lang.invoke.MethodHandles;
@ -35,8 +45,6 @@ import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
@ -75,15 +83,8 @@ import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.solr.client.solrj.cloud.autoscaling.Policy.CLUSTER_PREFERENCES;
import static org.apache.solr.client.solrj.cloud.autoscaling.TestPolicy2.loadFromResource;
import static org.apache.solr.client.solrj.cloud.autoscaling.Variable.Type.CORES;
import static org.apache.solr.client.solrj.cloud.autoscaling.Variable.Type.FREEDISK;
import static org.apache.solr.client.solrj.cloud.autoscaling.Variable.Type.REPLICA;
import static org.apache.solr.common.cloud.ZkStateReader.CLUSTER_STATE;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.ADDREPLICA;
import static org.apache.solr.common.params.CollectionParams.CollectionAction.MOVEREPLICA;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
public class TestPolicy extends SolrTestCaseJ4 {
boolean useNodeset ;
@ -1843,7 +1844,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
" 'freedisk':918005641216}," +
" '127.0.0.1:60089_solr':{" +
" 'cores':2," +
" 'freedisk':918005641216}}}");
" 'freedisk':918005641216}}");
Policy policy = new Policy((Map<String, Object>) Utils.fromJSONString(autoscaleJson));
Policy.Session session = policy.createSession(new DelegatingCloudManager(null) {
@ -2780,11 +2781,15 @@ public class TestPolicy extends SolrTestCaseJ4 {
StringWriter writer = new StringWriter();
NamedList<Object> val = new NamedList<>();
val.add("violations", violations);
new SolrJSONWriter(writer)
.writeObj(val)
.close();
JSONWriter.write(writer, true, JsonTextWriter.JSON_NL_MAP, val);
if (random().nextBoolean()) {
new SolrJSONWriter(writer)
.writeObj(val)
.close();
} else {
JSONWriter.write(writer, true, JsonTextWriter.JSON_NL_MAP, val);
}
Object root = Utils.fromJSONString(writer.toString());
assertEquals(2l,
Utils.getObjectByPath(root, true, "violations[0]/violation/replica/NRT"));

18
solr/solrj/src/test/org/apache/solr/client/solrj/request/TestV2Request.java
View File

@ -24,6 +24,7 @@ import java.util.List;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.BaseHttpSolrClient;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.response.V2Response;
import org.apache.solr.cloud.SolrCloudTestCase;
@ -104,7 +105,7 @@ public class TestV2Request extends SolrCloudTestCase {
" 'replicationFactor' : 2," +
" 'config' : 'config'" +
" }" +
"}").build());
"}" + "/* ignore comment*/").build());
assertSuccess(client, new V2Request.Builder("/c").build());
assertSuccess(client, new V2Request.Builder("/c/_introspect").build());
@ -122,7 +123,20 @@ public class TestV2Request extends SolrCloudTestCase {
// TODO: this is not guaranteed now - beast test if you try to fix
// assertFalse( collections.contains("test"));
try{
NamedList<Object> res1 = client.request(new V2Request.Builder("/collections")
.withMethod(SolrRequest.METHOD.POST)
.withPayload("{" +
" 'create' : {" +
" 'name' : 'jsontailtest'," +
" 'numShards' : 2," +
" 'replicationFactor' : 2," +
" 'config' : 'config'" +
" }" +
"}" + ", 'something':'bogus'").build());
assertFalse("The request failed", res1.get("responseHeader").toString().contains("status=0"));
}catch(BaseHttpSolrClient.RemoteExecutionException itsOk) {
}
}
public void testV2Forwarding() throws Exception {

33
solr/solrj/src/test/org/noggit/TestObjectBuilder.java
View File

@ -25,12 +25,15 @@ import java.util.Map;
import org.apache.solr.SolrTestCaseJ4;
import org.junit.Test;
import org.noggit.JSONParser.ParseException;
@SolrTestCaseJ4.SuppressSSL
public class TestObjectBuilder extends SolrTestCaseJ4 {
public void _test(String val, Object expected) throws IOException {
val = val.replace('\'','"');
Object v = ObjectBuilder.fromJSON(val);
Object v = random().nextBoolean() ? ObjectBuilder.fromJSON(val) :
ObjectBuilder.fromJSONStrict(val) ;
String s1 = JSONUtil.toJSON(v,-1);
String s2 = JSONUtil.toJSON(expected,-1);
@ -96,4 +99,32 @@ public class TestObjectBuilder extends SolrTestCaseJ4 {
_testVariations("[false,true,false]", new boolean[]{false,true,false});
}
@Test
public void testStrictPositive() throws IOException {
assertEquals(O("foo","bar", "ban", "buzz"),
ObjectBuilder.fromJSONStrict("{\"foo\":\"bar\", \"ban\":\"buzz\"}"));
assertEquals(O("foo","bar" ),
ObjectBuilder.fromJSONStrict("{\"foo\":\"bar\"/*, \"ban\":\"buzz\"*/}"));
assertEquals(O("foo","bar" ),
ObjectBuilder.fromJSONStrict("{\"foo\":\"bar\"} /*\"ban\":\"buzz\"*/"));
assertEquals("foo",
ObjectBuilder.fromJSONStrict("\"foo\""));
assertEquals("fromJSON() ignores tail.",O("foo","bar" ),
ObjectBuilder.fromJSON("{\"foo\":\"bar\"} \"ban\":\"buzz\"}"));
assertEquals("old method ignores tails", "foo",
ObjectBuilder.fromJSON("\"foo\" \"baar\" "));
expectThrows(ParseException.class,
() -> ObjectBuilder.fromJSONStrict("\"foo\" \"bar\""));
expectThrows(ParseException.class,
() -> ObjectBuilder.fromJSONStrict("{\"foo\":\"bar\"} \"ban\":\"buzz\"}"));
expectThrows(ParseException.class,
() -> ObjectBuilder.getValStrict(new JSONParser("{\"foo\":\"bar\"} \"ban\":\"buzz\"}")));
expectThrows(ParseException.class,
() -> new ObjectBuilder(new JSONParser("{\"foo\":\"bar\"} \"ban\":\"buzz\"}")).getValStrict());
}
}