only allow integer field to accept integers and strings, fixes #2835 by ryangallen · Pull Request #2836 · encode/django-rest-framework (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

ryangallen

@kevin-brown

I can't say I'm thrilled at the idea of doing type checking here, but I will point out that in Python 2 you also have to be able to handle long. And you can pass in a tuple as the second argument to isinstance instead of doing or multiple times.

Django itself handles this by calling str before int (and also stripping trailing decimals). This will trigger errors if decimal values are passed in, and it avoids type checking.

@ryangallen

Thanks Kevin, good call. I adjusted and squashed the change.

…s being stored as ints, fixes encode#2835.

Match IntegerField validation with Django IntegerField, prevents decimal values being stored as ints, fixes encode#2835 On branch master

@jpadilla

kevin-brown added a commit that referenced this pull request

Apr 20, 2015

@kevin-brown

only allow integer field to accept integers and strings, fixes #2835

@kevin-brown

Looks good, thanks for making the change.

tomchristie

@@ -682,7 +682,7 @@ def to_internal_value(self, data):
self.fail('max_string_length')
try:
data = int(data)
data = int(re.compile(r'\.0*\s*$').sub('', str(data)))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We should push this to self.re_decimal as per in Django so that it's only compiled once.
  2. We should comment that this regex substitution is so that we allow eg '1.0' as an int, but not '1.2'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomchristie

Coupla inline comments that would be good to resolve.

tinchodipalma added a commit to tinchodipalma/django-rest-framework that referenced this pull request

May 25, 2015

@tinchodipalma

This was referenced

Mar 9, 2017

This was referenced

Oct 6, 2017

This was referenced

Oct 16, 2017

This was referenced

Oct 16, 2017

This was referenced

Nov 6, 2017

This was referenced

Nov 14, 2017

This was referenced

Dec 10, 2017

This was referenced

Dec 20, 2017