parse_user_agent: Assert user agent is not None.

This commit asserts that parse_user_agent never returns None. The
RegEx will match any string, so that `match` is never None. This
brings test coverage of lib/user_agent.py to 100%. Changes were also
made in test/test_decorators.py and views/compatibility.py to reflect
that parse_user_agent cannot return None.

Improves: #7089.
Fixes: #8779.
This commit is contained in:
Jack Weatherilt
2018-03-22 19:02:15 +00:00
committed by Tim Abbott
parent 66b007acf0
commit 8535625341
3 changed files with 4 additions and 11 deletions

View File

@@ -1,11 +1,10 @@
import re
from typing import Optional, Dict
from typing import Dict
# Warning: If you change this parsing, please test using
# zerver/tests/test_decorators.py
# And extend zerver/fixtures/user_agents_unique with any new test cases
def parse_user_agent(user_agent: str) -> Optional[Dict[str, str]]:
def parse_user_agent(user_agent: str) -> Dict[str, str]:
match = re.match("^(?P<name>[^/ ]*[^0-9/(]*)(/(?P<version>[^/ ]*))?([ /].*)?$", user_agent)
if match is None:
return None
assert match is not None
return match.groupdict()

View File

@@ -1315,7 +1315,6 @@ class TestUserAgentParsing(ZulipTestCase):
"""Test for our user agent parsing logic, using a large data set."""
user_agents_parsed = defaultdict(int) # type: Dict[str, int]
user_agents_path = os.path.join(settings.DEPLOY_ROOT, "zerver/fixtures/user_agents_unique")
parse_errors = []
for line in open(user_agents_path).readlines():
line = line.strip()
match = re.match('^(?P<count>[0-9]+) "(?P<user_agent>.*)"$', line)
@@ -1324,13 +1323,8 @@ class TestUserAgentParsing(ZulipTestCase):
count = groupdict["count"]
user_agent = groupdict["user_agent"]
ret = parse_user_agent(user_agent)
self.assertIsNotNone(ret)
if ret is None: # nocoverage
parse_errors.append(line)
continue
user_agents_parsed[ret["name"]] += int(count)
self.assertEqual(len(parse_errors), 0)
class TestIgnoreUnhashableLRUCache(ZulipTestCase):
def test_cache_hit(self) -> None:

View File

@@ -7,6 +7,6 @@ from zerver.lib.user_agent import parse_user_agent
def check_compatibility(request: HttpRequest) -> HttpResponse:
user_agent = parse_user_agent(request.META["HTTP_USER_AGENT"])
if user_agent is None or user_agent['name'] == "ZulipInvalid":
if user_agent['name'] == "ZulipInvalid":
return json_error("Client is too old")
return json_success()