From 07a89e7bf627b08b4cef323dd4d187da92b3a411 Mon Sep 17 00:00:00 2001 From: Munendra S N Date: Fri, 31 Jul 2020 20:05:41 +0530 Subject: [PATCH] SOLR-14516: fix NPE is resp writer while writing docvalue only field This issue occurs only while fetching uncommitted doc through /get. Instead of directly calling stringValue() on IndexableField use FieldType's toExtern() or toObject() to get the writable value for the field. --- solr/CHANGES.txt | 3 +++ .../src/java/org/apache/solr/schema/BoolField.java | 2 +- .../src/java/org/apache/solr/schema/FieldType.java | 3 +++ .../org/apache/solr/schema/PreAnalyzedField.java | 2 +- .../src/java/org/apache/solr/schema/StrField.java | 2 +- .../src/java/org/apache/solr/schema/TextField.java | 2 +- .../src/java/org/apache/solr/schema/UUIDField.java | 4 ++-- .../solr/collection1/conf/schema_latest.xml | 4 ++++ .../test/org/apache/solr/search/TestRealTimeGet.java | 12 ++++++++++-- .../org/apache/solr/common/util/JsonTextWriter.java | 5 ----- 10 files changed, 26 insertions(+), 13 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index d00b741ff6a..b46ccfea886 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -153,6 +153,9 @@ Bug Fixes * SOLR-11656: TLOG replication doesn't work properly after rebalancing leaders. (Yuki Yano via Erick Erickson) +* SOLR-14516: Fix NPE in JSON response writer(wt=json) with /get when writing non-stored, non-indexed docvalue field + from an uncommitted document (noble, Ishan Chattopadhyaya, Munendra S N) + Other Changes --------------------- diff --git a/solr/core/src/java/org/apache/solr/schema/BoolField.java b/solr/core/src/java/org/apache/solr/schema/BoolField.java index 83b4395e020..3a4b49a7c0c 100644 --- a/solr/core/src/java/org/apache/solr/schema/BoolField.java +++ b/solr/core/src/java/org/apache/solr/schema/BoolField.java @@ -169,7 +169,7 @@ public class BoolField extends PrimitiveFieldType { @Override public void write(TextResponseWriter writer, String name, IndexableField f) throws IOException { - writer.writeBool(name, f.stringValue().charAt(0) == 'T'); + writer.writeBool(name, toObject(f)); } @Override diff --git a/solr/core/src/java/org/apache/solr/schema/FieldType.java b/solr/core/src/java/org/apache/solr/schema/FieldType.java index ee6dfdb9b16..daac8c04bad 100644 --- a/solr/core/src/java/org/apache/solr/schema/FieldType.java +++ b/solr/core/src/java/org/apache/solr/schema/FieldType.java @@ -677,6 +677,9 @@ public abstract class FieldType extends FieldProperties { /** * calls back to TextResponseWriter to write the field value + *

+ * Sub-classes should prefer using {@link #toExternal(IndexableField)} or {@link #toObject(IndexableField)} + * to get the writeable external value of f instead of directly using f.stringValue() or f.binaryValue() */ public abstract void write(TextResponseWriter writer, String name, IndexableField f) throws IOException; diff --git a/solr/core/src/java/org/apache/solr/schema/PreAnalyzedField.java b/solr/core/src/java/org/apache/solr/schema/PreAnalyzedField.java index 84265e2d97a..87a874b5b1d 100644 --- a/solr/core/src/java/org/apache/solr/schema/PreAnalyzedField.java +++ b/solr/core/src/java/org/apache/solr/schema/PreAnalyzedField.java @@ -149,7 +149,7 @@ public class PreAnalyzedField extends TextField implements HasImplicitIndexAnaly @Override public void write(TextResponseWriter writer, String name, IndexableField f) throws IOException { - writer.writeStr(name, f.stringValue(), true); + writer.writeStr(name, toExternal(f), true); } /** Utility method to convert a field to a string that is parse-able by this diff --git a/solr/core/src/java/org/apache/solr/schema/StrField.java b/solr/core/src/java/org/apache/solr/schema/StrField.java index 3413ce18a57..bd7fa06c9cf 100644 --- a/solr/core/src/java/org/apache/solr/schema/StrField.java +++ b/solr/core/src/java/org/apache/solr/schema/StrField.java @@ -98,7 +98,7 @@ public class StrField extends PrimitiveFieldType { @Override public void write(TextResponseWriter writer, String name, IndexableField f) throws IOException { - writer.writeStr(name, f.stringValue(), true); + writer.writeStr(name, toExternal(f), true); } @Override diff --git a/solr/core/src/java/org/apache/solr/schema/TextField.java b/solr/core/src/java/org/apache/solr/schema/TextField.java index bddaf00c760..2e44e6741a5 100644 --- a/solr/core/src/java/org/apache/solr/schema/TextField.java +++ b/solr/core/src/java/org/apache/solr/schema/TextField.java @@ -139,7 +139,7 @@ public class TextField extends FieldType { @Override public void write(TextResponseWriter writer, String name, IndexableField f) throws IOException { - writer.writeStr(name, f.stringValue(), true); + writer.writeStr(name, toExternal(f), true); } @Override diff --git a/solr/core/src/java/org/apache/solr/schema/UUIDField.java b/solr/core/src/java/org/apache/solr/schema/UUIDField.java index 00495c97755..aa04b0dd21d 100644 --- a/solr/core/src/java/org/apache/solr/schema/UUIDField.java +++ b/solr/core/src/java/org/apache/solr/schema/UUIDField.java @@ -65,7 +65,7 @@ public class UUIDField extends StrField { @Override public void write(TextResponseWriter writer, String name, IndexableField f) throws IOException { - writer.writeStr(name, f.stringValue(), false); + writer.writeStr(name, toExternal(f), false); } /** @@ -99,7 +99,7 @@ public class UUIDField extends StrField { @Override public UUID toObject(IndexableField f) { - return UUID.fromString(f.stringValue()); + return UUID.fromString(toExternal(f)); } @Override diff --git a/solr/core/src/test-files/solr/collection1/conf/schema_latest.xml b/solr/core/src/test-files/solr/collection1/conf/schema_latest.xml index af445bc67ed..18cfedc780a 100644 --- a/solr/core/src/test-files/solr/collection1/conf/schema_latest.xml +++ b/solr/core/src/test-files/solr/collection1/conf/schema_latest.xml @@ -260,6 +260,10 @@ + + + + diff --git a/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java b/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java index 30cb450e176..b0f684ec447 100644 --- a/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java +++ b/solr/core/src/test/org/apache/solr/search/TestRealTimeGet.java @@ -56,17 +56,25 @@ public class TestRealTimeGet extends TestRTGBase { "a_f","-1.5", "a_fd","-1.5", "a_fdS","-1.5", "a_fs","1.0","a_fs","2.5", "a_fds","1.0","a_fds","2.5", "a_fdsS","1.0","a_fdsS","2.5", "a_d","-1.2E99", "a_dd","-1.2E99", "a_ddS","-1.2E99", "a_ds","1.0","a_ds","2.5", "a_dds","1.0","a_dds","2.5", "a_ddsS","1.0","a_ddsS","2.5", "a_i","-1", "a_id","-1", "a_idS","-1", "a_is","1","a_is","2", "a_ids","1","a_ids","2", "a_idsS","1","a_idsS","2", - "a_l","-9999999999", "a_ld","-9999999999", "a_ldS","-9999999999", "a_ls","1","a_ls","9999999999", "a_lds","1","a_lds","9999999999", "a_ldsS","1","a_ldsS","9999999999" + "a_l","-9999999999", "a_ld","-9999999999", "a_ldS","-9999999999", "a_ls","1","a_ls","9999999999", "a_lds","1","a_lds","9999999999", "a_ldsS","1","a_ldsS","9999999999", + "a_s", "abc", "a_sd", "bcd", "a_sdS", "cde", "a_ss","def","a_ss", "efg", "a_sds","fgh","a_sds","ghi", "a_sdsS","hij","a_sdsS","ijk", + "a_b", "false", "a_bd", "true", "a_bdS", "false", "a_bs","true","a_bs", "false", "a_bds","true","a_bds","false", "a_bdsS","true","a_bdsS","false" )); assertJQ(req("q","id:1") ,"/response/numFound==0" ); - assertJQ(req("qt","/get", "id","1", "fl","id, a_f,a_fd,a_fdS a_fs,a_fds,a_fdsS, a_d,a_dd,a_ddS, a_ds,a_dds,a_ddsS, a_i,a_id,a_idS a_is,a_ids,a_idsS, a_l,a_ld,a_ldS a_ls,a_lds,a_ldsS") + assertJQ(req("qt","/get", "id","1", "fl","id, a_f,a_fd,a_fdS a_fs,a_fds,a_fdsS, " + + "a_d,a_dd,a_ddS, a_ds,a_dds,a_ddsS, a_i,a_id,a_idS a_is,a_ids,a_idsS, " + + "a_l,a_ld,a_ldS, a_ls,a_lds,a_ldsS, a_s,a_sd,a_sdS a_ss,a_sds,a_sdsS, " + + "a_b,a_bd,a_bdS, a_bs,a_bds,a_bdsS" + ) ,"=={'doc':{'id':'1'" + ", a_f:-1.5, a_fd:-1.5, a_fdS:-1.5, a_fs:[1.0,2.5], a_fds:[1.0,2.5],a_fdsS:[1.0,2.5]" + ", a_d:-1.2E99, a_dd:-1.2E99, a_ddS:-1.2E99, a_ds:[1.0,2.5],a_dds:[1.0,2.5],a_ddsS:[1.0,2.5]" + ", a_i:-1, a_id:-1, a_idS:-1, a_is:[1,2],a_ids:[1,2],a_idsS:[1,2]" + ", a_l:-9999999999, a_ld:-9999999999, a_ldS:-9999999999, a_ls:[1,9999999999],a_lds:[1,9999999999],a_ldsS:[1,9999999999]" + + ", a_s:'abc', a_sd:'bcd', a_sdS:'cde', a_ss:['def','efg'],a_sds:['fgh','ghi'],a_sdsS:['hij','ijk']" + + ", a_b:false, a_bd:true, a_bdS:false, a_bs:[true,false],a_bds:[true,false],a_bdsS:[true,false]" + " }}" ); assertJQ(req("qt","/get","ids","1", "fl","id") diff --git a/solr/solrj/src/java/org/apache/solr/common/util/JsonTextWriter.java b/solr/solrj/src/java/org/apache/solr/common/util/JsonTextWriter.java index 8a5c25697d9..ec472ad79eb 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/JsonTextWriter.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/JsonTextWriter.java @@ -67,11 +67,6 @@ public interface JsonTextWriter extends TextWriter { } default void writeStr(String name, String val, boolean needsEscaping) throws IOException { - if (val == null) { - writeNull(name); - return; - } - // it might be more efficient to use a stringbuilder or write substrings // if writing chars to the stream is slow. if (needsEscaping) {