Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix python 3 compatibility #475

Merged
merged 3 commits into from Jun 17, 2019
Merged

fix python 3 compatibility #475

merged 3 commits into from Jun 17, 2019

Conversation

ziirish
Copy link
Contributor

@ziirish ziirish commented Mar 13, 2019

Hi,

I have just updated my WTForms requirements from 2.1 to 2.2.1 but it triggers what looks like a python 3 compatibility issue (I reproduced the issue both with python 3.7.2 and 3.6.8).

Here is a traceback:

  File "/opt/workspace/burp-ui/burpui/routes.py", line 479, in login
    form = LoginForm(request.form)
  File "/root/.pyenv/versions/3.6.8/envs/bui-3.6.8/lib/python3.6/site-packages/wtforms/form.py", line 212, in __call__
    return type.__call__(cls, *args, **kwargs)
  File "/root/.pyenv/versions/3.6.8/envs/bui-3.6.8/lib/python3.6/site-packages/flask_wtf/form.py", line 88, in __init__
    super(FlaskForm, self).__init__(formdata=formdata, **kwargs)
  File "/root/.pyenv/versions/3.6.8/envs/bui-3.6.8/lib/python3.6/site-packages/wtforms/form.py", line 272, in __init__
    super(Form, self).__init__(self._unbound_fields, meta=meta_obj, prefix=prefix)
  File "/root/.pyenv/versions/3.6.8/envs/bui-3.6.8/lib/python3.6/site-packages/wtforms/form.py", line 52, in __init__
    field = meta.bind_field(self, unbound_field, options)
  File "/root/.pyenv/versions/3.6.8/envs/bui-3.6.8/lib/python3.6/site-packages/wtforms/meta.py", line 27, in bind_field
    return unbound_field.bind(form=form, **options)
  File "/root/.pyenv/versions/3.6.8/envs/bui-3.6.8/lib/python3.6/site-packages/wtforms/fields/core.py", line 353, in bind
    return self.field_class(*self.args, **kw)
  File "/root/.pyenv/versions/3.6.8/envs/bui-3.6.8/lib/python3.6/site-packages/wtforms/fields/core.py", line 451, in __init__
    self.choices = copy(choices)
  File "/root/.pyenv/versions/3.6.8/lib/python3.6/copy.py", line 96, in copy
    rv = reductor(4)
TypeError: can't pickle dict_items objects

My guess is this was introduced by #286

I'm also not sure whereas the default should be None or [] (since copy(None) => None)

I also don't know if I should make this PR against the 2.2 maintenance branch or not.

@ftm
Copy link
Contributor

@ftm ftm commented Jun 15, 2019

Hello, apologies for the delay in getting back to you. Are you still having the issue which caused you to make this PR?

Additionally, could you please give a minimal example that causes the issue this is meant to fix.

@ziirish
Copy link
Contributor Author

@ziirish ziirish commented Jun 15, 2019

@ziirish
Copy link
Contributor Author

@ziirish ziirish commented Jun 17, 2019

Hi,

So I just gave a try at both latest pypi release available and running pip install -e . against current git master and I can reproduce the issue in both.

Here is the traceback:

Traceback (most recent call last):
  File "/home/master/.pyenv/versions/3.6.8/envs/wtforms/lib/python3.6/site-packages/flask/app.py", line 2311, in wsgi_app                             
    response = self.full_dispatch_request()
  File "/home/master/.pyenv/versions/3.6.8/envs/wtforms/lib/python3.6/site-packages/flask/app.py", line 1834, in full_dispatch_request                
    rv = self.handle_user_exception(e)
  File "/home/master/.pyenv/versions/3.6.8/envs/wtforms/lib/python3.6/site-packages/flask/app.py", line 1737, in handle_user_exception                
    reraise(exc_type, exc_value, tb)
  File "/home/master/.pyenv/versions/3.6.8/envs/wtforms/lib/python3.6/site-packages/flask/_compat.py", line 36, in reraise                            
    raise value
  File "/home/master/.pyenv/versions/3.6.8/envs/wtforms/lib/python3.6/site-packages/flask/app.py", line 1832, in full_dispatch_request                
    rv = self.dispatch_request()
  File "/home/master/.pyenv/versions/3.6.8/envs/wtforms/lib/python3.6/site-packages/flask/app.py", line 1818, in dispatch_request                     
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/master/workspace/gh/wtf/app.py", line 22, in login
    form = LoginForm(request.form)
  File "/home/master/workspace/gh/wtforms/src/wtforms/form.py", line 199, in __call__                                                                 
    return type.__call__(cls, *args, **kwargs)
  File "/home/master/.pyenv/versions/3.6.8/envs/wtforms/lib/python3.6/site-packages/flask_wtf/form.py", line 88, in __init__                          
    super(FlaskForm, self).__init__(formdata=formdata, **kwargs)
  File "/home/master/workspace/gh/wtforms/src/wtforms/form.py", line 262, in __init__                                                                 
    super(Form, self).__init__(self._unbound_fields, meta=meta_obj, prefix=prefix)                                                                    
  File "/home/master/workspace/gh/wtforms/src/wtforms/form.py", line 48, in __init__                                                                  
    field = meta.bind_field(self, unbound_field, options)
  File "/home/master/workspace/gh/wtforms/src/wtforms/meta.py", line 27, in bind_field                                                                
    return unbound_field.bind(form=form, **options)
  File "/home/master/workspace/gh/wtforms/src/wtforms/fields/core.py", line 406, in bind                                                              
    return self.field_class(*self.args, **kw)
  File "/home/master/workspace/gh/wtforms/src/wtforms/fields/core.py", line 522, in __init__                                                          
    self.choices = copy(choices)
  File "/home/master/.pyenv/versions/3.6.8/lib/python3.6/copy.py", line 96, in copy                                                                   
    rv = reductor(4)
