Pick slackapi#833 for v3 branch

terrorizer1980 · Oct 6, 2020 · 5317c81 · 5317c81
1 parent 0df7596
commit 5317c81
Show file tree

Hide file tree

Showing 15 changed files with 118 additions and 28 deletions.
diff --git a/slack/web/base_client.py b/slack/web/base_client.py
@@ -475,7 +475,7 @@ def _perform_urllib_http_request(
                     resp = urlopen(  # skipcq: BAN-B310
                         req, context=self.ssl, timeout=self.timeout
                     )
-                charset = resp.headers.get_content_charset()
+                charset = resp.headers.get_content_charset() or "utf-8"
                 body: str = resp.read().decode(charset)  # read the response body here
                 return {"status": resp.code, "headers": resp.headers, "body": body}
             raise SlackRequestError(f"Invalid URL detected: {url}")
@@ -485,7 +485,7 @@ def _perform_urllib_http_request(
                 # for compatibility with aiohttp
                 resp["headers"]["Retry-After"] = resp["headers"]["retry-after"]
 
-            charset = e.headers.get_content_charset()
+            charset = e.headers.get_content_charset() or "utf-8"
             body: str = e.read().decode(charset)  # read the response body here
             resp["body"] = body
             return resp

diff --git a/slack/webhook/client.py b/slack/webhook/client.py
@@ -136,7 +136,7 @@ def _perform_http_request(
             return resp
 
         except HTTPError as e:
-            charset = e.headers.get_content_charset()
+            charset = e.headers.get_content_charset() or "utf-8"
             body: str = e.read().decode(charset)  # read the response body here
             resp = WebhookResponse(
                 url=url, status_code=e.code, body=body, headers=e.headers,

diff --git a/slack_sdk/web/base_client.py b/slack_sdk/web/base_client.py
@@ -390,7 +390,7 @@ def _perform_urllib_http_request(
                     resp = urlopen(  # skipcq: BAN-B310
                         req, context=self.ssl, timeout=self.timeout
                     )
-                charset = resp.headers.get_content_charset()
+                charset = resp.headers.get_content_charset() or "utf-8"
                 body: str = resp.read().decode(charset)  # read the response body here
                 return {"status": resp.code, "headers": resp.headers, "body": body}
             raise SlackRequestError(f"Invalid URL detected: {url}")

diff --git a/slack_sdk/web/legacy_base_client.py b/slack_sdk/web/legacy_base_client.py
@@ -470,7 +470,7 @@ def _perform_urllib_http_request(
                     resp = urlopen(  # skipcq: BAN-B310
                         req, context=self.ssl, timeout=self.timeout
                     )
-                charset = resp.headers.get_content_charset()
+                charset = resp.headers.get_content_charset() or "utf-8"
                 body: str = resp.read().decode(charset)  # read the response body here
                 return {"status": resp.code, "headers": resp.headers, "body": body}
             raise SlackRequestError(f"Invalid URL detected: {url}")

diff --git a/tests/slack_sdk/web/mock_web_api_server.py b/tests/slack_sdk/web/mock_web_api_server.py
@@ -20,6 +20,8 @@ class MockHandler(SimpleHTTPRequestHandler):
 
     html_response_body = '<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">\n<html><head>\n<title>404 Not Found</title>\n</head><body>\n<h1>Not Found</h1>\n<p>The requested URL /api/team.info was not found on this server.</p>\n</body></html>\n'
 
+    error_html_response_body = '<!DOCTYPE html>\n<html lang="en">\n<head>\n\t<meta charset="utf-8">\n\t<title>Server Error | Slack</title>\n\t<meta name="author" content="Slack">\n\t<style></style>\n</head>\n<body>\n\t<nav class="top persistent">\n\t\t<a href="https://status.slack.com/" class="logo" data-qa="logo"></a>\n\t</nav>\n\t<div id="page">\n\t\t<div id="page_contents">\n\t\t\t<h1>\n\t\t\t\t<svg width="30px" height="27px" viewBox="0 0 60 54" class="warning_icon"><path d="" fill="#D94827"/></svg>\n\t\t\t\tServer Error\n\t\t\t</h1>\n\t\t\t<div class="card">\n\t\t\t\t<p>It seems like there’s a problem connecting to our servers, and we’re investigating the issue.</p>\n\t\t\t\t<p>Please <a href="https://status.slack.com/">check our Status page for updates</a>.</p>\n\t\t\t</div>\n\t\t</div>\n\t</div>\n\t<script type="text/javascript">\n\t\tif (window.desktop) {\n\t\t\tdocument.documentElement.className = \'desktop\';\n\t\t}\n\n\t\tvar FIVE_MINS = 5 * 60 * 1000;\n\t\tvar TEN_MINS = 10 * 60 * 1000;\n\n\t\tfunction randomBetween(min, max) {\n\t\t\treturn Math.floor(Math.random() * (max - (min + 1))) + min;\n\t\t}\n\n\t\twindow.setTimeout(function () {\n\t\t\twindow.location.reload(true);\n\t\t}, randomBetween(FIVE_MINS, TEN_MINS));\n\t</script>\n</body>\n</html>'
+
     def is_valid_user_agent(self):
         user_agent = self.headers["User-Agent"]
         return self.pattern_for_language.search(
@@ -112,11 +114,22 @@ def _handle(self):
 
                 if pattern == "html_response":
                     self.send_response(404)
-                    self.set_common_headers()
+                    self.send_header("content-type", "text/html; charset=utf-8")
+                    self.end_headers()
                     self.wfile.write(self.html_response_body.encode("utf-8"))
                     self.wfile.close()
                     return
 
+                if pattern == "error_html_response":
+                    self.send_response(503)
+                    # no charset here is intentional for testing
+                    self.send_header("content-type", "text/html")
+                    self.send_header("connection", "close")
+                    self.end_headers()
+                    self.wfile.write(self.error_html_response_body.encode("utf-8"))
+                    self.wfile.close()
+                    return
+
                 if pattern.startswith("user-agent"):
                     elements = pattern.split(" ")
                     prefix, suffix = elements[1], elements[-1]

diff --git a/tests/slack_sdk/web/test_web_client_issue_829.py b/tests/slack_sdk/web/test_web_client_issue_829.py
@@ -0,0 +1,27 @@
+import unittest
+
+import slack_sdk.errors as err
+from slack_sdk.web import WebClient
+from tests.slack_sdk.web.mock_web_api_server import (
+    setup_mock_web_api_server,
+    cleanup_mock_web_api_server,
+)
+
+
+class TestWebClient_Issue_829(unittest.TestCase):
+    def setUp(self):
+        setup_mock_web_api_server(self)
+
+    def tearDown(self):
+        cleanup_mock_web_api_server(self)
+
+    def test_html_response_body_issue_829(self):
+        client = WebClient(base_url="http://localhost:8888")
+        try:
+            client.users_list(token="xoxb-error_html_response")
+            self.fail("SlackApiError expected here")
+        except err.SlackApiError as e:
+            self.assertTrue(
+                str(e).startswith("Failed to parse the response body: Expecting value: "),
+                e,
+            )
diff --git a/tests/slack_sdk/webhook/mock_web_api_server.py b/tests/slack_sdk/webhook/mock_web_api_server.py
@@ -16,6 +16,8 @@ class MockHandler(SimpleHTTPRequestHandler):
     pattern_for_language = re.compile("python/(\\S+)", re.IGNORECASE)
     pattern_for_package_identifier = re.compile("slackclient/(\\S+)")
 
+    error_html_response_body = '<!DOCTYPE html>\n<html lang="en">\n<head>\n\t<meta charset="utf-8">\n\t<title>Server Error | Slack</title>\n\t<meta name="author" content="Slack">\n\t<style></style>\n</head>\n<body>\n\t<nav class="top persistent">\n\t\t<a href="https://status.slack.com/" class="logo" data-qa="logo"></a>\n\t</nav>\n\t<div id="page">\n\t\t<div id="page_contents">\n\t\t\t<h1>\n\t\t\t\t<svg width="30px" height="27px" viewBox="0 0 60 54" class="warning_icon"><path d="" fill="#D94827"/></svg>\n\t\t\t\tServer Error\n\t\t\t</h1>\n\t\t\t<div class="card">\n\t\t\t\t<p>It seems like there’s a problem connecting to our servers, and we’re investigating the issue.</p>\n\t\t\t\t<p>Please <a href="https://status.slack.com/">check our Status page for updates</a>.</p>\n\t\t\t</div>\n\t\t</div>\n\t</div>\n\t<script type="text/javascript">\n\t\tif (window.desktop) {\n\t\t\tdocument.documentElement.className = \'desktop\';\n\t\t}\n\n\t\tvar FIVE_MINS = 5 * 60 * 1000;\n\t\tvar TEN_MINS = 10 * 60 * 1000;\n\n\t\tfunction randomBetween(min, max) {\n\t\t\treturn Math.floor(Math.random() * (max - (min + 1))) + min;\n\t\t}\n\n\t\twindow.setTimeout(function () {\n\t\t\twindow.location.reload(true);\n\t\t}, randomBetween(FIVE_MINS, TEN_MINS));\n\t</script>\n</body>\n</html>'
+
     def is_valid_user_agent(self):
         user_agent = self.headers["User-Agent"]
         return self.pattern_for_language.search(
@@ -52,8 +54,11 @@ def do_POST(self):
 
             if self.path == "/error":
                 self.send_response(HTTPStatus.INTERNAL_SERVER_ERROR)
-                self.set_common_headers()
-                self.wfile.write("error".encode("utf-8"))
+                # no charset here is intentional for testing
+                self.send_header("content-type", "text/html")
+                self.send_header("connection", "close")
+                self.end_headers()
+                self.wfile.write(self.error_html_response_body.encode("utf-8"))
                 self.wfile.close()
                 return
 

diff --git a/tests/slack_sdk/webhook/test_webhook.py b/tests/slack_sdk/webhook/test_webhook.py
@@ -160,7 +160,7 @@ def test_error_response(self):
         client = WebhookClient(url="http://localhost:8888/error")
         resp: WebhookResponse = client.send_dict({"text": "hello!"})
         self.assertEqual(500, resp.status_code)
-        self.assertEqual("error", resp.body)
+        self.assertTrue(resp.body.startswith("<!DOCTYPE html>"))
 
     def test_proxy_issue_714(self):
         client = WebhookClient(

diff --git a/tests/slack_sdk_async/web/test_async_web_client.py b/tests/slack_sdk_async/web/test_async_web_client.py
@@ -131,11 +131,10 @@ async def test_html_response_body_issue_718_async(self):
             await self.client.users_list(token="xoxb-html_response")
             self.fail("SlackApiError expected here")
         except err.SlackApiError as e:
-            self.assertTrue(
-                str(e).startswith(
-                    "Failed to parse the response body: Expecting value: line 1 column 1 (char 0)"
-                ),
-                e,
+            self.assertEqual(
+                "The request to the Slack API failed.\n"
+                "The server responded with: {}",
+                str(e)
             )
 
     @async_test

diff --git a/tests/slack_sdk_async/web/test_web_client_issue_829.py b/tests/slack_sdk_async/web/test_web_client_issue_829.py
@@ -0,0 +1,30 @@
+import unittest
+
+import slack_sdk.errors as err
+from slack_sdk.web.async_client import AsyncWebClient
+from tests.helpers import async_test
+from tests.slack_sdk.web.mock_web_api_server import (
+    setup_mock_web_api_server,
+    cleanup_mock_web_api_server,
+)
+
+
+class TestWebClient_Issue_829(unittest.TestCase):
+    def setUp(self):
+        setup_mock_web_api_server(self)
+
+    def tearDown(self):
+        cleanup_mock_web_api_server(self)
+
+    @async_test
+    async def test_html_response_body_issue_829_async(self):
+        client = AsyncWebClient(base_url="http://localhost:8888")
+        try:
+            await client.users_list(token="xoxb-error_html_response")
+            self.fail("SlackApiError expected here")
+        except err.SlackApiError as e:
+            self.assertEqual(
+                "The request to the Slack API failed.\n"
+                "The server responded with: {}",
+                str(e)
+            )
diff --git a/tests/web/mock_web_api_server.py b/tests/web/mock_web_api_server.py
@@ -20,6 +20,8 @@ class MockHandler(SimpleHTTPRequestHandler):
 
     html_response_body = '<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">\n<html><head>\n<title>404 Not Found</title>\n</head><body>\n<h1>Not Found</h1>\n<p>The requested URL /api/team.info was not found on this server.</p>\n</body></html>\n'
 
+    error_html_response_body = '<!DOCTYPE html>\n<html lang="en">\n<head>\n\t<meta charset="utf-8">\n\t<title>Server Error | Slack</title>\n\t<meta name="author" content="Slack">\n\t<style></style>\n</head>\n<body>\n\t<nav class="top persistent">\n\t\t<a href="https://status.slack.com/" class="logo" data-qa="logo"></a>\n\t</nav>\n\t<div id="page">\n\t\t<div id="page_contents">\n\t\t\t<h1>\n\t\t\t\t<svg width="30px" height="27px" viewBox="0 0 60 54" class="warning_icon"><path d="" fill="#D94827"/></svg>\n\t\t\t\tServer Error\n\t\t\t</h1>\n\t\t\t<div class="card">\n\t\t\t\t<p>It seems like there’s a problem connecting to our servers, and we’re investigating the issue.</p>\n\t\t\t\t<p>Please <a href="https://status.slack.com/">check our Status page for updates</a>.</p>\n\t\t\t</div>\n\t\t</div>\n\t</div>\n\t<script type="text/javascript">\n\t\tif (window.desktop) {\n\t\t\tdocument.documentElement.className = \'desktop\';\n\t\t}\n\n\t\tvar FIVE_MINS = 5 * 60 * 1000;\n\t\tvar TEN_MINS = 10 * 60 * 1000;\n\n\t\tfunction randomBetween(min, max) {\n\t\t\treturn Math.floor(Math.random() * (max - (min + 1))) + min;\n\t\t}\n\n\t\twindow.setTimeout(function () {\n\t\t\twindow.location.reload(true);\n\t\t}, randomBetween(FIVE_MINS, TEN_MINS));\n\t</script>\n</body>\n</html>'
+
     def is_valid_user_agent(self):
         user_agent = self.headers["User-Agent"]
         return self.pattern_for_language.search(
@@ -112,11 +114,22 @@ def _handle(self):
 
                 if pattern == "html_response":
                     self.send_response(404)
-                    self.set_common_headers()
+                    self.send_header("content-type", "text/html; charset=utf-8")
+                    self.end_headers()
                     self.wfile.write(self.html_response_body.encode("utf-8"))
                     self.wfile.close()
                     return
 
+                if pattern == "error_html_response":
+                    self.send_response(503)
+                    # no charset here is intentional for testing
+                    self.send_header("content-type", "text/html")
+                    self.send_header("connection", "close")
+                    self.end_headers()
+                    self.wfile.write(self.error_html_response_body.encode("utf-8"))
+                    self.wfile.close()
+                    return
+
                 if pattern.startswith("user-agent"):
                     elements = pattern.split(" ")
                     prefix, suffix = elements[1], elements[-1]

diff --git a/tests/web/test_async_web_client.py b/tests/web/test_async_web_client.py
@@ -128,11 +128,10 @@ async def test_html_response_body_issue_718_async(self):
             await self.client.users_list(token="xoxb-html_response")
             self.fail("SlackApiError expected here")
         except err.SlackApiError as e:
-            self.assertTrue(
-                str(e).startswith(
-                    "Failed to parse the response body: Expecting value: line 1 column 1 (char 0)"
-                ),
-                e,
+            self.assertEqual(
+                "The request to the Slack API failed.\n"
+                "The server responded with: {}",
+                str(e)
             )
 
     @async_test

diff --git a/tests/web/test_web_client.py b/tests/web/test_web_client.py
@@ -273,11 +273,10 @@ async def test_html_response_body_issue_718_async(self):
             await client.users_list(token="xoxb-html_response")
             self.fail("SlackApiError expected here")
         except err.SlackApiError as e:
-            self.assertTrue(
-                str(e).startswith(
-                    "Failed to parse the response body: Expecting value: line 1 column 1 (char 0)"
-                ),
-                e,
+            self.assertEqual(
+                "The request to the Slack API failed.\n"
+                "The server responded with: {}",
+                str(e)
             )
 
     def test_user_agent_customization_issue_769(self):

diff --git a/tests/webhook/mock_web_api_server.py b/tests/webhook/mock_web_api_server.py
@@ -16,6 +16,8 @@ class MockHandler(SimpleHTTPRequestHandler):
     pattern_for_language = re.compile("python/(\\S+)", re.IGNORECASE)
     pattern_for_package_identifier = re.compile("slackclient/(\\S+)")
 
+    error_html_response_body = '<!DOCTYPE html>\n<html lang="en">\n<head>\n\t<meta charset="utf-8">\n\t<title>Server Error | Slack</title>\n\t<meta name="author" content="Slack">\n\t<style></style>\n</head>\n<body>\n\t<nav class="top persistent">\n\t\t<a href="https://status.slack.com/" class="logo" data-qa="logo"></a>\n\t</nav>\n\t<div id="page">\n\t\t<div id="page_contents">\n\t\t\t<h1>\n\t\t\t\t<svg width="30px" height="27px" viewBox="0 0 60 54" class="warning_icon"><path d="" fill="#D94827"/></svg>\n\t\t\t\tServer Error\n\t\t\t</h1>\n\t\t\t<div class="card">\n\t\t\t\t<p>It seems like there’s a problem connecting to our servers, and we’re investigating the issue.</p>\n\t\t\t\t<p>Please <a href="https://status.slack.com/">check our Status page for updates</a>.</p>\n\t\t\t</div>\n\t\t</div>\n\t</div>\n\t<script type="text/javascript">\n\t\tif (window.desktop) {\n\t\t\tdocument.documentElement.className = \'desktop\';\n\t\t}\n\n\t\tvar FIVE_MINS = 5 * 60 * 1000;\n\t\tvar TEN_MINS = 10 * 60 * 1000;\n\n\t\tfunction randomBetween(min, max) {\n\t\t\treturn Math.floor(Math.random() * (max - (min + 1))) + min;\n\t\t}\n\n\t\twindow.setTimeout(function () {\n\t\t\twindow.location.reload(true);\n\t\t}, randomBetween(FIVE_MINS, TEN_MINS));\n\t</script>\n</body>\n</html>'
+
     def is_valid_user_agent(self):
         user_agent = self.headers["User-Agent"]
         return self.pattern_for_language.search(
@@ -52,8 +54,11 @@ def do_POST(self):
 
             if self.path == "/error":
                 self.send_response(HTTPStatus.INTERNAL_SERVER_ERROR)
-                self.set_common_headers()
-                self.wfile.write("error".encode("utf-8"))
+                # no charset here is intentional for testing
+                self.send_header("content-type", "text/html")
+                self.send_header("connection", "close")
+                self.end_headers()
+                self.wfile.write(self.error_html_response_body.encode("utf-8"))
                 self.wfile.close()
                 return
 

diff --git a/tests/webhook/test_webhook.py b/tests/webhook/test_webhook.py
@@ -160,7 +160,7 @@ def test_error_response(self):
         client = WebhookClient(url="http://localhost:8888/error")
         resp: WebhookResponse = client.send_dict({"text": "hello!"})
         self.assertEqual(500, resp.status_code)
-        self.assertEqual("error", resp.body)
+        self.assertTrue(resp.body.startswith("<!DOCTYPE html>"))
 
     def test_proxy_issue_714(self):
         client = WebhookClient(