Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

webhooks: Replace headers parsing logic with a common source. 

When parsing custom HTTP headers in the integrations dev panel, http
headers from fixtures system and the send_webhook_fixture_message
we now use a singular source of logic: standardize_headers which
will take care of converting a dictionary of input headers into a
standard form that Django expects.
This commit is contained in:
Hemanth V. Alluri 2019-06-21 08:11:30 +05:30 committed by Tim Abbott
parent 50d43902fb
commit e2549b3b84
7 changed files with 52 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
zerver/lib/test_classes.py
View File

@ -23,7 +23,7 @@ from zerver.lib.initial_password import initial_password
from zerver.lib.utils import is_remote_server
from zerver.lib.users import get_api_key
from zerver.lib.sessions import get_session_dict_user
from zerver.lib.webhooks.common import get_fixture_http_headers, parse_headers_dict
from zerver.lib.webhooks.common import get_fixture_http_headers, standardize_headers
from zerver.lib.actions import (
check_send_message, create_stream_if_needed, bulk_add_subscriptions,
@ -753,7 +753,7 @@ class WebhookTestCase(ZulipTestCase):
if content_type is not None:
kwargs['content_type'] = content_type
headers = get_fixture_http_headers(self.FIXTURE_DIR_NAME, fixture_name)
headers = parse_headers_dict(headers)
headers = standardize_headers(headers)
kwargs.update(headers)
msg = self.send_json_payload(self.test_user, self.url, payload,
self.STREAM_NAME, **kwargs)
@ -769,7 +769,7 @@ class WebhookTestCase(ZulipTestCase):
if content_type is not None:
kwargs['content_type'] = content_type
headers = get_fixture_http_headers(self.FIXTURE_DIR_NAME, fixture_name)
headers = parse_headers_dict(headers)
headers = standardize_headers(headers)
kwargs.update(headers)
sender = kwargs.get('sender', self.test_user)
msg = self.send_json_payload(sender, self.url, payload,

34
zerver/lib/webhooks/common.py
View File

@ -97,21 +97,27 @@ def check_send_webhook_message(
# webhook-errors.log
pass
def parse_headers_dict(custom_headers: Union[None, Dict[str, Any]]) -> Dict[str, str]:
headers = {}
if not custom_headers:
def standardize_headers(input_headers: Union[None, Dict[str, Any]]) -> Dict[str, str]:
""" This method can be used to standardize a dictionary of headers with
the standard format that Django expects. For reference, refer to:
https://docs.djangoproject.com/en/2.2/ref/request-response/#django.http.HttpRequest.headers
NOTE: Historically, Django's headers were not case-insensitive. We're still
capitalizing our headers to make it easier to compare/search later if required.
"""
canonical_headers = {}
if not input_headers:
return {}
for header in custom_headers:
# Ensure what we return is in the Django HTTP headers format,
# with HTTP_ prefixes before most actual HTTP header names.
if header.startswith("HTTP_") or header.startswith("http_"):
headers[header.upper().replace("-", "_")] = str(custom_headers[header])
else:
new_header = header.upper().replace("-", "_")
if new_header not in ["CONTENT_TYPE", "CONTENT_LENGTH"]:
new_header = "HTTP_" + new_header
headers[new_header] = str(custom_headers[header])
return headers
for raw_header in input_headers:
polished_header = raw_header.upper().replace("-", "_")
if polished_header not in["CONTENT_TYPE", "CONTENT_LENGTH"]:
if not polished_header.startswith("HTTP_"):
polished_header = "HTTP_" + polished_header
canonical_headers[polished_header] = str(input_headers[raw_header])
return canonical_headers
def validate_extract_webhook_http_header(request: HttpRequest, header: str,
integration_name: str,

26
zerver/management/commands/send_webhook_fixture_message.py
View File

@ -7,28 +7,9 @@ from django.core.management.base import CommandParser
from django.test import Client
from zerver.lib.management import ZulipBaseCommand, CommandError
from zerver.lib.webhooks.common import standardize_headers
from zerver.models import get_realm
def parse_headers(custom_headers: Union[None, str]) -> Union[None, Dict[str, str]]:
""" The main aim of this method is be to convert regular HTTP headers into a format that
Django prefers. Note: This function throws a ValueError and thus it should be used in a
try/except block. """
headers = {}
if not custom_headers:
return None
custom_headers_dict = ujson.loads(custom_headers)
for header in custom_headers_dict:
if len(header.split(" ")) > 1:
raise ValueError("custom header '%s' contains a space." % (header,))
new_header = header.upper().replace("-", "_")
if new_header not in ["CONTENT_TYPE", "CONTENT_LENGTH"]:
# See https://docs.djangoproject.com/en/2.2/ref/request-response/
# for how Django formats HTTP headers.
new_header = "HTTP_" + new_header
headers[new_header] = str(custom_headers_dict[header])
return headers
class Command(ZulipBaseCommand):
help = """
Create webhook message based on given fixture
@ -70,11 +51,14 @@ approach shown above.
self.add_realm_args(parser, help="Specify which realm/subdomain to connect to; default is zulip")
def parse_headers(self, custom_headers: Union[None, str]) -> Union[None, Dict[str, str]]:
if not custom_headers:
return {}
try:
return parse_headers(custom_headers)
custom_headers_dict = ujson.loads(custom_headers)
except ValueError as ve:
raise CommandError('Encountered an error while attempting to parse custom headers: {}\n'
'Note: all strings must be enclosed within "" instead of \'\''.format(ve))
return standardize_headers(custom_headers_dict)
def handle(self, **options: str) -> None:
if options['fixture'] is None or options['url'] is None:

14
zerver/tests/test_integrations_dev_panel.py
View File

@ -16,7 +16,7 @@ class TestIntegrationsDevPanel(ZulipTestCase):
data = {
"url": url,
"body": body,
"custom_headers": "",
"custom_headers": "{}",
"is_json": True
}
@ -36,7 +36,7 @@ class TestIntegrationsDevPanel(ZulipTestCase):
data = {
"url": url,
"body": body,
"custom_headers": "",
"custom_headers": "{}",
"is_json": True
}
@ -134,7 +134,7 @@ class TestIntegrationsDevPanel(ZulipTestCase):
data = {
"url": url,
"custom_headers": "",
"custom_headers": "{}",
"integration_name": "appfollow",
}
@ -156,8 +156,8 @@ class TestIntegrationsDevPanel(ZulipTestCase):
r["message"] = ujson.loads(r["message"])
self.assertEqual(response.status_code, 200)
for r in responses:
# We have to use this roundabout manner since the order may vary each time. This is not
# an issue.
# We have to use this roundabout manner since the order may vary each time.
# This is not an issue.
self.assertTrue(r in expected_responses)
expected_responses.remove(r)
@ -178,7 +178,7 @@ class TestIntegrationsDevPanel(ZulipTestCase):
data = {
"url": url,
"custom_headers": "",
"custom_headers": "{}",
"integration_name": "wordpress",
}
@ -246,7 +246,7 @@ class TestIntegrationsDevPanel(ZulipTestCase):
url = "/api/v1/external/appfollow?api_key={key}&stream=Denmark&topic=Appfollow Bulk Notifications".format(key=bot.api_key)
data = {
"url": url,
"custom_headers": "",
"custom_headers": "{}",
"integration_name": "appfollow",
}
response = self.client_post("/devtools/integrations/send_all_webhook_fixture_messages", data)

32
zerver/tests/test_management_commands.py
View File

@ -20,7 +20,6 @@ from zerver.lib.test_runner import slow
from zerver.models import Recipient, get_user_profile_by_email, get_stream
from django.utils.timezone import now as timezone_now
from zerver.management.commands.send_webhook_fixture_message import Command
from zerver.lib.test_helpers import most_recent_message
from zerver.models import get_realm, UserProfile, Realm, Reaction, Message
from confirmation.models import RealmCreationKey, generate_realm_creation_url
@ -202,8 +201,8 @@ class TestSendWebhookFixtureMessage(TestCase):
ujson_mock: MagicMock,
client_mock: MagicMock,
os_path_exists_mock: MagicMock) -> None:
ujson_mock.loads.return_value = '{}'
ujson_mock.dumps.return_value = {}
ujson_mock.loads.return_value = {}
ujson_mock.dumps.return_value = "{}"
os_path_exists_mock.return_value = True
client = client_mock()
@ -213,34 +212,9 @@ class TestSendWebhookFixtureMessage(TestCase):
self.assertTrue(ujson_mock.dumps.called)
self.assertTrue(ujson_mock.loads.called)
self.assertTrue(open_mock.called)
client.post.assert_called_once_with(self.url, {}, content_type="application/json",
client.post.assert_called_once_with(self.url, "{}", content_type="application/json",
HTTP_HOST="zulip.testserver")
@patch('zerver.management.commands.send_webhook_fixture_message.Command._get_fixture_as_json')
@patch('zerver.management.commands.send_webhook_fixture_message.Command._does_fixture_path_exist')
@patch('zerver.management.commands.send_webhook_fixture_message.Command.parse_headers')
def test_check_post_request_with_improper_custom_header(self,
parse_headers_mock: MagicMock,
does_fixture_path_exist_mock: MagicMock,
get_fixture_as_json_mock: MagicMock) -> None:
does_fixture_path_exist_mock.return_value = True
get_fixture_as_json_mock.return_value = "{}"
improper_headers = '{"X-Custom - Headers": "some_val"}'
with self.assertRaises(CommandError):
call_command(self.COMMAND_NAME, fixture=self.fixture_path, url=self.url,
custom_headers=improper_headers)
parse_headers_mock.assert_called_once_with(improper_headers)
def test_parse_headers_method(self) -> None:
command = Command()
self.assertEqual(command.parse_headers(None), None)
self.assertEqual(command.parse_headers('{"Content-Type": "text/plain", "X-Custom-Header": "value"}'),
{"CONTENT_TYPE": "text/plain", "HTTP_X_CUSTOM_HEADER": "value"})
with self.assertRaises(CommandError):
command.parse_headers('{"X-Custom - Headers": "some_val"}')
class TestGenerateRealmCreationLink(ZulipTestCase):
COMMAND_NAME = "generate_realm_creation_link"

8
zerver/tests/test_webhooks_common.py
View File

@ -11,7 +11,7 @@ from zerver.lib.test_classes import ZulipTestCase, WebhookTestCase
from zerver.lib.webhooks.common import \
validate_extract_webhook_http_header, \
MISSING_EVENT_HEADER_MESSAGE, MissingHTTPEventHeader, \
INVALID_JSON_MESSAGE, get_fixture_http_headers, parse_headers_dict
INVALID_JSON_MESSAGE, get_fixture_http_headers, standardize_headers
from zerver.models import get_user, get_realm, UserProfile
from zerver.lib.users import get_api_key
from zerver.lib.send_email import FromAddress
@ -116,11 +116,11 @@ class WebhooksCommonTestCase(ZulipTestCase):
with self.assertRaises(AttributeError):
get_fixture_http_headers("some_integration", "simple_fixture")
def test_parse_headers_dict(self) -> None:
self.assertEqual(parse_headers_dict({}), {})
def test_standardize_headers(self) -> None:
self.assertEqual(standardize_headers({}), {})
raw_headers = {"Content-Type": "text/plain", "X-Event-Type": "ping"}
djangoified_headers = parse_headers_dict(raw_headers)
djangoified_headers = standardize_headers(raw_headers)
expected_djangoified_headers = {"CONTENT_TYPE": "text/plain", "HTTP_X_EVENT_TYPE": "ping"}
self.assertEqual(djangoified_headers, expected_djangoified_headers)

20
zerver/views/development/integrations.py
View File

@ -10,8 +10,8 @@ from zerver.lib.integrations import WEBHOOK_INTEGRATIONS
from zerver.lib.request import has_request_variables, REQ
from zerver.lib.response import json_success, json_error
from zerver.models import UserProfile, get_realm
from zerver.management.commands.send_webhook_fixture_message import parse_headers
from zerver.lib.webhooks.common import get_fixture_http_headers
from zerver.lib.webhooks.common import get_fixture_http_headers, \
standardize_headers
ZULIP_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), '../../../')
@ -30,21 +30,21 @@ def dev_panel(request: HttpRequest) -> HttpResponse:
def send_webhook_fixture_message(url: str=REQ(),
body: str=REQ(),
is_json: bool=REQ(),
custom_headers: str=REQ()) -> HttpResponse:
custom_headers_str: str=REQ()) -> HttpResponse:
client = Client()
realm = get_realm("zulip")
try:
headers = parse_headers(custom_headers)
custom_headers = ujson.loads(custom_headers_str)
except ValueError as ve:
return json_error("Custom HTTP headers are not in a valid JSON format. {}".format(ve)) # nolint
if not headers:
headers = {}
http_host = headers.pop("HTTP_HOST", realm.host)
standardized_headers = standardize_headers(custom_headers)
http_host = standardized_headers.pop("HTTP_HOST", realm.host)
if is_json:
content_type = headers.pop("HTTP_CONTENT_TYPE", "application/json")
content_type = standardized_headers.pop("HTTP_CONTENT_TYPE", "application/json")
else:
content_type = headers.pop("HTTP_CONTENT_TYPE", "text/plain")
return client.post(url, body, content_type=content_type, HTTP_HOST=http_host, **headers)
content_type = standardized_headers.pop("HTTP_CONTENT_TYPE", "text/plain")
return client.post(url, body, content_type=content_type, HTTP_HOST=http_host,
**standardized_headers)
@has_request_variables
def get_fixtures(request: HttpResponse,