Merge pull request #1214 from wasphin/feature/restful-mapped-only

add option to allow access methods from default url

src/brpc/policy/http_rpc_protocol.cpp

@@ -1011,7 +1011,7 @@ FindMethodPropertyByURI(const std::string& uri_path, const Server* server,
     const Server::MethodProperty* mp =
         FindMethodPropertyByURIImpl(uri_path, server, unresolved_path);
     if (mp != NULL) {
-        if (mp->http_url != NULL) {
+        if (mp->http_url != NULL && !mp->params.allow_default_url) {
             // the restful method is accessed from its
             // default url (SERVICE/METHOD) which should be rejected.
             return NULL;

src/brpc/server.cpp

@@ -155,6 +155,7 @@ ServerSSLOptions* ServerOptions::mutable_ssl_options() {
 
 Server::MethodProperty::OpaqueParams::OpaqueParams()
     : is_tabbed(false)
+    , allow_default_url(false)
     , allow_http_body_to_pb(true)
     , pb_bytes_to_base64(false) {
 }
@@ -1207,6 +1208,7 @@ int Server::AddServiceInternal(google::protobuf::Service* service,
         mp.is_builtin_service = is_builtin_service;
         mp.own_method_status = true;
         mp.params.is_tabbed = !!tabbed;
+        mp.params.allow_default_url = svc_opt.allow_default_url;
         mp.params.allow_http_body_to_pb = svc_opt.allow_http_body_to_pb;
         mp.params.pb_bytes_to_base64 = svc_opt.pb_bytes_to_base64;
         mp.service = service;
@@ -1293,6 +1295,7 @@ int Server::AddServiceInternal(google::protobuf::Service* service,
                 }
                 MethodProperty::OpaqueParams params;
                 params.is_tabbed = !!tabbed;
+                params.allow_default_url = svc_opt.allow_default_url;
                 params.allow_http_body_to_pb = svc_opt.allow_http_body_to_pb;
                 params.pb_bytes_to_base64 = svc_opt.pb_bytes_to_base64;
                 if (!_global_restful_map->AddMethod(
@@ -1330,6 +1333,7 @@ int Server::AddServiceInternal(google::protobuf::Service* service,
             }
             MethodProperty::OpaqueParams params;
             params.is_tabbed = !!tabbed;
+            params.allow_default_url = svc_opt.allow_default_url;
             params.allow_http_body_to_pb = svc_opt.allow_http_body_to_pb;
             params.pb_bytes_to_base64 = svc_opt.pb_bytes_to_base64;
             if (!m->AddMethod(mappings[i].path, service, params,
@@ -1384,6 +1388,7 @@ int Server::AddServiceInternal(google::protobuf::Service* service,
 
 ServiceOptions::ServiceOptions()
     : ownership(SERVER_DOESNT_OWN_SERVICE)
+    , allow_default_url(false)
     , allow_http_body_to_pb(true)
 #ifdef BAIDU_INTERNAL
     , pb_bytes_to_base64(false)
@@ -1401,11 +1406,13 @@ int Server::AddService(google::protobuf::Service* service,
 
 int Server::AddService(google::protobuf::Service* service,
                        ServiceOwnership ownership,
-                       const butil::StringPiece& restful_mappings) {
+                       const butil::StringPiece& restful_mappings,
+                       bool allow_default_url) {
     ServiceOptions options;
     options.ownership = ownership;
     // TODO: This is weird
     options.restful_mappings = restful_mappings.as_string();
+    options.allow_default_url = allow_default_url;
     return AddServiceInternal(service, false, options);
 }
 

src/brpc/server.h

@@ -277,6 +277,11 @@ struct ServiceOptions {
     // Default: empty
     std::string restful_mappings;
 
+    // Work with restful_mappings, if this flag is false, reject methods accessed
+    // from default urls (SERVICE/METHOD).
+    // Default: false
+    bool allow_default_url;
+
     // [ Not recommended to change this option ]
     // If this flag is true, the service will convert http body to protobuf
     // when the pb schema is non-empty in http servings. The body must be
@@ -338,6 +343,7 @@ public:
         // will be used when the service is queried.
         struct OpaqueParams {
             bool is_tabbed;
+            bool allow_default_url;
             bool allow_http_body_to_pb;
             bool pb_bytes_to_base64;
             OpaqueParams();
@@ -414,7 +420,8 @@ public:
                    ServiceOwnership ownership);
     int AddService(google::protobuf::Service* service,
                    ServiceOwnership ownership,
-                   const butil::StringPiece& restful_mappings);
+                   const butil::StringPiece& restful_mappings,
+                   bool allow_default_url = false);
     int AddService(google::protobuf::Service* service,
                    const ServiceOptions& options);
 

test/brpc_server_unittest.cpp

@@ -669,6 +669,16 @@ TEST_F(ServerTest, restful_mapping) {
                   " /v1/*/* => Echo"));
     ASSERT_EQ(0u, server9.service_count());
     
+    // default url access
+    brpc::Server server10;
+    ASSERT_EQ(0, server10.AddService(
+                  &service_v1,
+                  brpc::SERVER_DOESNT_OWN_SERVICE,
+                  "/v1/echo => Echo",
+                  true));
+    ASSERT_EQ(1u, server10.service_count());
+    ASSERT_FALSE(server10._global_restful_map);
+
     // Access services
     ASSERT_EQ(0, server1.Start(port, NULL));
     brpc::Channel http_channel;
@@ -867,6 +877,31 @@ TEST_F(ServerTest, restful_mapping) {
     server1.Stop(0);
     server1.Join();
 
+    ASSERT_EQ(0, server10.Start(port, NULL));
+
+    // access v1.Echo via /v1/echo.
+    cntl.Reset();
+    cntl.http_request().uri() = "/v1/echo";
+    cntl.http_request().set_method(brpc::HTTP_METHOD_POST);
+    cntl.request_attachment().append("{\"message\":\"foo\"}");
+    http_channel.CallMethod(NULL, &cntl, NULL, NULL, NULL);
+    ASSERT_FALSE(cntl.Failed()) << cntl.ErrorText();
+    ASSERT_EQ(12, service_v1.ncalled.load());
+    ASSERT_EQ("{\"message\":\"foo_v1\"}", cntl.response_attachment());
+
+    // access v1.Echo via default url
+    cntl.Reset();
+    cntl.http_request().uri() = "/EchoService/Echo";
+    cntl.http_request().set_method(brpc::HTTP_METHOD_POST);
+    cntl.request_attachment().append("{\"message\":\"foo\"}");
+    http_channel.CallMethod(NULL, &cntl, NULL, NULL, NULL);
+    ASSERT_FALSE(cntl.Failed()) << cntl.ErrorText();
+    ASSERT_EQ(13, service_v1.ncalled.load());
+    ASSERT_EQ("{\"message\":\"foo_v1\"}", cntl.response_attachment());
+
+    server10.Stop(0);
+    server10.Join();
+
     // Removing the service should update _global_restful_map.
     ASSERT_EQ(0, server1.RemoveService(&service_v1));
     ASSERT_EQ(0u, server1.service_count());