TypeError: can't pickle dict_items objects

And here is the reproducer:

app.py:

from flask import Flask, request, flash, render_template
from flask_wtf import FlaskForm
from wtforms import StringField, PasswordField, BooleanField, SelectField, validators

LANGUAGES = {
    'fr': "Français",
    'en': "English",
}

app = Flask(__name__)
app.config['SECRET_KEY'] = "IizAsuperSecret"


class LoginForm(FlaskForm):
    username = StringField("Username", [validators.DataRequired()])
    password = PasswordField("Password", [validators.DataRequired()])
    language = SelectField("Language", choices=LANGUAGES.items(), default="fr")
    remember = BooleanField('Remember me', [validators.Optional()])


@app.route('/login', methods=['POST', 'GET'])
def login():
    form = LoginForm(request.form)

    if form.validate_on_submit():
        flash(_('Logged in successfully'), 'success')
    return render_template('login.html', form=form)

templates/login.html:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Login</title>
</head>
<body>
  <form action="/login" method="POST">
    {{ form.username.label }} {{ form.username }}
    {{ form.password.label }} {{ form.password }}
    {{ form.language.label }} {{ form.language }}
    {{ form.remember.label }} {{ form.remember }}
  </form>
</body>
</html>

command: FLASK_APP=app.py flask run

@ftm
Copy link
Contributor

@ftm ftm commented Jun 17, 2019

Thanks for providing the example!

It seems your issue is caused by .items() on a dict not returning a list, but returning a dict_items object - using list( ... ) on this as you suggest transforms this into a list that copy can work with.

>>> LANGUAGES = {
...     'fr': "Français",
...     'en': "English",
... }
>>> LANGUAGES.items()
dict_items([('fr', 'Français'), ('en', 'English')])
>>> list(LANGUAGES.items())
[('fr', 'Français'), ('en', 'English')]

I don't know if list() has the same behaviour as copy() so I don't know if this change will cause an issue. @davidism what do you think since you changed it to copy() in the first place?

@davidism
Copy link
Member

@davidism davidism commented Jun 17, 2019

Looks fine. I probably chose copy() because list.copy() is Python 3 only, but list() should work too.

@ftm
Copy link
Contributor

@ftm ftm commented Jun 17, 2019

Okay I'll resolve the conflict and merge it (and do the changelog)

@ziirish
Copy link
Contributor Author

@ziirish ziirish commented Jun 17, 2019

Oh thanks, I knew I forgot something... sorry about the lack of changelog!

@ftm
Copy link
Contributor

@ftm ftm commented Jun 17, 2019

No worries, thanks for the PR.

@ftm ftm merged commit 277a437 into wtforms:master Jun 17, 2019
1 check passed
@ziirish
Copy link
Contributor Author

@ziirish ziirish commented Jun 19, 2019

FYI, I did some research and it looks like both copy.copy() and list() provide the same result for lists.
Note they only instantiate a new list but they don't copy the objects.

But, list() is faster than copy.copy() (although this was not my primary intent when submitting this patch):

In [1]: d = {'a': 1, 'b': 2, 'c': 3}

In [2]: i = list(d.items())

In [3]: import copy

In [4]: %timeit c = copy.copy(i)
398 ns ± 6.54 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [5]: %timeit l = list(i)
166 ns ± 4.58 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

azmeuk added a commit to azmeuk/wtforms that referenced this issue Apr 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants