Skip to content

Commit c66c939

Browse files
karlcowkarlcow
committed
Merge pull request #1032 from /issues/1030/1
Fixes #1030. More robust handling for UA parsing methods.
2 parents 1e523d4 + ea6d265 commit c66c939

File tree

3 files changed

+138
-77
lines changed

3 files changed

+138
-77
lines changed

‎tests/test_helpers.py

+80-31
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,14 @@
1515

1616
import webcompat
1717
from webcompat.helpers import format_link_header
18+
from webcompat.helpers import get_browser_name
19+
from webcompat.helpers import get_browser
20+
from webcompat.helpers import get_os
1821
from webcompat.helpers import normalize_api_params
1922
from webcompat.helpers import parse_link_header
2023
from webcompat.helpers import rewrite_and_sanitize_link
2124
from webcompat.helpers import rewrite_links
2225
from webcompat.helpers import sanitize_link
23-
from webcompat.helpers import get_browser_name
24-
from webcompat.helpers import get_browser
2526

2627

2728
ACCESS_TOKEN_LINK = '<https://api.github.com/repositories/17839063/issues?per_page=50&page=3&access_token=12345>; rel="next", <https://api.github.com/repositories/17839063/issues?access_token=12345&per_page=50&page=4>; rel="last", <https://api.github.com/repositories/17839063/issues?per_page=50&access_token=12345&page=1>; rel="first", <https://api.github.com/repositories/17839063/issues?per_page=50&page=1&access_token=12345>; rel="prev"' # nopep8
@@ -30,12 +31,16 @@
3031
REWRITTEN_ISSUES_LINK_HEADER = '</api/issues?per_page=50&page=3>; rel="next", </api/issues?per_page=50&page=4>; rel="last", </api/issues?per_page=50&page=1>; rel="first", </api/issues?per_page=50&page=1>; rel="prev"' # nopep8
3132
REWRITTEN_SEARCH_LINK_HEADER = '</api/issues/search?q=taco&page=2>; rel="next", </api/issues/search?q=taco&page=26>; rel="last"' # nopep8
3233
PARSED_LINKED_HEADERS = [{'link': 'https://api.github.com/repositories/17839063/issues?per_page=50&page=3', 'rel': 'next'}, {'link': 'https://api.github.com/repositories/17839063/issues?per_page=50&page=4', 'rel': 'last'}, {'link': 'https://api.github.com/repositories/17839063/issues?per_page=50&page=1', 'rel': 'first'}, {'link': 'https://api.github.com/repositories/17839063/issues?per_page=50&page=1', 'rel': 'prev'}] # nopep8
33-
NON_TABLET_UA = "Mozilla/5.0 (Android; Mobile; rv:40.0) Gecko/40.0 Firefox/40.0" # nopep8
34-
TABLET_UA = "Mozilla/5.0 (Android 4.4; Tablet; rv:41.0) Gecko/41.0 Firefox/41.0" # nopep8
35-
PARSED_NON_TABLET_BROWSER_NAME = "firefox mobile"
36-
PARSED_TABLET_BROWSER_NAME = "firefox mobile tablet"
37-
PARSED_NON_TABLET_BROWSER = "Firefox Mobile 40.0"
38-
PARSED_TABLET_BROWSER = "Firefox Mobile 41.0 (Tablet)"
34+
FIREFOX_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:48.0) Gecko/20100101 Firefox/48.0' # nopep8
35+
FIREFOX_MOBILE_UA_OLD = 'Mozilla/5.0 (Android; Mobile; rv:40.0) Gecko/40.0 Firefox/40.0' # nopep8
36+
FIREFOX_MOBILE_UA = 'Mozilla/5.0 (Android 6.0.1; Mobile; rv:40.0) Gecko/40.0 Firefox/40.0' # nopep8
37+
FIREFOX_TABLET_UA = 'Mozilla/5.0 (Android 4.4; Tablet; rv:41.0) Gecko/41.0 Firefox/41.0' # nopep8
38+
SAFARI_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11) AppleWebKit/601.1.39 (KHTML, like Gecko) Version/9.0 Safari/601.1.39' # nopep8
39+
SAFARI_MOBILE_UA = 'Mozilla/5.0 (iPhone; CPU iPhone OS 6_1_4 like Mac OS X) AppleWebKit/536.26 (KHTML, like Gecko) Version/6.0 Mobile/10B350 Safari/8536.25' # nopep8
40+
SAFARI_TABLET_UA = 'Mozilla/5.0 (iPad; CPU OS 5_1_1 like Mac OS X) AppleWebKit/534.46 (KHTML, like Gecko) Version/5.1 Mobile/9B206 Safari/7534.48.3' # nopep8
41+
CHROME_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2720.0 Safari/537.36' # nopep8
42+
CHROME_MOBILE_UA = 'Mozilla/5.0 (Linux; Android 4.0.4; Galaxy Nexus Build/IMM76B) AppleWebKit/535.19 (KHTML, like Gecko) Chrome/18.0.1025.133 Mobile Safari/535.19' # nopep8
43+
CHROME_TABLET_UA = 'Mozilla/5.0 (Linux; Android 4.0.4; Galaxy Nexus Build/IMM76B) AppleWebKit/535.19 (KHTML, like Gecko) Chrome/18.0.1025.133 Safari/535.19' # nopep8
3944

