Fix #330 Potentially request.body can be None when using a custom adapter (#331)

* Fix #330 Potentially request.body can be None when using a custom adapter

* Add scenario tests

Author: seratch

slack_bolt/request/async_request.py

@@ -9,7 +9,6 @@
     build_normalized_headers,
     extract_content_type,
     error_message_raw_body_required_in_http_mode,
-    error_message_unknown_request_body_type,
 )
 
 
@@ -45,7 +44,7 @@ def __init__(
 
         if mode == "http":
             # HTTP Mode
-            if not isinstance(body, str):
+            if body is not None and not isinstance(body, str):
                 raise BoltError(error_message_raw_body_required_in_http_mode())
             self.raw_body = body if body is not None else ""
         else:
@@ -60,12 +59,14 @@ def __init__(
         self.query = parse_query(query)
         self.headers = build_normalized_headers(headers)
         self.content_type = extract_content_type(self.headers)
+
         if isinstance(body, str):
             self.body = parse_body(self.raw_body, self.content_type)
         elif isinstance(body, dict):
             self.body = body
         else:
-            raise BoltError(error_message_unknown_request_body_type())
+            self.body = {}
+
         self.context = build_async_context(
             AsyncBoltContext(context if context else {}), self.body
         )

slack_bolt/request/internals.py

@@ -183,10 +183,6 @@ def error_message_raw_body_required_in_http_mode() -> str:
     return "`body` must be a raw string data when running in the HTTP server mode"
 
 
-def error_message_unknown_request_body_type() -> str:
-    return "`body` must be either str or dict"
-
-
 def debug_multiple_response_urls_detected() -> str:
     return (
         "`response_urls` in the body has multiple URLs in it. "

slack_bolt/request/request.py

@@ -9,7 +9,6 @@
     build_context,
     extract_content_type,
     error_message_raw_body_required_in_http_mode,
-    error_message_unknown_request_body_type,
 )
 
 
@@ -44,7 +43,7 @@ def __init__(
         """
         if mode == "http":
             # HTTP Mode
-            if not isinstance(body, str):
+            if body is not None and not isinstance(body, str):
                 raise BoltError(error_message_raw_body_required_in_http_mode())
             self.raw_body = body if body is not None else ""
         else:
@@ -59,12 +58,13 @@ def __init__(
         self.query = parse_query(query)
         self.headers = build_normalized_headers(headers)
         self.content_type = extract_content_type(self.headers)
+
         if isinstance(body, str):
             self.body = parse_body(self.raw_body, self.content_type)
         elif isinstance(body, dict):
             self.body = body
         else:
-            raise BoltError(error_message_unknown_request_body_type())
+            self.body = {}
 
         self.context = build_context(BoltContext(context if context else {}), self.body)
         self.lazy_only = bool(self.headers.get("x-slack-bolt-lazy-only", [False])[0])

tests/scenario_tests/test_app.py

@@ -3,7 +3,7 @@
 from slack_sdk.oauth.installation_store import FileInstallationStore
 from slack_sdk.oauth.state_store import FileOAuthStateStore
 
-from slack_bolt import App, Say
+from slack_bolt import App, Say, BoltRequest
 from slack_bolt.authorization import AuthorizeResult
 from slack_bolt.error import BoltError
 from slack_bolt.oauth import OAuthFlow
@@ -187,3 +187,38 @@ def test_installation_store_conflicts(self):
             installation_store=store1,
         )
         assert app.installation_store is store1
+
+    def test_none_body(self):
+        app = App(signing_secret="valid", client=self.web_client)
+
+        req = BoltRequest(body=None, headers={}, mode="http")
+        response = app.dispatch(req)
+        # request verification failure
+        assert response.status == 401
+        assert response.body == '{"error": "invalid request"}'
+
+        req = BoltRequest(body=None, headers={}, mode="socket_mode")
+        response = app.dispatch(req)
+        # request verification is skipped for Socket Mode
+        assert response.status == 404
+        assert response.body == '{"error": "unhandled request"}'
+
+    def test_none_body_no_middleware(self):
+        app = App(
+            signing_secret="valid",
+            client=self.web_client,
+            ssl_check_enabled=False,
+            ignoring_self_events_enabled=False,
+            request_verification_enabled=False,
+            token_verification_enabled=False,
+            url_verification_enabled=False,
+        )
+        req = BoltRequest(body=None, headers={}, mode="http")
+        response = app.dispatch(req)
+        assert response.status == 404
+        assert response.body == '{"error": "unhandled request"}'
+
+        req = BoltRequest(body=None, headers={}, mode="socket_mode")
+        response = app.dispatch(req)
+        assert response.status == 404
+        assert response.body == '{"error": "unhandled request"}'

tests/scenario_tests_async/test_app_dispatch.py

@@ -0,0 +1,72 @@
+import asyncio
+
+import pytest
+from slack_sdk.web.async_client import AsyncWebClient
+
+from slack_bolt.app.async_app import AsyncApp
+from slack_bolt.request.async_request import AsyncBoltRequest
+from tests.mock_web_api_server import (
+    setup_mock_web_api_server,
+    cleanup_mock_web_api_server,
+)
+from tests.utils import remove_os_env_temporarily, restore_os_env
+
+
+class TestAsyncAppDispatch:
+    signing_secret = "secret"
+    valid_token = "xoxb-valid"
+    mock_api_server_base_url = "http://localhost:8888"
+    web_client = AsyncWebClient(token=valid_token, base_url=mock_api_server_base_url)
+
+    @pytest.fixture
+    def event_loop(self):
+        old_os_env = remove_os_env_temporarily()
+        try:
+            setup_mock_web_api_server(self)
+            loop = asyncio.get_event_loop()
+            yield loop
+            loop.close()
+            cleanup_mock_web_api_server(self)
+        finally:
+            restore_os_env(old_os_env)
+
+    @pytest.mark.asyncio
+    async def test_none_body(self):
+        app = AsyncApp(
+            client=self.web_client,
+            signing_secret=self.signing_secret,
+        )
+
+        req = AsyncBoltRequest(body=None, headers={}, mode="http")
+        response = await app.async_dispatch(req)
+        # request verification failure
+        assert response.status == 401
+        assert response.body == '{"error": "invalid request"}'
+
+        req = AsyncBoltRequest(body=None, headers={}, mode="socket_mode")
+        response = await app.async_dispatch(req)
+        # request verification is skipped for Socket Mode
+        assert response.status == 404
+        assert response.body == '{"error": "unhandled request"}'
+
+    @pytest.mark.asyncio
+    async def test_none_body_no_middleware(self):
+        app = AsyncApp(
+            client=self.web_client,
+            signing_secret=self.signing_secret,
+            ssl_check_enabled=False,
+            ignoring_self_events_enabled=False,
+            request_verification_enabled=False,
+            # token_verification_enabled=False,
+            url_verification_enabled=False,
+        )
+
+        req = AsyncBoltRequest(body=None, headers={}, mode="http")
+        response = await app.async_dispatch(req)
+        assert response.status == 404
+        assert response.body == '{"error": "unhandled request"}'
+
+        req = AsyncBoltRequest(body=None, headers={}, mode="socket_mode")
+        response = await app.async_dispatch(req)
+        assert response.status == 404
+        assert response.body == '{"error": "unhandled request"}'

tests/slack_bolt/request/test_request.py

@@ -0,0 +1,23 @@
+from slack_bolt.request.request import BoltRequest
+
+
+class TestRequest:
+    def setup_method(self):
+        pass
+
+    def teardown_method(self):
+        pass
+
+    def test_all_none_inputs_http(self):
+        req = BoltRequest(body=None, headers=None, query=None, context=None)
+        assert req is not None
+        assert req.raw_body == ""
+        assert req.body == {}
+
+    def test_all_none_inputs_socket_mode(self):
+        req = BoltRequest(
+            body=None, headers=None, query=None, context=None, mode="socket_mode"
+        )
+        assert req is not None
+        assert req.raw_body == ""
+        assert req.body == {}

tests/slack_bolt_async/request/__init__.py

@@ -0,0 +1,21 @@
+import pytest
+
+from slack_bolt.request.async_request import AsyncBoltRequest
+
+
+class TestAsyncRequest:
+    @pytest.mark.asyncio
+    async def test_all_none_values_http(self):
+        req = AsyncBoltRequest(body=None, headers=None, query=None, context=None)
+        assert req is not None
+        assert req.raw_body == ""
+        assert req.body == {}
+
+    @pytest.mark.asyncio
+    async def test_all_none_values_socket_mode(self):
+        req = AsyncBoltRequest(
+            body=None, headers=None, query=None, context=None, mode="socket_mode"
+        )
+        assert req is not None
+        assert req.raw_body == ""
+        assert req.body == {}