jenkins-bot has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/634829 )

Change subject: [bugfix] fix http_tests.LiveFakeUserAgentTestCase ......................................................................

[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(-)

Approvals: Xqt: Looks good to me, approved jenkins-bot: Verified

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):