Mercurial > p > roundup > code
changeset 5653:ba67e397f063
Fix string/bytes issues under python 3.
1) cgi/client.py: override cgi.FieldStorage's make_file so that file
is always created in binary/byte mode. This means that json (and
xml) are bytes not strings.
2) rest.py: try harder to find dicttoxml in roundup directory or on
sys.path. This just worked under python 2 but python 3 only
searches sys.path by default and does not search relative like
python 2.
3) rest.py: replace headers.getheader call removed from python 3 with
equivalent code.
4) rest.py: make value returned from dispatch into bytes not string.
5) test/caseinsensitivedict.py, test/test_CaseInsensitiveDict.py:
get code from stackoverflow that implements a case insensitive key
dict. So dict['foo'], dict['Foo'] are the same entry. Used for
looking up headers in mocked http rewuset header array.
6) test/rest_common.py: rework tests for etags and rest to properly
supply bytes to the called routines. Calls to s2b and b2s and use
of BytesIO and overriding make_file in cgi.FieldStorage to try to
make sure it works under python 3.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Sun, 17 Mar 2019 19:28:26 -0400 |
| parents | 9689d1bf9bb0 |
| children | 207e0f5d551c |
| files | roundup/cgi/client.py roundup/rest.py test/rest_common.py test/test_CaseInsensitiveDict.py |
| diffstat | 4 files changed, 169 insertions(+), 67 deletions(-) [+] |
line wrap: on
line diff
--- a/roundup/cgi/client.py Sun Mar 17 19:00:43 2019 -0400 +++ b/roundup/cgi/client.py Sun Mar 17 19:28:26 2019 -0400 @@ -375,7 +375,18 @@ logger.debug("Setting CONTENT_LENGTH to 0 for method: %s", self.env['REQUEST_METHOD']) + # cgi.FieldStorage must save all data as + # binary/bytes. This is needed for handling json and xml + # data blobs under python 3. Under python 2, str and binary + # are interchangable, not so under 3. + def make_file(self): + import tempfile + return tempfile.TemporaryFile("wb+") + + saved_make_file = cgi.FieldStorage.make_file + cgi.FieldStorage.make_file = make_file self.form = cgi.FieldStorage(fp=request.rfile, environ=env) + cgi.FieldStorage.make_file = saved_make_file # In some case (e.g. content-type application/xml), cgi # will not parse anything. Fake a list property in this case if self.form.list is None:
--- a/roundup/rest.py Sun Mar 17 19:00:43 2019 -0400 +++ b/roundup/rest.py Sun Mar 17 19:28:26 2019 -0400 @@ -20,9 +20,15 @@ import re try: - from dicttoxml import dicttoxml + # if dicttoxml installed in roundup directory, use it + from .dicttoxml import dicttoxml except ImportError: - dicttoxml = None + try: + # else look in sys.path + from dicttoxml import dicttoxml + except ImportError: + # else not supported + dicttoxml = None from roundup import hyperdb from roundup import date @@ -160,7 +166,8 @@ etags = [] if '@etag' in input: etags.append(input['@etag'].value); - etags.append(headers.getheader("ETag", None)) + if "ETag" in headers: + etags.append(headers["ETag"]) return etags def parse_accept_header(accept): @@ -220,7 +227,9 @@ else: media_params.append((key, value)) result.append((media_type, dict(media_params), q)) - result.sort(lambda x, y: -cmp(x[2], y[2])) + # was: result.sort(lambda x, y: -cmp(x[2], y[2])) + # change for python 3 support + result.sort(key=lambda x: x[2], reverse=True) return result @@ -1299,7 +1308,9 @@ headers = self.client.request.headers # Never allow GET to be an unsafe operation (i.e. data changing). # User must use POST to "tunnel" DELETE, PUT, OPTIONS etc. - override = headers.getheader('X-HTTP-Method-Override') + override = None + if 'X-HTTP-Method-Override' in headers: + override = headers['X-HTTP-Method-Override'] output = None if override: if method.upper() != 'GET': @@ -1314,7 +1325,9 @@ uri) # parse Accept header and get the content type - accept_header = parse_accept_header(headers.getheader('Accept')) + accept_header = [] + if 'Accept' in headers: + accept_header = parse_accept_header(headers['Accept']) accept_type = "invalid" for part in accept_header: if part[0] in self.__accepted_content_type: @@ -1350,7 +1363,9 @@ # Is there an input.value with format json data? # If so turn it into an object that emulates enough # of the FieldStorge methods/props to allow a response. - content_type_header = headers.getheader('Content-Type', None) + content_type_header = None + if 'Content-Type' in headers: + content_type_header = headers['Content-Type'] if type(input.value) == str and content_type_header: parsed_content_type_header = content_type_header # the structure of a content-type header @@ -1404,7 +1419,7 @@ # Make output json end in a newline to # separate from following text in logs etc.. - return output + "\n" + return bs2b(output + "\n") class RoundupJSONEncoder(json.JSONEncoder):
--- a/test/rest_common.py Sun Mar 17 19:00:43 2019 -0400 +++ b/test/rest_common.py Sun Mar 17 19:28:26 2019 -0400 @@ -8,15 +8,18 @@ from roundup.rest import RestfulInstance, calculate_etag from roundup.backends import list_backends from roundup.cgi import client +from roundup.anypy.strings import s2b, b2s import random from .db_test_base import setupTracker from .mocknull import MockNull -from io import StringIO +from io import BytesIO import json +from .caseinsensitivedict import CaseInsensitiveDict + NEEDS_INSTANCE = 1 @@ -68,7 +71,7 @@ 'TRACKER_NAME': 'rounduptest' } self.dummy_client = client.Client(self.instance, MockNull(), env, [], None) - self.dummy_client.request.headers.getheader = self.get_header + self.dummy_client.request.headers = CaseInsensitiveDict() self.empty_form = cgi.FieldStorage() self.server = RestfulInstance(self.dummy_client, self.db) @@ -81,12 +84,6 @@ if error.errno not in (errno.ENOENT, errno.ESRCH): raise - def get_header (self, header, not_found=None): - try: - return self.headers[header.lower()] - except (AttributeError, KeyError): - return not_found - def testGet(self): """ Retrieve all three users @@ -351,14 +348,15 @@ Run over header only, etag in form only, both, each one broke and no etag. Use the put command - to triger the etag checking code. + to trigger the etag checking code. ''' for mode in ('header', 'etag', 'both', 'brokenheader', 'brokenetag', 'none'): try: - # clean up any old header - del(self.headers) - except AttributeError: + # use lower case for key to delete. Probably + # a bug. + del(self.dummy_client.request.headers['etag']) + except (AttributeError,KeyError): pass form = cgi.FieldStorage() @@ -369,21 +367,21 @@ if mode == 'header': print("Mode = %s"%mode) - self.headers = {'etag': etag} + self.dummy_client.request.headers['Etag'] = etag elif mode == 'etag': print("Mode = %s"%mode) form.list.append(cgi.MiniFieldStorage('@etag', etag)) elif mode == 'both': print("Mode = %s"%mode) - self.headers = {'etag': etag} + self.dummy_client.request.headers['Etag'] = etag form.list.append(cgi.MiniFieldStorage('@etag', etag)) elif mode == 'brokenheader': print("Mode = %s"%mode) - self.headers = {'etag': 'bad'} + self.dummy_client.request.headers['Etag'] = 'bad' form.list.append(cgi.MiniFieldStorage('@etag', etag)) elif mode == 'brokenetag': print("Mode = %s"%mode) - self.headers = {'etag': etag} + self.dummy_client.request.headers['Etag'] = etag form.list.append(cgi.MiniFieldStorage('@etag', 'bad')) elif mode == 'none': print( "Mode = %s"%mode) @@ -398,6 +396,10 @@ else: self.assertEqual(self.dummy_client.response_code, 412) + def make_file(self): + import tempfile + return tempfile.TemporaryFile("wb+") + def testDispatch(self): """ run changes through rest dispatch(). This also tests @@ -405,11 +407,20 @@ code that changes json payload into something we can process. """ - # Set joe's 'realname' using json data. + + # Override the make_file so it is always set to binary + # read mode. This is needed so we can send a json + # body. + saved_make_file = cgi.FieldStorage.make_file + cgi.FieldStorage.make_file = self.make_file + + + # TEST #1 + # PUT: joe's 'realname' using json data. # simulate: /rest/data/user/<id>/realname # use etag in header etag = calculate_etag(self.db.user.getnode(self.joeid)) - body=u'{ "data": "Joe Doe 1" }' + body='{ "data": "Joe Doe 1" }' env = { "CONTENT_TYPE": "application/json", "CONTENT_LENGTH": len(body), "REQUEST_METHOD": "PUT" @@ -418,31 +429,31 @@ "content-type": env['CONTENT_TYPE'], "etag": etag } - self.headers=headers # we need to generate a FieldStorage the looks like # FieldStorage(None, None, 'string') rather than # FieldStorage(None, None, []) - body_file=StringIO(body) # FieldStorage needs a file + body_file=BytesIO(s2b(body)) # FieldStorage needs a file + cgi.FieldStorage.make_file = self.make_file form = cgi.FieldStorage(body_file, headers=headers, environ=env) - self.server.client.request.headers.getheader=self.get_header + self.server.client.request.headers.update(headers) # set headers results = self.server.dispatch('PUT', "/rest/data/user/%s/realname"%self.joeid, form) - self.assertEqual(self.server.client.response_code, 200) results = self.server.get_element('user', self.joeid, self.empty_form) self.assertEqual(self.dummy_client.response_code, 200) self.assertEqual(results['data']['attributes']['realname'], 'Joe Doe 1') - del(self.headers) + self.server.client.request.headers.clear() # set headers + # TEST #2 # Set joe's 'realname' using json data. # simulate: /rest/data/user/<id>/realname # use etag in payload etag = calculate_etag(self.db.user.getnode(self.joeid)) - body=u'{ "@etag": "%s", "data": "Joe Doe 2" }'%etag + body='{ "@etag": "%s", "data": "Joe Doe 2" }'%etag env = { "CONTENT_TYPE": "application/json", "CONTENT_LENGTH": len(body), "REQUEST_METHOD": "PUT" @@ -450,12 +461,11 @@ headers={"accept": "application/json", "content-type": env['CONTENT_TYPE'] } - self.headers=headers - body_file=StringIO(body) # FieldStorage needs a file + body_file=BytesIO(s2b(body)) # FieldStorage needs a file form = cgi.FieldStorage(body_file, headers=headers, environ=env) - self.server.client.request.headers.getheader=self.get_header + self.server.client.request.headers.update(headers) # set headers results = self.server.dispatch('PUT', "/rest/data/user/%s/realname"%self.joeid, form) @@ -465,15 +475,13 @@ self.assertEqual(self.dummy_client.response_code, 200) self.assertEqual(results['data']['attributes']['realname'], 'Joe Doe 2') - del(self.headers) + self.server.client.request.headers.clear() + # TEST #3 # change Joe's realname via a normal web form - # This generates a FieldStorage that looks like: - # FieldStorage(None, None, []) # use etag from header # - # also use a GET on the uri via the dispatch to get - # the results from the db. + # Also do a GET using dispatch to get the results from the db. etag = calculate_etag(self.db.user.getnode(self.joeid)) headers={"etag": etag, "accept": "application/json", @@ -482,8 +490,7 @@ form.list = [ cgi.MiniFieldStorage('data', 'Joe Doe'), ] - self.headers = headers - self.server.client.request.headers.getheader = self.get_header + self.server.client.request.headers.update(headers) # set headers results = self.server.dispatch('PUT', "/rest/data/user/%s/realname"%self.joeid, form) @@ -492,25 +499,27 @@ "/rest/data/user/%s/realname"%self.joeid, self.empty_form) self.assertEqual(self.dummy_client.response_code, 200) - json_dict = json.loads(results) + json_dict = json.loads(b2s(results)) self.assertEqual(json_dict['data']['data'], 'Joe Doe') self.assertEqual(json_dict['data']['link'], "http://tracker.example/cgi-bin/" "roundup.cgi/bugs/rest/data/user/3/realname") - self.assertEqual(json_dict['data']['type'], "<type 'str'>") + self.assertIn(json_dict['data']['type'], ("<class 'str'>", + "<type 'str'>")) self.assertEqual(json_dict['data']["id"], "3") - del(self.headers) + self.server.client.request.headers.clear() - # PATCH joe's email address with json + # TEST #4 + # PATCH: joe's email address with json # save address so we can use it later stored_results = self.server.get_element('user', self.joeid, self.empty_form) self.assertEqual(self.dummy_client.response_code, 200) etag = calculate_etag(self.db.user.getnode(self.joeid)) - body=u'{ "address": "demo2@example.com", "@etag": "%s"}'%etag + body='{ "address": "demo2@example.com", "@etag": "%s"}'%etag env = { "CONTENT_TYPE": "application/json", "CONTENT_LENGTH": len(body), "REQUEST_METHOD": "PATCH" @@ -518,12 +527,11 @@ headers={"accept": "application/json", "content-type": env['CONTENT_TYPE'] } - self.headers=headers - body_file=StringIO(body) # FieldStorage needs a file + body_file=BytesIO(s2b(body)) # FieldStorage needs a file form = cgi.FieldStorage(body_file, headers=headers, environ=env) - self.server.client.request.headers.getheader=self.get_header + self.server.client.request.headers.update(headers) # set headers results = self.server.dispatch('PATCH', "/rest/data/user/%s"%self.joeid, form) @@ -533,18 +541,16 @@ self.assertEqual(self.dummy_client.response_code, 200) self.assertEqual(results['data']['attributes']['address'], 'demo2@example.com') - - # and set it back + # and set it back reusing env and headers from last test etag = calculate_etag(self.db.user.getnode(self.joeid)) - body=u'{ "address": "%s", "@etag": "%s"}'%( + body='{ "address": "%s", "@etag": "%s"}'%( stored_results['data']['attributes']['address'], etag) # reuse env and headers from prior test. - body_file=StringIO(body) # FieldStorage needs a file + body_file=BytesIO(s2b(body)) # FieldStorage needs a file form = cgi.FieldStorage(body_file, headers=headers, environ=env) - self.server.client.request.headers.getheader=self.get_header results = self.server.dispatch('PATCH', "/rest/data/user/%s"%self.joeid, form) @@ -554,11 +560,17 @@ self.assertEqual(self.dummy_client.response_code, 200) self.assertEqual(results['data']['attributes']['address'], 'random@home.org') - del(self.headers) + self.server.client.request.headers.clear() + - # POST to create new issue - body=u'{ "title": "foo bar", "priority": "critical" }' - + # TEST #5 + # POST: create new issue + # no etag needed + # FIXME at some point we probably want to implement + # Post Once Only, so we need to add a Post Once Exactly + # test and a resubmit as well. + etag = "not needed" + body='{ "title": "foo bar", "priority": "critical" }' env = { "CONTENT_TYPE": "application/json", "CONTENT_LENGTH": len(body), "REQUEST_METHOD": "POST" @@ -566,18 +578,16 @@ headers={"accept": "application/json", "content-type": env['CONTENT_TYPE'] } - self.headers=headers - body_file=StringIO(body) # FieldStorage needs a file + body_file=BytesIO(s2b(body)) # FieldStorage needs a file form = cgi.FieldStorage(body_file, headers=headers, environ=env) - self.server.client.request.headers.getheader=self.get_header + self.server.client.request.headers.update(headers) # set headers results = self.server.dispatch('POST', "/rest/data/issue", form) - self.assertEqual(self.server.client.response_code, 201) - json_dict = json.loads(results) + json_dict = json.loads(b2s(results)) issue_id=json_dict['data']['id'] results = self.server.get_element('issue', str(issue_id), # must be a string not unicode @@ -585,7 +595,10 @@ self.assertEqual(self.dummy_client.response_code, 200) self.assertEqual(results['data']['attributes']['title'], 'foo bar') - del(self.headers) + self.server.client.request.headers.clear() + + # reset the make_file method in the class + cgi.FieldStorage.make_file = saved_make_file def testPut(self): """ @@ -615,7 +628,7 @@ cgi.MiniFieldStorage('data', 'Joe Doe Doe'), ] - self.headers = {'etag': etag } # use etag in header + self.dummy_client.request.headers['ETag'] = etag # use etag in header results = self.server.put_attribute( 'user', self.joeid, 'realname', form ) @@ -625,7 +638,7 @@ ) self.assertEqual(self.dummy_client.response_code, 200) self.assertEqual(results['data']['data'], 'Joe Doe Doe') - del(self.headers) + del(self.dummy_client.request.headers['etag']) # Reset joe's 'realname'. etag in body form = cgi.FieldStorage()
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/test_CaseInsensitiveDict.py Sun Mar 17 19:28:26 2019 -0400 @@ -0,0 +1,63 @@ +# test_CaseInsensitiveDict.py +# By miloskot at: +# https://stackoverflow.com/questions/2082152/case-insensitive-dictionary + +import json +import unittest +from .caseinsensitivedict import * + +class Key(unittest.TestCase): + def setUp(self): + self.Key = CaseInsensitiveDict.Key + self.lower = self.Key('a') + self.upper = self.Key('A') + + def test_eq(self): + self.assertEqual(self.lower, self.upper) + + def test_hash(self): + self.assertEqual(hash(self.lower), hash(self.upper)) + + def test_str(self): + self.assertEqual(str(self.lower), 'a') + self.assertEqual(str(self.upper), 'A') + +class Dict(unittest.TestCase): + def setUp(self): + self.Dict = CaseInsensitiveDict + self.d1 = self.Dict() + self.d2 = self.Dict() + self.d1['a'] = 1 + self.d1['B'] = 2 + self.d2['A'] = 1 + self.d2['b'] = 2 + + def test_contains(self): + self.assertIn('B', self.d1) + d = self.Dict({'a':1, 'B':2}) + self.assertIn('b', d) + + def test_init(self): + d = self.Dict() + self.assertFalse(d) + d = self.Dict({'a':1, 'B':2}) + self.assertTrue(d) + + def test_items(self): + self.assertDictEqual(self.d1, self.d2) + self.assertEqual( + [v for v in self.d1.items()], + [v for v in self.d2.items()]) + + def test_json_dumps(self): + s = json.dumps(self.d1) + self.assertIn('a', s) + self.assertIn('B', s) + + def test_keys(self): + self.assertEqual(self.d1.keys(), self.d2.keys()) + + def test_values(self): + self.assertEqual( + [v for v in self.d1.values()], + [v for v in self.d2.values()])