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 0000000000000..3ebdbab761e35 --- /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 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 f9f767c622f2b..f4c4fc1837d9d 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 @@ public class JsonMapperProvider implements ContextResolver { 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 0000000000000..2d7b8e7e78340 --- /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 a79499438f5c5..408e776d66d07 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 @@ private void addWebServerHandlers(WebService webService, // 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 0117ae31f287d..508146da63f42 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.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 WebService(PulsarService pulsar) throws PulsarServerException { } public void addRestResources(String basePath, boolean requiresAuthentication, Map 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 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 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 0000000000000..176063ba0ffd9 --- /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 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 aa0f6793fcdc5..239ef8546cfc6 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.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);