You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by pe...@apache.org on 2022/07/25 01:33:53 UTC
[pulsar] branch master updated: [improve] [admin] [PIP-179] Support the admin API to check unknown request parameters (#16577)
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 93e947b2ee0 [improve] [admin] [PIP-179] Support the admin API to check unknown request parameters (#16577)
93e947b2ee0 is described below
commit 93e947b2ee022a678979d97d44af13faf9c3db31
Author: fengyubiao <yu...@streamnative.io>
AuthorDate: Mon Jul 25 09:33:42 2022 +0800
[improve] [admin] [PIP-179] Support the admin API to check unknown request parameters (#16577)
---
.../web/DynamicSkipUnknownPropertyHandler.java | 56 +++++++++
.../pulsar/broker/web/JsonMapperProvider.java | 11 ++
.../web/DynamicSkipUnknownPropertyHandlerTest.java | 126 +++++++++++++++++++++
.../org/apache/pulsar/broker/PulsarService.java | 12 +-
.../org/apache/pulsar/broker/web/WebService.java | 22 +++-
.../apache/pulsar/broker/admin/AdminRestTest.java | 96 ++++++++++++++++
.../pulsar/broker/admin/NamespacesV2Test.java | 2 +-
7 files changed, 312 insertions(+), 13 deletions(-)
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/DynamicSkipUnknownPropertyHandler.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/DynamicSkipUnknownPropertyHandler.java
new file mode 100644
index 00000000000..3ebdbab761e
--- /dev/null
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/DynamicSkipUnknownPropertyHandler.java
@@ -0,0 +1,56 @@
+/**
+ * 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.pulsar.broker.web;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.deser.DeserializationProblemHandler;
+import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
+import java.io.IOException;
+import java.util.Collection;
+import lombok.Getter;
+import lombok.Setter;
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
+public class DynamicSkipUnknownPropertyHandler extends DeserializationProblemHandler {
+
+ @Getter
+ @Setter
+ private boolean skipUnknownProperty = true;
+
+ @Override
+ public boolean handleUnknownProperty(DeserializationContext deserializationContext, JsonParser p,
+ JsonDeserializer<?> deserializer, Object beanOrClass,
+ String propertyName) throws IOException {
+ Collection<Object> propIds = (deserializer == null) ? null : deserializer.getKnownPropertyNames();
+ UnrecognizedPropertyException unrecognizedPropertyException = UnrecognizedPropertyException
+ .from(p, beanOrClass, propertyName, propIds);
+ if (skipUnknownProperty){
+ if (log.isDebugEnabled()) {
+ log.debug(unrecognizedPropertyException.getMessage());
+ }
+ p.skipChildren();
+ return skipUnknownProperty;
+ } else {
+ throw unrecognizedPropertyException;
+ }
+ }
+}
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/JsonMapperProvider.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/JsonMapperProvider.java
index f9f767c622f..f4c4fc1837d 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/JsonMapperProvider.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/JsonMapperProvider.java
@@ -18,7 +18,9 @@
*/
package org.apache.pulsar.broker.web;
+import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.deser.DeserializationProblemHandler;
import javax.ws.rs.ext.ContextResolver;
import javax.ws.rs.ext.Provider;
import org.apache.pulsar.common.util.ObjectMapperFactory;
@@ -27,6 +29,15 @@ import org.apache.pulsar.common.util.ObjectMapperFactory;
public class JsonMapperProvider implements ContextResolver<ObjectMapper> {
private final ObjectMapper mapper = ObjectMapperFactory.create();
+ public JsonMapperProvider(){
+
+ }
+
+ public JsonMapperProvider(DeserializationProblemHandler handler){
+ mapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+ mapper.addHandler(handler);
+ }
+
@Override
public ObjectMapper getContext(Class<?> type) {
return mapper;
diff --git a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/web/DynamicSkipUnknownPropertyHandlerTest.java b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/web/DynamicSkipUnknownPropertyHandlerTest.java
new file mode 100644
index 00000000000..2d7b8e7e783
--- /dev/null
+++ b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/web/DynamicSkipUnknownPropertyHandlerTest.java
@@ -0,0 +1,126 @@
+/**
+ * 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.pulsar.broker.web;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
+import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
+import lombok.Data;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+@Test(groups = "broker-admin")
+public class DynamicSkipUnknownPropertyHandlerTest {
+
+ @Test
+ public void testHandleUnknownProperty() throws Exception{
+ DynamicSkipUnknownPropertyHandler handler = new DynamicSkipUnknownPropertyHandler();
+ handler.setSkipUnknownProperty(true);
+ // Case 1: initial ObjectMapper with "enable feature".
+ ObjectMapper objectMapper = new ObjectMapper();
+ objectMapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+ objectMapper.addHandler(handler);
+ ObjectReader objectReader = objectMapper.readerFor(TestBean.class);
+ // Assert skip unknown property and logging: objectMapper.
+ String json = "{\"name1\": \"James\",\"nm\":\"Paul\",\"name2\":\"Eric\"}";
+ TestBean testBean = objectMapper.readValue(json, TestBean.class);
+ Assert.assertNull(testBean.getName());
+ Assert.assertEquals(testBean.getName1(), "James");
+ Assert.assertEquals(testBean.getName2(), "Eric");
+ // Assert skip unknown property and logging: objectReader.
+ testBean = objectReader.readValue(json, TestBean.class);
+ Assert.assertNull(testBean.getName());
+ Assert.assertEquals(testBean.getName1(), "James");
+ Assert.assertEquals(testBean.getName2(), "Eric");
+ // Assert failure on unknown property.
+ handler.setSkipUnknownProperty(false);
+ try {
+ objectMapper.readValue(json, TestBean.class);
+ Assert.fail("Expect UnrecognizedPropertyException when set skipUnknownProperty false.");
+ } catch (UnrecognizedPropertyException e){
+
+ }
+ try {
+ objectReader.readValue(json, TestBean.class);
+ Assert.fail("Expect UnrecognizedPropertyException when set skipUnknownProperty false.");
+ } catch (UnrecognizedPropertyException e){
+
+ }
+ // Case 2: initial ObjectMapper with "disabled feature".
+ objectMapper = new ObjectMapper();
+ objectMapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+ objectMapper.addHandler(handler);
+ objectReader = objectMapper.readerFor(TestBean.class);
+ // Assert failure on unknown property.
+ try {
+ objectMapper.readValue(json, TestBean.class);
+ Assert.fail("Expect UnrecognizedPropertyException when set skipUnknownProperty false.");
+ } catch (UnrecognizedPropertyException e){
+
+ }
+ try {
+ objectReader.readValue(json, TestBean.class);
+ Assert.fail("Expect UnrecognizedPropertyException when set skipUnknownProperty false.");
+ } catch (UnrecognizedPropertyException e){
+
+ }
+ // Assert skip unknown property and logging.
+ handler.setSkipUnknownProperty(true);
+ testBean = objectMapper.readValue(json, TestBean.class);
+ Assert.assertNull(testBean.getName());
+ Assert.assertEquals(testBean.getName1(), "James");
+ Assert.assertEquals(testBean.getName2(), "Eric");
+ testBean = objectReader.readValue(json, TestBean.class);
+ Assert.assertNull(testBean.getName());
+ Assert.assertEquals(testBean.getName1(), "James");
+ Assert.assertEquals(testBean.getName2(), "Eric");
+ // Case 3: unknown property deserialize by object json.
+ json = "{\"name1\": \"James\",\"nm\":{\"name\":\"Paul\",\"age\":18},\"name2\":\"Eric\"}";
+ // Assert skip unknown property and logging.
+ handler.setSkipUnknownProperty(true);
+ testBean = objectMapper.readValue(json, TestBean.class);
+ Assert.assertNull(testBean.getName());
+ Assert.assertEquals(testBean.getName1(), "James");
+ Assert.assertEquals(testBean.getName2(), "Eric");
+ testBean = objectReader.readValue(json, TestBean.class);
+ Assert.assertNull(testBean.getName());
+ Assert.assertEquals(testBean.getName1(), "James");
+ Assert.assertEquals(testBean.getName2(), "Eric");
+ // Case 4: unknown property deserialize by array json.
+ json = "{\"name1\": \"James\",\"nm\":[\"name\",\"Paul\"],\"name2\":\"Eric\"}";
+ // Assert skip unknown property and logging.
+ handler.setSkipUnknownProperty(true);
+ testBean = objectMapper.readValue(json, TestBean.class);
+ Assert.assertNull(testBean.getName());
+ Assert.assertEquals(testBean.getName1(), "James");
+ Assert.assertEquals(testBean.getName2(), "Eric");
+ testBean = objectReader.readValue(json, TestBean.class);
+ Assert.assertNull(testBean.getName());
+ Assert.assertEquals(testBean.getName1(), "James");
+ Assert.assertEquals(testBean.getName2(), "Eric");
+ }
+
+ @Data
+ private static class TestBean {
+ private String name1;
+ private String name;
+ private String name2;
+ }
+}
\ No newline at end of file
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
index a79499438f5..408e776d66d 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
@@ -908,18 +908,18 @@ public class PulsarService implements AutoCloseable, ShutdownService {
// Add admin rest resources
webService.addRestResource("/",
- false, vipAttributeMap, VipStatus.class);
+ false, vipAttributeMap, false, VipStatus.class);
webService.addRestResources("/admin",
- true, attributeMap, "org.apache.pulsar.broker.admin.v1");
+ true, attributeMap, false, "org.apache.pulsar.broker.admin.v1");
webService.addRestResources("/admin/v2",
- true, attributeMap, "org.apache.pulsar.broker.admin.v2");
+ true, attributeMap, true, "org.apache.pulsar.broker.admin.v2");
webService.addRestResources("/admin/v3",
- true, attributeMap, "org.apache.pulsar.broker.admin.v3");
+ true, attributeMap, true, "org.apache.pulsar.broker.admin.v3");
webService.addRestResource("/lookup",
- true, attributeMap, TopicLookup.class,
+ true, attributeMap, true, TopicLookup.class,
org.apache.pulsar.broker.lookup.v2.TopicLookup.class);
webService.addRestResource("/topics",
- true, attributeMap, Topics.class);
+ true, attributeMap, true, Topics.class);
// Add metrics servlet
webService.addServlet("/metrics",
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java
index 0117ae31f28..508146da63f 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java
@@ -27,6 +27,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.servlet.DispatcherType;
+import lombok.Getter;
import org.apache.pulsar.broker.PulsarServerException;
import org.apache.pulsar.broker.PulsarService;
import org.apache.pulsar.broker.ServiceConfiguration;
@@ -75,6 +76,10 @@ public class WebService implements AutoCloseable {
private final FilterInitializer filterInitializer;
private JettyStatisticsCollector jettyStatisticsCollector;
+ @Getter
+ private static final DynamicSkipUnknownPropertyHandler sharedUnknownPropertyHandler =
+ new DynamicSkipUnknownPropertyHandler();
+
public WebService(PulsarService pulsar) throws PulsarServerException {
this.handlers = Lists.newArrayList();
this.pulsar = pulsar;
@@ -150,26 +155,31 @@ public class WebService implements AutoCloseable {
}
public void addRestResources(String basePath, boolean requiresAuthentication, Map<String, Object> attributeMap,
- String... javaPackages) {
+ boolean useSharedJsonMapperProvider, String... javaPackages) {
ResourceConfig config = new ResourceConfig();
for (String javaPackage : javaPackages) {
config.packages(false, javaPackage);
}
- addResourceServlet(basePath, requiresAuthentication, attributeMap, config);
+ addResourceServlet(basePath, requiresAuthentication, attributeMap, config, useSharedJsonMapperProvider);
}
public void addRestResource(String basePath, boolean requiresAuthentication, Map<String, Object> attributeMap,
- Class<?>... resourceClasses) {
+ boolean useSharedJsonMapperProvider, Class<?>... resourceClasses) {
ResourceConfig config = new ResourceConfig();
for (Class<?> resourceClass : resourceClasses) {
config.register(resourceClass);
}
- addResourceServlet(basePath, requiresAuthentication, attributeMap, config);
+ addResourceServlet(basePath, requiresAuthentication, attributeMap, config, useSharedJsonMapperProvider);
}
private void addResourceServlet(String basePath, boolean requiresAuthentication, Map<String, Object> attributeMap,
- ResourceConfig config) {
- config.register(JsonMapperProvider.class);
+ ResourceConfig config, boolean useSharedJsonMapperProvider) {
+ if (useSharedJsonMapperProvider){
+ JsonMapperProvider jsonMapperProvider = new JsonMapperProvider(sharedUnknownPropertyHandler);
+ config.register(jsonMapperProvider);
+ } else {
+ config.register(JsonMapperProvider.class);
+ }
config.register(MultiPartFeature.class);
ServletHolder servletHolder = new ServletHolder(new ServletContainer(config));
servletHolder.setAsyncSupported(true);
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminRestTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminRestTest.java
new file mode 100644
index 00000000000..176063ba0ff
--- /dev/null
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminRestTest.java
@@ -0,0 +1,96 @@
+/**
+ * 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.pulsar.broker.admin;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import javax.ws.rs.client.Client;
+import javax.ws.rs.client.ClientBuilder;
+import javax.ws.rs.client.Entity;
+import javax.ws.rs.client.WebTarget;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
+import org.apache.pulsar.common.policies.data.ClusterData;
+import org.apache.pulsar.common.policies.data.TenantInfoImpl;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+@Slf4j
+@Test(groups = "broker-admin")
+public class AdminRestTest extends MockedPulsarServiceBaseTest {
+
+ private final String clusterName = "test";
+ private final String tenantName = "t-tenant";
+ private final String namespaceName = "t-tenant/test-namespace";
+ private final String topicNameSuffix = "t-rest-topic";
+ private final String topicName = "persistent://" + namespaceName + "/" + topicNameSuffix;
+
+ @Test
+ public void testRejectUnknownEntityProperties() throws Exception{
+ // Build request command.
+ int port = pulsar.getWebService().getListenPortHTTP().get();
+ Client client = ClientBuilder.newClient();
+ WebTarget target = client.target("http://127.0.0.1:" + port
+ + "/admin/v2/persistent/" + namespaceName + "/" + topicNameSuffix + "/retention");
+ Map<String,Object> data = new HashMap<>();
+ data.put("retention_size_in_mb", -1);
+ data.put("retention_time_in_minutes", 40320);
+ // Configuration default, response success.
+ Response response = target.request(MediaType.APPLICATION_JSON_TYPE).buildPost(Entity.json(data)).invoke();
+ Assert.assertTrue(response.getStatus() / 200 == 1);
+ // Enabled feature, bad request response.
+ pulsar.getWebService().getSharedUnknownPropertyHandler().setSkipUnknownProperty(false);
+ response = target.request(MediaType.APPLICATION_JSON_TYPE).buildPost(Entity.json(data)).invoke();
+ Assert.assertEquals(response.getStatus(), 400);
+ // Disabled feature, response success.
+ pulsar.getWebService().getSharedUnknownPropertyHandler().setSkipUnknownProperty(true);
+ response = target.request(MediaType.APPLICATION_JSON_TYPE).buildPost(Entity.json(data)).invoke();
+ Assert.assertTrue(response.getStatus() / 200 == 1);
+ }
+
+ @BeforeMethod
+ @Override
+ protected void setup() throws Exception {
+ resetConfig();
+ super.internalSetup();
+ // Create tenant, namespace, topic
+ admin.clusters().createCluster(clusterName, ClusterData.builder().serviceUrl(brokerUrl.toString()).build());
+ admin.tenants().createTenant(tenantName,
+ new TenantInfoImpl(Collections.singleton("a"), Collections.singleton(clusterName)));
+ admin.namespaces().createNamespace(namespaceName);
+ admin.topics().createNonPartitionedTopic(topicName);
+ }
+
+ @AfterMethod
+ @Override
+ protected void cleanup() throws Exception {
+ // cleanup.
+ admin.topics().delete(topicName);
+ admin.namespaces().deleteNamespace(namespaceName);
+ admin.tenants().deleteTenant(tenantName);
+ admin.clusters().deleteCluster(clusterName);
+ // super cleanup.
+ super.internalCleanup();
+ }
+}
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesV2Test.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesV2Test.java
index aa0f6793fcd..239ef8546cf 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesV2Test.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesV2Test.java
@@ -53,7 +53,7 @@ import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
-@Test(groups = "broker-admin-v2")
+@Test(groups = "broker-admin")
public class NamespacesV2Test extends MockedPulsarServiceBaseTest {
private static final Logger log = LoggerFactory.getLogger(NamespacesV2Test.class);