qs and query in request_history are converted to lowercase

Bug #1584008 reported by Tim Preece
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
requests-mock
Fix Released
Low
Jamie Lennox

Bug Description

# Hi, I have noticed that qs and query in the requests_history seem to be converted to lowercase ...
# The following test fails.
# Using requests-mock 1.0.0

import unittest

import requests
import requests_mock

class TestRequestsMock(unittest.TestCase):
    def setUp(self):
        self.site = 'http://somewhere.com/'
        self.key = 'cursorMark'
        self.value = 'AoEpVkRCREIxQTE2'

    def test_request_history_qs_matches_query_in_request(self):
        with requests_mock.Mocker() as m:
            m.register_uri('GET', self.site, text='resp')
            requests.get('{}?{}={}'.format(self.site, self.key, self.value))
        self.assertDictEqual(
            m.request_history[0].qs,
            {self.key: [self.value]}
        )

    def test_request_history_query_matches_query_in_request(self):
        with requests_mock.Mocker() as m:
            m.register_uri('GET', self.site, text='resp')
            requests.get('{}?{}={}'.format(self.site, self.key, self.value))
        self.assertEqual(
            m.request_history[0].query,
            '{}={}'.format(self.key, self.value)
        )

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

# > $ python atest.py
# FF
# ======================================================================
# FAIL: test_request_history_qs_matches_query_in_request (__main__.TestRequestsMock)
# ----------------------------------------------------------------------
# Traceback (most recent call last):
# File "atest.py", line 21, in test_request_history_qs_matches_query_in_request
# {self.key: [self.value]}
# AssertionError: {'cursormark': ['aoepvkrcreixqte2']} != {'cursorMark': ['AoEpVkRCREIxQTE2']}
# - {'cursormark': ['aoepvkrcreixqte2']}
# + {'cursorMark': ['AoEpVkRCREIxQTE2']}

# ======================================================================
# FAIL: test_request_history_query_matches_query_in_request (__main__.TestRequestsMock)
# ----------------------------------------------------------------------
# Traceback (most recent call last):
# File "atest.py", line 30, in test_request_history_query_matches_query_in_request
# '{}={}'.format(self.key, self.value)
# AssertionError: 'cursormark=aoepvkrcreixqte2' != 'cursorMark=AoEpVkRCREIxQTE2'

# ----------------------------------------------------------------------
# Ran 2 tests in 0.011s

# FAILED (failures=2)

Changed in requests-mock:
importance: Undecided → Low
status: New → Confirmed
Revision history for this message
Jamie Lennox (jamielennox) wrote :

So i confirmed this is the case, but i'll need to have a think what to do about it. It seems to be the result of using functions intended for the server side on the client side.

requests-mock uses the standard urllib.parse_qs [1] function for creating this dictionary. Ideally this would be a CaseInsensitiveDict like requests uses for headers. Changing this would be backwards incompatible, though we could figure out a way to expose it.

I'm guessing that the reason parse_qs converts it to lowercase is because these parts of uris are expected to be case insensitive and represented as lower case [2] but i'm certainly not an expert in this area and would be happy to be informed here.

There's nothing from urllib3 that can help, but i've confirmed that if we use the following methods from uritools [3] we get case sensitive results:

uritools.urisplit(m.last_request.url).getquery()
uritools.urisplit(m.last_request.url).getquerydict()

however doing a straw poll on my current fedora machine this is not packaged in the repos so i'm not sure i would want a dependency like that.

Any other suggestions are welcome.

[1] https://github.com/openstack/requests-mock/blob/780eb4b4fe25dddb802b5bfb39f846d7fa60ea9a/requests_mock/adapter.py#L76-L80
[2] http://tools.ietf.org/html/rfc3986#section-6.2.2.1
[3] http://pythonhosted.org/uritools

Revision history for this message
Ian Cordasco (icordasc) wrote :

rfc3986 should be packaged already. That said, I don't think it provides anything to parse the query string for you. I would be open to a PR for that though.

Revision history for this message
Adrian Rogers (bridgenut) wrote :

Is there a chance this will be looked at any time soon.

I also struggled with this today and will have to find a solution elsewhere as it's not just lowercasing the keys but the values too meaning that I can't accurately check that the right things are being sent.

Revision history for this message
Adrian Rogers (bridgenut) wrote :

Apologies if I've missed something here but parse_qs (for me at least with python 2.7.8) seems to give case sensitive results:

>>> urlparse.parse_qs('a=1')
{'a': ['1']}
>>> urlparse.parse_qs('a=1A')
{'a': ['1A']}
>>> urlparse.parse_qs('B=1A')
{'B': ['1A']}

Surely https://github.com/openstack/requests-mock/blob/780eb4b4fe25dddb802b5bfb39f846d7fa60ea9a/requests_mock/adapter.py#L55 is where the problem lies as it's explicitly calling .lower() on the url. Could that bit not be changed?

Revision history for this message
Jamie Lennox (jamielennox) wrote :

Hey, it so happens i've been looking back at requests-mock and i just found (independantly) that you're exactly right. The problem is that i explicity lowercase the url before parsing it.

I've got no idea why i thought that was a good idea and there are no tests verifying that behaviour.

This is something I'd love to change, i 'm just wondering if there's a compatibility issue here and how we could get around it.

Changed in requests-mock:
milestone: none → 1.1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to requests-mock (master)

Fix proposed to branch: master
Review: https://review.openstack.org/360838

Changed in requests-mock:
assignee: nobody → Jamie Lennox (jamielennox)
status: Confirmed → In Progress
Revision history for this message
Jamie Lennox (jamielennox) wrote :

Ok, I got into it properly.

The reason its doing this is that the request object that is showing up in request_history/last_request is the same one that the matcher uses internally to decide whether a mock matches the request.

At some point I have lowercased both the incoming request URL and the matcher URL to do case insensitive comparisons. This also raises a problem in that

register_uri('GET', '/abC')

and

register_uri('GET', '/aBC')

both match the same request - which they probably shouldn't.

I've posted a fix that will work and preserves the existing behaviour today. It'll be linked by the bot. If you can have a look and tell me your thoughts i'd appreciate it.

The short version is you should be able to do:

requests_mock.mock.case_sensitive = True

once somewhere at the beginning of your test files and it will globally turn on case sensitive matching, which will also turn on case sensitive history. It's unlikely that case sensitive matching will give you any problems.

As part of case sensitive matching i left the protocol and netloc matching as case insensitive as they are supposed to be case insensitive. I think this is the right thing to do, but opinions welcome.

Thanks for chasing this up again and sorry i incorrectly wrote it off as a python problem to begin with. I was convinced at the time that it was out of my control without basically reimplementing query string parsing (not that hard, but not the right place for it).

Revision history for this message
Jamie Lennox (jamielennox) wrote :

My plan was to cut a new 1.1 release early next week sometime anyway so we should be able to include this fix then.

Revision history for this message
Adrian Rogers (bridgenut) wrote :

That sounds great, thank you!

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to requests-mock (master)

Reviewed: https://review.openstack.org/360838
Committed: https://git.openstack.org/cgit/openstack/requests-mock/commit/?id=1b08dcc70557b2d58c56a923e6d3176c2b64a14f
Submitter: Jenkins
Branch: master

commit 1b08dcc70557b2d58c56a923e6d3176c2b64a14f
Author: Jamie Lennox <email address hidden>
Date: Fri Aug 26 10:54:27 2016 +1000

    Enable case sensitive matching

    When matching URLs both strings are always lowercased to provide case
    insensitive matching. Whilst this makes sense for the protocol and the
    host names it does not necessarily hold true for paths and query
    strings.

    A byproduct of this is that the lowercased strings are being reported in
    request_history which makes it harder to verify requests you made.

    We enable globally and per adapter setting case sensitive matching. This
    is intended to become the default in future releases.

    Change-Id: I7bde70a52995ecf31a0eaeff96f2823a1a6682b2
    Closes-Bug: #1584008

Changed in requests-mock:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.