jenkins-bot submitted this change.

View Change

[bugfix] fix http_tests.LiveFakeUserAgentTestCase

Fix handling of User Agent in http.fetch(), making logic more explicit:
- now http.fetch() raises ValueError on an empty parameter

Also:
- added suppress_warning for deprecated get_fake_user_agent() tests.
- renamed loffer to 'comms.http'

Bug: T265842
Change-Id: I94ef2e0b095b85f3301f7b2933c629dbfd18d04e
---
M pywikibot/comms/http.py
M tests/http_tests.py
2 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/pywikibot/comms/http.py b/pywikibot/comms/http.py
index fc201d7..34b4103 100644
--- a/pywikibot/comms/http.py
+++ b/pywikibot/comms/http.py
@@ -52,7 +52,7 @@
 # 'certificate verify failed' is a commonly detectable string
 SSL_CERT_VERIFY_FAILED_MSG = 'certificate verify failed'
 
-_logger = 'comm.http'
+_logger = 'comms.http'
 
 cookie_file_path = config.datafilepath('pywikibot.lwp')
 file_mode_checker(cookie_file_path, create=True)
@@ -446,21 +446,35 @@
     headers = headers or {}
     headers.update(config.extra_headers.copy() or {})
 
-    if headers.get('user-agent', False):
-        user_agent_format_string = headers.get('user-agent')
-        if not user_agent_format_string or '{' in user_agent_format_string:
-            headers['user-agent'] = user_agent(None, user_agent_format_string)
-    else:
-        # if not already specified,
-        # get fake UA exceptions from `fake_user_agent_exceptions` config.
+    def assign_fake_user_agent(use_fake_user_agent, uri):
         uri_domain = urlparse(uri).netloc
         use_fake_user_agent = config.fake_user_agent_exceptions.get(
             uri_domain, use_fake_user_agent)
 
+        if use_fake_user_agent is False:
+            return user_agent()
+        if use_fake_user_agent is True:
+            return fake_user_agent()
         if use_fake_user_agent and isinstance(use_fake_user_agent, str):
-            headers['user-agent'] = use_fake_user_agent  # Custom UA.
-        elif use_fake_user_agent is True:
-            headers['user-agent'] = fake_user_agent()
+            return use_fake_user_agent  # Custom UA.
+        raise ValueError('Invalid parameter: '
+                         'use_fake_user_agent={}'.format(use_fake_user_agent))
+
+    def assign_user_agent(user_agent_format_string):
+        if not user_agent_format_string or '{' in user_agent_format_string:
+            return user_agent(None, user_agent_format_string)
+        else:
+            # do nothing, it is already a UA
+            return user_agent_format_string
+
+    # If not already specified.
+    if 'user-agent' not in headers:
+        # Get fake UA exceptions from `fake_user_agent_exceptions` config.
+        headers['user-agent'] = assign_fake_user_agent(use_fake_user_agent,
+                                                       uri)
+    # Already specified.
+    else:
+        headers['user-agent'] = assign_user_agent(headers.get('user-agent'))
 
     callbacks = kwargs.pop('callbacks', [])
     callback = kwargs.pop('callback', None)
diff --git a/tests/http_tests.py b/tests/http_tests.py
index 9a8b0b7..3955dab 100644
--- a/tests/http_tests.py
+++ b/tests/http_tests.py
@@ -338,6 +338,18 @@
             use_fake_user_agent='ARBITRARY')
         self.assertEqual(r.headers['user-agent'], 'ARBITRARY')
 
+        # Empty value
+        self.assertRaisesRegex(ValueError,
+                               'Invalid parameter: use_fake_user_agent',
+                               http.fetch, self.get_httpbin_url('/status/200'),
+                               use_fake_user_agent='')
+
+        # Parameter wrongly set to None
+        self.assertRaisesRegex(ValueError,
+                               'Invalid parameter: use_fake_user_agent',
+                               http.fetch, self.get_httpbin_url('/status/200'),
+                               use_fake_user_agent=None)
+
         # Manually overridden domains
         config.fake_user_agent_exceptions = {
             self.get_httpbin_hostname(): 'OVERRIDDEN'}
@@ -369,17 +381,18 @@
 
     def _test_config_settings(self):
         """Test if method honours configuration toggle."""
-        # ON: True and None in config are considered turned on.
-        config.fake_user_agent = True
-        self.assertNotEqual(http.get_fake_user_agent(), http.user_agent())
-        config.fake_user_agent = None
-        self.assertNotEqual(http.get_fake_user_agent(), http.user_agent())
+        with suppress_warnings(r'.*?get_fake_user_agent is deprecated'):
+            # ON: True and None in config are considered turned on.
+            config.fake_user_agent = True
+            self.assertNotEqual(http.get_fake_user_agent(), http.user_agent())
+            config.fake_user_agent = None
+            self.assertNotEqual(http.get_fake_user_agent(), http.user_agent())
 
-        # OFF: All other values won't make it return random UA.
-        config.fake_user_agent = False
-        self.assertEqual(http.get_fake_user_agent(), http.user_agent())
-        config.fake_user_agent = 'ARBITRARY'
-        self.assertEqual(http.get_fake_user_agent(), 'ARBITRARY')
+            # OFF: All other values won't make it return random UA.
+            config.fake_user_agent = False
+            self.assertEqual(http.get_fake_user_agent(), http.user_agent())
+            config.fake_user_agent = 'ARBITRARY'
+            self.assertEqual(http.get_fake_user_agent(), 'ARBITRARY')
 
     @require_modules('fake_useragent')
     def test_with_fake_useragent(self):

To view, visit change 634829. To unsubscribe, or for help writing mail filters, visit settings.