4045

4146
class TestHelpers(unittest.TestCase):
@@ -117,29 +122,73 @@ def test_format_http_link_headers(self):
117122
link_header = GITHUB_ISSUES_LINK_HEADER
118123
self.assertEqual(format_link_header(parsed_headers), link_header)
119124

120-
def test_get_browser_name_Tablet(self):
121-
'''Test Browser name parsing for tablet devices.'''
122-
user_agent = TABLET_UA
123-
parsed_browser_name = PARSED_TABLET_BROWSER_NAME
124-
self.assertEqual(get_browser_name(user_agent), parsed_browser_name)
125-
126-
def test_get_browser_name_Non_Tablet(self):
127-
'''Test Browser name parsing for non-tablet devices.'''
128-
user_agent = NON_TABLET_UA
129-
parsed_browser_name = PARSED_NON_TABLET_BROWSER_NAME
130-
self.assertEqual(get_browser_name(user_agent), parsed_browser_name)
131-
132-
def test_get_browser_Tablet(self):
133-
'''Test Browser parsing for tablet devices.'''
134-
user_agent = TABLET_UA
135-
parsed_browser = PARSED_TABLET_BROWSER
136-
self.assertEqual(get_browser(user_agent), parsed_browser)
137-
138-
def test_get_browser_Non_Tablet(self):
139-
'''Test Browser parsing for non-tablet devices.'''
140-
user_agent = NON_TABLET_UA
141-
parsed_browser = PARSED_NON_TABLET_BROWSER
142-
self.assertEqual(get_browser(user_agent), parsed_browser)
125+
def test_get_browser_name(self):
126+
'''Test browser name parsing via get_browser_name helper method.'''
127+
self.assertEqual(get_browser_name(FIREFOX_UA), 'firefox')
128+
self.assertEqual(get_browser_name(FIREFOX_MOBILE_UA), 'firefox mobile')
129+
self.assertEqual(get_browser_name(FIREFOX_MOBILE_UA_OLD),
130+
'firefox mobile')
131+
self.assertEqual(get_browser_name(FIREFOX_TABLET_UA),
132+
'firefox mobile (tablet)')
133+
self.assertEqual(get_browser_name(SAFARI_UA), 'safari')
134+
self.assertEqual(get_browser_name(SAFARI_MOBILE_UA), 'mobile safari')
135+
self.assertEqual(get_browser_name(SAFARI_TABLET_UA), 'mobile safari')
136+
self.assertEqual(get_browser_name(CHROME_UA), 'chrome')
137+
self.assertEqual(get_browser_name(CHROME_MOBILE_UA), 'chrome mobile')
138+
self.assertEqual(get_browser_name(CHROME_TABLET_UA), 'chrome')
139+
self.assertEqual(get_browser_name(''), 'unknown')
140+
self.assertEqual(get_browser_name(None), 'unknown')
141+
self.assertEqual(get_browser_name(), 'unknown')
142+
self.assertEqual(get_browser_name(u'💀'), 'unknown')
143+
self.assertEqual(get_browser('<script>lol()</script>'), 'Unknown')
144+
self.assertEqual(get_browser(True), 'Unknown')
145+
self.assertEqual(get_browser(False), 'Unknown')
146+
self.assertEqual(get_browser(None), 'Unknown')
147+
148+
def test_get_browser(self):
149+
'''Test browser parsing via get_browser helper method.'''
150+
self.assertEqual(get_browser(FIREFOX_UA), 'Firefox 48.0')
151+
self.assertEqual(get_browser(FIREFOX_MOBILE_UA), 'Firefox Mobile 40.0')
152+
self.assertEqual(get_browser_name(FIREFOX_MOBILE_UA_OLD),
153+
'firefox mobile')
154+
self.assertEqual(get_browser(FIREFOX_TABLET_UA),
155+
'Firefox Mobile (Tablet) 41.0')
156+
self.assertEqual(get_browser(SAFARI_UA), 'Safari 9.0')
157+
self.assertEqual(get_browser(SAFARI_MOBILE_UA), 'Mobile Safari 6.0')
158+
self.assertEqual(get_browser(SAFARI_TABLET_UA), 'Mobile Safari 5.1')
159+
self.assertEqual(get_browser(CHROME_UA), 'Chrome 52.0.2720')
160+
self.assertEqual(get_browser(CHROME_MOBILE_UA),
161+
'Chrome Mobile 18.0.1025')
162+
self.assertEqual(get_browser(CHROME_TABLET_UA), 'Chrome 18.0.1025')
163+
self.assertEqual(get_browser(''), 'Unknown')
164+
self.assertEqual(get_browser(), 'Unknown')
165+
self.assertEqual(get_browser(u'💀'), 'Unknown')
166+
self.assertEqual(get_browser('<script>lol()</script>'), 'Unknown')
167+
self.assertEqual(get_browser(True), 'Unknown')
168+
self.assertEqual(get_browser(False), 'Unknown')
169+
self.assertEqual(get_browser(None), 'Unknown')
170+
171+
def test_get_os(self):
172+
'''Test OS parsing via get_os helper method.'''
173+
self.assertEqual(get_os(FIREFOX_UA), 'Mac OS X 10.11')
174+
self.assertEqual(get_os(FIREFOX_MOBILE_UA), 'Android 6.0.1')
175+
self.assertEqual(get_os(FIREFOX_MOBILE_UA_OLD), 'Android')
176+
self.assertEqual(get_os(FIREFOX_TABLET_UA), 'Android 4.4')
177+
self.assertEqual(get_os(SAFARI_UA), 'Mac OS X 10.11')
178+
self.assertEqual(get_os(SAFARI_MOBILE_UA), 'iOS 6.1.4')
179+
self.assertEqual(get_os(SAFARI_TABLET_UA), 'iOS 5.1.1')
180+
self.assertEqual(get_os(CHROME_UA), 'Mac OS X 10.11.4')
181+
self.assertEqual(get_os(CHROME_MOBILE_UA),
182+
'Android 4.0.4')
183+
self.assertEqual(get_os(CHROME_TABLET_UA), 'Android 4.0.4')
184+
self.assertEqual(get_os(''), 'Unknown')
185+
self.assertEqual(get_os(), 'Unknown')
186+
self.assertEqual(get_os(u'💀'), 'Unknown')
187+
self.assertEqual(get_os('<script>lol()</script>'), 'Unknown')
188+
self.assertEqual(get_os(True), 'Unknown')
189+
self.assertEqual(get_os(False), 'Unknown')
190+
self.assertEqual(get_os(None), 'Unknown')
191+
143192

