Skip to content

Commit

Permalink
Fix #132: enforce 64-bit ranges on long values for protobuf attrs.
Browse files Browse the repository at this point in the history
  • Loading branch information
@tseaver
tseaver committed Sep 24, 2014
1 parent b97ce73 commit 0abfa25
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
5 changes: 4 additions & 1 deletion gcloud/datastore/helpers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
import calendar
from datetime import datetime, timedelta

from google.protobuf.internal.type_checkers import Int64ValueChecker
import pytz

from gcloud.datastore.key import Key


INT64 = Int64ValueChecker().CheckValue

def get_protobuf_attribute_and_value(val):
"""Given a value, return the protobuf attribute name and proper value.
Expand Down Expand Up @@ -53,7 +56,7 @@ def get_protobuf_attribute_and_value(val):
elif isinstance(val, float):
name, value = 'double', val
elif isinstance(val, (int, long)):
name, value = 'integer', val
name, value = 'integer', INT64(val)

This comment has been minimized.

Sign in to view

Copy link
@jgeewax

jgeewax Oct 3, 2014

Contributor

@tseaver : This actually shouldn't work :(

https://code.google.com/p/protobuf/source/browse/trunk/python/google/protobuf/internal/type_checkers.py?r=163#100 returns None. We need to check and then return "val".

Opening a ticket for this.

This comment has been minimized.

Sign in to view

Copy link
@dhermes

dhermes Oct 3, 2014

Contributor

My bad (as LGTM'er). I tested INT64 but not the return value. D'oh.

UPDATE: I did check it and INT64(val) == long(val) on my install (not None).

This comment has been minimized.

Sign in to view

Copy link
@jgeewax

jgeewax Oct 3, 2014

Contributor

To be fair, the link I have there is an early version... However, I'm on Ubuntu 13.10, with the python-protobuf package installed (via apt, not pip) and it returns None.

Due to this, I think the patch might be a good idea...

(master)jj@jjg-gigabyte:~/projects/gcloud$ sudo apt-get install python-protobuf
Reading package lists... Done
Building dependency tree       
Reading state information... Done
python-protobuf is already the newest version.
The following packages were automatically installed and are no longer required:
  libgetopt-argvfile-perl libmodule-scandeps-perl libmodule-signature-perl libpar-dist-perl libpar-perl python-keyring python-launchpadlib python-lazr.restfulclient python-lazr.uri python-oauth python-problem-report python-secretstorage python-simplejson python-wadllib
Use 'apt-get autoremove' to remove them.
0 upgraded, 0 newly installed, 0 to remove and 6 not upgraded.
(master)jj@jjg-gigabyte:~/projects/gcloud$ dpkg -s python-protobuf
Package: python-protobuf
Status: install ok installed
Priority: optional
Section: python
Installed-Size: 458
Maintainer: Ubuntu Developers <[email protected]>
Architecture: amd64
Source: protobuf
Version: 2.4.1-3ubuntu2
Provides: python2.7-protobuf
Depends: libc6 (>= 2.4), libgcc1 (>= 1:4.1.1), libprotobuf7, libstdc++6 (>= 4.1.1), python (>= 2.7), python (<< 2.8), python:any (>= 2.7.1-0ubuntu2)
Recommends: protobuf-compiler
Description: Python bindings for protocol buffers
 Protocol buffers are a flexible, efficient, automated mechanism for
 serializing structured data - similar to XML, but smaller, faster, and
 simpler. You define how you want your data to be structured once, then you can
 use special generated source code to easily write and read your structured
 data to and from a variety of data streams and using a variety of languages.
 You can even update your data structure without breaking deployed programs
 that are compiled against the "old" format.
 .
 Google uses Protocol Buffers for almost all of its internal RPC protocols and
 file formats.
 .
 This package contains the Python bindings for the protocol buffers. You will
 need the protoc tool (in the protobuf-compiler package) to compile your
 definition to Python classes, and then the modules in this package will allow
 you to use those classes in your programs.
 .
 This package contains both the traditional Python-based
 implementation and the new C++-based one, and you can select at
 runtime between the two.
Homepage: http://code.google.com/p/protobuf/
Original-Maintainer: Iustin Pop <[email protected]>
(master)jj@jjg-gigabyte:~/projects/gcloud$ python 
Python 2.7.5+ (default, Feb 27 2014, 19:37:08) 
[GCC 4.8.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from google import protobuf
>>> from google.protobuf.internal.type_checkers import Int64ValueChecker
>>> print Int64ValueChecker().CheckValue(123)
None

This comment has been minimized.

Sign in to view

Copy link
@dhermes

dhermes Oct 3, 2014

Contributor

Let's take this discussion to #215 (I commented here before reading all my GitHub notifications).
elif isinstance(val, basestring):
name, value = 'string', val

Expand Down
10 changes: 9 additions & 1 deletion gcloud/datastore/test_helpers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,19 @@ def test_int(self):
self.assertEqual(value, 42)

def test_long(self):
must_be_long = 1 << 63
must_be_long = (1 << 63) - 1
name, value = self._callFUT(must_be_long)
self.assertEqual(name, 'integer_value')
self.assertEqual(value, must_be_long)

def test_long_too_small(self):
too_small = -(1 << 63) - 1
self.assertRaises(ValueError, self._callFUT, too_small)

def test_long_too_large(self):
too_large = 1 << 63
self.assertRaises(ValueError, self._callFUT, too_large)

def test_native_str(self):
name, value = self._callFUT('str')
self.assertEqual(name, 'string_value')
Expand Down

0 comments on commit 0abfa25

Please sign in to comment.