)]}'
{"glanceclient/v1/images.py":[{"author":{"_account_id":15168,"name":"Kirill Zaitsev","email":"k.zaitsev@me.com","username":"kzaitsev"},"change_message_id":"aa72f914e048b729639177103f87595e606e7c54","unresolved":false,"context_lines":[{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        def paginate(qp, return_request_id\u003dNone):"},{"line_number":227,"context_line":"            for param, value in six.iteritems(qp):"},{"line_number":228,"context_line":"                if isinstance(value, six.text_type):"},{"line_number":229,"context_line":"                    # encode unicode params to UTF-8 for safe url encoding"},{"line_number":230,"context_line":"                    qp[param] \u003d value.encode(\u0027utf-8\u0027)"},{"line_number":231,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_db9c40c2","line":228,"updated":"2015-10-08 18:23:13.000000000","message":"you can probably just omit this if, if you use safe_encode from oslo encodeutils:\n\nhttps://github.com/openstack/oslo.utils/blob/d2b054a6451120b03249a7fa2a0e02bcd85f562d/oslo_utils/encodeutils.py#L88-L95\n\nThe error was that previously safe_decode was used (and even the comment message, that suggests, that param would be str seems wrong) But using safe_encode looks like a good idea here.","commit_id":"5a9f7e3d335bffb4e462c5e8917facfeeb36a099"},{"author":{"_account_id":12071,"name":"Richard Jones","email":"r1chardj0n3s@gmail.com","username":"richard"},"change_message_id":"59517eafef57c7ac7fbecf2f3ae71f096e122613","unresolved":false,"context_lines":[{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        def paginate(qp, return_request_id\u003dNone):"},{"line_number":227,"context_line":"            for param, value in six.iteritems(qp):"},{"line_number":228,"context_line":"                if isinstance(value, six.text_type):"},{"line_number":229,"context_line":"                    # encode unicode params to UTF-8 for safe url encoding"},{"line_number":230,"context_line":"                    qp[param] \u003d value.encode(\u0027utf-8\u0027)"},{"line_number":231,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_64b45934","line":228,"in_reply_to":"7a2fa921_db9c40c2","updated":"2015-10-09 03:34:26.000000000","message":"My concern is that safe_encode does way more than is necessary for this case, and could in fact mask (or generate) strange errors from mis-use of this code (say if someone passed pre-encoded high-bit data - the use of safe_encode would not help and I\u0027m pretty sure would generate a UnicodeDecodeError in an odd place).\n\nAll we need to do is encode the param if the param is unicode, so why complicate things?","commit_id":"5a9f7e3d335bffb4e462c5e8917facfeeb36a099"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"aa24fa0dcfc70faff755420a6cd1fe3e7d74077c","unresolved":false,"context_lines":[{"line_number":232,"context_line":"                    #"},{"line_number":233,"context_line":"                    # Making sure all params are str before"},{"line_number":234,"context_line":"                    # trying to encode them"},{"line_number":235,"context_line":"                    qp[param] \u003d encodeutils.safe_decode(value)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"            url \u003d \u0027/v1/images/detail?%s\u0027 % urlparse.urlencode(qp)"},{"line_number":238,"context_line":"            images, resp \u003d self._list(url, \"images\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afc51ec_643b85ec","side":"PARENT","line":235,"updated":"2016-03-11 20:20:23.000000000","message":"what about using safe_encode here, instead of safe_decode? It does what you\u0027re doing in your patch (including the check of whether it\u0027s a text_type or not)","commit_id":"22a03fd475d1807dee41b24291862f1bdc361273"},{"author":{"_account_id":12071,"name":"Richard Jones","email":"r1chardj0n3s@gmail.com","username":"richard"},"change_message_id":"d3cbddbdc60df9883cbefbf862c853b54f6bcb8e","unresolved":false,"context_lines":[{"line_number":232,"context_line":"                    #"},{"line_number":233,"context_line":"                    # Making sure all params are str before"},{"line_number":234,"context_line":"                    # trying to encode them"},{"line_number":235,"context_line":"                    qp[param] \u003d encodeutils.safe_decode(value)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"            url \u003d \u0027/v1/images/detail?%s\u0027 % urlparse.urlencode(qp)"},{"line_number":238,"context_line":"            images, resp \u003d self._list(url, \"images\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a18252c_7ae56dda","side":"PARENT","line":235,"in_reply_to":"3afc51ec_643b85ec","updated":"2016-04-14 06:35:09.000000000","message":"I chose to do the explicit thing here because safe_encode does a whole lot more, trying to guess what the input data might be. In this case, we know what the input data should be, and if it\u0027s a text type then we need to encode it.","commit_id":"22a03fd475d1807dee41b24291862f1bdc361273"}],"glanceclient/v2/images.py":[{"author":{"_account_id":15168,"name":"Kirill Zaitsev","email":"k.zaitsev@me.com","username":"kzaitsev"},"change_message_id":"aa72f914e048b729639177103f87595e606e7c54","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            if not isinstance(tag, six.string_types):"},{"line_number":137,"context_line":"                raise exc.HTTPBadRequest(\"Invalid tag value %s\" % tag)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"            if isinstance(tag, six.text_type):"},{"line_number":140,"context_line":"                # encode unicode tags to UTF-8 for safe url encoding"},{"line_number":141,"context_line":"                tag \u003d tag.encode(\u0027utf-8\u0027)"},{"line_number":142,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_9b79e884","line":139,"updated":"2015-10-08 18:23:13.000000000","message":"same here — safe_encode from oslo encodeutils seems to already do the check.\n\nhttps://github.com/openstack/oslo.utils/blob/d2b054a6451120b03249a7fa2a0e02bcd85f562d/oslo_utils/encodeutils.py#L88-L95\n\nis it really worth it to remove correct use of the oslo library?","commit_id":"5a9f7e3d335bffb4e462c5e8917facfeeb36a099"},{"author":{"_account_id":12071,"name":"Richard Jones","email":"r1chardj0n3s@gmail.com","username":"richard"},"change_message_id":"59517eafef57c7ac7fbecf2f3ae71f096e122613","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            if not isinstance(tag, six.string_types):"},{"line_number":137,"context_line":"                raise exc.HTTPBadRequest(\"Invalid tag value %s\" % tag)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"            if isinstance(tag, six.text_type):"},{"line_number":140,"context_line":"                # encode unicode tags to UTF-8 for safe url encoding"},{"line_number":141,"context_line":"                tag \u003d tag.encode(\u0027utf-8\u0027)"},{"line_number":142,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_246501b4","line":139,"in_reply_to":"7a2fa921_9b79e884","updated":"2015-10-09 03:34:26.000000000","message":"safe_encode does much more than just encode string (unicode in py2) objects, it attempts an unsafe (input encoding unknown) transcoding if the input is a bytes (str in py2) object.","commit_id":"5a9f7e3d335bffb4e462c5e8917facfeeb36a099"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"aa24fa0dcfc70faff755420a6cd1fe3e7d74077c","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            if not isinstance(tag, six.string_types):"},{"line_number":138,"context_line":"                raise exc.HTTPBadRequest(\"Invalid tag value %s\" % tag)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"            tags_url_params.append({\u0027tag\u0027: encodeutils.safe_encode(tag)})"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        for param, value in six.iteritems(filters):"},{"line_number":143,"context_line":"            if isinstance(value, six.string_types):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afc51ec_24317d0c","side":"PARENT","line":140,"updated":"2016-03-11 20:20:23.000000000","message":"I\u0027m not sure this is right. The code you wrote on your patch does exactly the same as safe_encode.","commit_id":"22a03fd475d1807dee41b24291862f1bdc361273"},{"author":{"_account_id":12071,"name":"Richard Jones","email":"r1chardj0n3s@gmail.com","username":"richard"},"change_message_id":"d3cbddbdc60df9883cbefbf862c853b54f6bcb8e","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            if not isinstance(tag, six.string_types):"},{"line_number":138,"context_line":"                raise exc.HTTPBadRequest(\"Invalid tag value %s\" % tag)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"            tags_url_params.append({\u0027tag\u0027: encodeutils.safe_encode(tag)})"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        for param, value in six.iteritems(filters):"},{"line_number":143,"context_line":"            if isinstance(value, six.string_types):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a18252c_5a0fd187","side":"PARENT","line":140,"in_reply_to":"3afc51ec_24317d0c","updated":"2016-04-14 06:35:09.000000000","message":"If the incoming tag is a string, rather than text (unicode) then safe_encode will attempt to decode it (badly, resulting in UnicodeDecodeError in high-bit encoding situations). This code is more explicit and avoids that error.","commit_id":"22a03fd475d1807dee41b24291862f1bdc361273"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"aa24fa0dcfc70faff755420a6cd1fe3e7d74077c","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        for param, value in six.iteritems(filters):"},{"line_number":143,"context_line":"            if isinstance(value, six.string_types):"},{"line_number":144,"context_line":"                filters[param] \u003d encodeutils.safe_encode(value)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        url \u003d \u0027/v2/images?%s\u0027 % parse.urlencode(filters)"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3afc51ec_04360103","side":"PARENT","line":144,"updated":"2016-03-11 20:20:23.000000000","message":"ditto, although you could probably remove the if here","commit_id":"22a03fd475d1807dee41b24291862f1bdc361273"},{"author":{"_account_id":12071,"name":"Richard Jones","email":"r1chardj0n3s@gmail.com","username":"richard"},"change_message_id":"d3cbddbdc60df9883cbefbf862c853b54f6bcb8e","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        for param, value in six.iteritems(filters):"},{"line_number":143,"context_line":"            if isinstance(value, six.string_types):"},{"line_number":144,"context_line":"                filters[param] \u003d encodeutils.safe_encode(value)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        url \u003d \u0027/v2/images?%s\u0027 % parse.urlencode(filters)"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a18252c_3a7bf5f6","side":"PARENT","line":144,"in_reply_to":"3afc51ec_04360103","updated":"2016-04-14 06:35:09.000000000","message":"Please see my response above.","commit_id":"22a03fd475d1807dee41b24291862f1bdc361273"}]}