144193
if __name__ == '__main__':
145194
unittest.main()

‎webcompat/helpers.py

+55-45
Original file line numberDiff line numberDiff line change
@@ -99,61 +99,71 @@ def get_user_info():
9999
session['avatar_url'] = gh_user.get('avatar_url')
100100

101101

102-
def get_browser_name(user_agent_string):
103-
'''Return just the browser name.
104-
105-
unknown user agents will be reported as "other".
106-
'''
107-
ua_dict = user_agent_parser.Parse(user_agent_string)
108-
name = ua_dict.get('user_agent').get('family').lower()
109-
device = ua_dict.get('device').get('model')
110-
if (name == 'firefox mobile' and
111-
ua_dict.get('os').get('family') == 'Firefox OS'):
112-
name = 'other'
113-
if device == 'Tablet':
114-
name += " " + device.lower()
115-
return name
116-
117-
118-
def get_browser(user_agent_string):
102+
def get_browser(user_agent_string=None):
119103
'''Return browser name family and version.
120104
121105
It will pre-populate the bug reporting form.
122106
'''
123-
ua_dict = user_agent_parser.Parse(user_agent_string)
124-
ua = ua_dict.get('user_agent')
125-
name = ua.get('family')
126-
version = ua.get('major', u'Unknown')
127-
# Add on the minor and patch version numbers if they exist
128-
if version != u'Unknown' and ua.get('minor'):
129-
version = version + "." + ua.get('minor')
130-
if ua.get('patch'):
131-
version = version + "." + ua.get('patch')
132-
else:
133-
version = ''
134-
# Check for tablet devices
135-
if ua_dict.get('device').get('model') == 'Tablet':
136-
model = ' (Tablet)'
137-
else:
138-
model = ''
139-
return '{0} {1}{2}'.format(name, version, model)
107+
if user_agent_string and isinstance(user_agent_string, basestring):
108+
ua_dict = user_agent_parser.Parse(user_agent_string)
109+
ua = ua_dict.get('user_agent')
110+
name = ua.get('family')
111+
version = ua.get('major', u'Unknown')
112+
# Add on the minor and patch version numbers if they exist
113+
if version != u'Unknown' and ua.get('minor'):
114+
version = version + "." + ua.get('minor')
115+
if ua.get('patch'):
116+
version = version + "." + ua.get('patch')
117+
else:
118+
version = ''
119+
# Check for tablet devices
120+
if ua_dict.get('device').get('model') == 'Tablet':
121+
model = '(Tablet) '
122+
else:
123+
model = ''
124+
rv = '{0} {1}{2}'.format(name, model, version)
125+
# bizarre UA strings can be parsed like so:
126+
# {'major': None, 'minor': None, 'family': 'Other', 'patch': None}
127+
# but we want to return "Unknown", rather than "Other"
128+
if rv.strip().lower() == "other":
129+
return "Unknown"
130+
return rv
131+
return "Unknown"
132+
133+
134+
def get_browser_name(user_agent_string=None):
135+
'''Return just the browser name.
136+
137+
unknown user agents will be reported as "unknown".
138+
'''
139+
if user_agent_string and isinstance(user_agent_string, basestring):
140+
# get_browser will return something like 'Chrome Mobile 47.0'
141+
# we just want 'chrome mobile', i.e., the lowercase name
142+
# w/o the version
143+
return get_browser(user_agent_string).rsplit(' ', 1)[0].lower()
144+
return "unknown"
140145

