This is an automated email from the ASF dual-hosted git repository.
penghui pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new ee5b558 Fix NPE caused by null value of SchemaInfo's properties
(#9985)
ee5b558 is described below
commit ee5b558f9a1147c1dbaa042ee986297885e5fe66
Author: Yunze Xu <[email protected]>
AuthorDate: Mon Mar 22 11:29:42 2021 +0800
Fix NPE caused by null value of SchemaInfo's properties (#9985)
Fixes #9964
### Motivation
If there exists a null value of `SchemaInfo`'s properties, NPE will be
thrown in `SchemaInfo#toString` or `Commands#newSubscribe`.
### Modifications
- Add null checks before `JsonPrimitive`'s constructor, `KeyValue#setKey`
and `KeyValue#setValue`.
- Add related tests.
---
.../java/org/apache/pulsar/schema/SchemaTest.java | 32 ++++++++++++++++++++++
.../pulsar/client/impl/schema/SchemaUtils.java | 2 +-
.../pulsar/client/impl/schema/SchemaInfoTest.java | 10 +++++++
.../apache/pulsar/common/protocol/Commands.java | 8 ++++--
4 files changed, 48 insertions(+), 4 deletions(-)
diff --git
a/pulsar-broker/src/test/java/org/apache/pulsar/schema/SchemaTest.java
b/pulsar-broker/src/test/java/org/apache/pulsar/schema/SchemaTest.java
index f6b2947..087223c 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/schema/SchemaTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/schema/SchemaTest.java
@@ -27,12 +27,17 @@ import static org.testng.Assert.assertTrue;
import com.google.common.collect.Sets;
import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
import lombok.extern.slf4j.Slf4j;
import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
import org.apache.pulsar.broker.service.schema.SchemaRegistryServiceImpl;
+import org.apache.pulsar.client.admin.PulsarAdminException;
import org.apache.pulsar.client.api.Consumer;
import org.apache.pulsar.client.api.Message;
import org.apache.pulsar.client.api.Producer;
+import org.apache.pulsar.client.api.PulsarClientException;
import org.apache.pulsar.client.api.Schema;
import org.apache.pulsar.client.api.schema.GenericRecord;
import org.apache.pulsar.client.api.schema.SchemaDefinition;
@@ -40,6 +45,7 @@ import org.apache.pulsar.common.naming.TopicDomain;
import org.apache.pulsar.common.naming.TopicName;
import org.apache.pulsar.common.policies.data.ClusterData;
import org.apache.pulsar.common.policies.data.TenantInfo;
+import org.apache.pulsar.common.schema.SchemaInfo;
import org.apache.pulsar.common.schema.SchemaType;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
@@ -205,4 +211,30 @@ public class SchemaTest extends
MockedPulsarServiceBaseTest {
}
}
}
+
+ @Test
+ public void testNullKeyValueProperty() throws PulsarAdminException,
PulsarClientException {
+ final String tenant = PUBLIC_TENANT;
+ final String namespace = "test-namespace-" + randomName(16);
+ final String topicName = "test";
+
+ final String topic = TopicName.get(
+ TopicDomain.persistent.value(),
+ tenant,
+ namespace,
+ topicName).toString();
+ admin.namespaces().createNamespace(
+ tenant + "/" + namespace,
+ Sets.newHashSet(CLUSTER_NAME));
+
+ final Map<String, String> map = new HashMap<>();
+ map.put("key", null);
+ map.put(null, "value"); // null key is not allowed for JSON, it's only
for test here
+ Schema.INT32.getSchemaInfo().setProperties(map);
+
+ final Consumer<Integer> consumer =
pulsarClient.newConsumer(Schema.INT32).topic(topic)
+ .subscriptionName("sub")
+ .subscribe();
+ consumer.close();
+ }
}
diff --git
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/SchemaUtils.java
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/SchemaUtils.java
index 7d9ae25f..23ecb4e 100644
---
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/SchemaUtils.java
+++
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/SchemaUtils.java
@@ -238,7 +238,7 @@ public final class SchemaUtils {
sortedProperties.putAll(properties);
JsonObject object = new JsonObject();
sortedProperties.forEach((key, value) -> {
- object.add(key, new JsonPrimitive(value));
+ object.add(key, (value != null) ? new JsonPrimitive(value) :
null);
});
return object;
}
diff --git
a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/SchemaInfoTest.java
b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/SchemaInfoTest.java
index d187446..7e34a64 100644
---
a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/SchemaInfoTest.java
+++
b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/SchemaInfoTest.java
@@ -26,6 +26,7 @@ import org.apache.pulsar.common.schema.SchemaType;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import java.util.HashMap;
import java.util.Map;
import static org.testng.Assert.assertEquals;
@@ -317,5 +318,14 @@ public class SchemaInfoTest {
assertEquals(schemaInfo.getProperties(), Maps.newHashMap(map));
}
+ @Test
+ public void testNullPropertyValue() {
+ final Map<String, String> map = new HashMap<>();
+ map.put("key", null);
+ final IntSchema schema = new IntSchema();
+ schema.getSchemaInfo().setProperties(map);
+ // null key will be skipped by Gson when serializing JSON to String
+ assertEquals(schema.getSchemaInfo().toString(), INT32_SCHEMA_INFO);
+ }
}
}
diff --git
a/pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java
b/pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java
index ba619fc..0c80395 100644
---
a/pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java
+++
b/pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java
@@ -694,9 +694,11 @@ public class Commands {
.setType(getSchemaType(schemaInfo.getType()));
schemaInfo.getProperties().entrySet().stream().forEach(entry -> {
- schema.addProperty()
- .setKey(entry.getKey())
- .setValue(entry.getValue());
+ if (entry.getKey() != null && entry.getValue() != null) {
+ schema.addProperty()
+ .setKey(entry.getKey())
+ .setValue(entry.getValue());
+ }
});
}