141146

142-
def get_os(user_agent_string):
147+
def get_os(user_agent_string=None):
143148
'''Return operating system name.
144149
145150
It pre-populates the bug reporting form.
146151
'''
147-
ua_dict = user_agent_parser.Parse(user_agent_string)
148-
os = ua_dict.get('os')
149-
version = os.get('major', u'Unknown')
150-
if version != u'Unknown' and os.get('major'):
151-
version = version + "." + os.get('minor')
152-
if os.get('patch'):
153-
version = version + "." + os.get('patch')
154-
else:
155-
version = ''
156-
return '{0} {1}'.format(os.get('family'), version)
152+
if user_agent_string and isinstance(user_agent_string, basestring):
153+
ua_dict = user_agent_parser.Parse(user_agent_string)
154+
os = ua_dict.get('os')
155+
version = os.get('major', u'Unknown')
156+
if version != u'Unknown' and os.get('minor'):
157+
version = version + "." + os.get('minor')
158+
if os.get('patch'):
159+
version = version + "." + os.get('patch')
160+
else:
161+
version = ''
162+
rv = '{0} {1}'.format(os.get('family'), version).rstrip()
163+
if rv.strip().lower() == "other":
164+
return "Unknown"
165+
return rv
166+
return "Unknown"
157167

158168

159169
def get_response_headers(response):

‎webcompat/views.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ def index():
138138
ua_header = request.headers.get('User-Agent')
139139
bug_form.browser.data = get_browser(ua_header)
140140
bug_form.os.data = get_os(ua_header)
141+
# browser_name is used in topbar.html to show the right add-on link
141142
browser_name = get_browser_name(ua_header)
142143
# GET means you want to file a report.
143144
if request.method == 'GET':
@@ -151,7 +152,8 @@ def index():
151152

152153
else:
153154
# Validation failed, re-render the form with the errors.
154-
return render_template('index.html', form=bug_form)
155+
return render_template('index.html', form=bug_form,
156+
browser=browser_name)
155157

156158

157159
@app.route('/issues')

0 commit comments

Comments
 (0)