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

Validation (FileAllowed) issue upgrading 0.13.1 -> 0.14 #276

Closed
docapotamus opened this issue Jan 9, 2017 · 13 comments
Closed

Validation (FileAllowed) issue upgrading 0.13.1 -> 0.14 #276

docapotamus opened this issue Jan 9, 2017 · 13 comments
Assignees
Labels
Milestone

Comments

@docapotamus
Copy link

docapotamus commented Jan 9, 2017

I upgraded Flask-WTF last night from 0.13.1 -> 0.14

The issue I have is that when the FileField is blank (if the user chooses not to upload an image) then the validation fails. I do not require the field.

class PostForm(FlaskForm):
    """Handle the input from the web for posts and replies."""
    body = TextAreaField('Post')

    upload = FileField('Upload', [
        FileAllowed(['gif', 'jpg', 'jpeg', 'png'],
                    'Only "gif", "jpg", "jpeg" and "png" files are supported')
    ])

    permission = RadioField('Permission', choices=[
        ('0', 'Public'),
        ('1', 'Pjuu'),
        ('2', 'Approved')
    ], default=0)

    def validate_body(self, field):
        if len(field.data.strip()) == 0 and not self.upload.data:
            raise ValidationError('Sorry. A message or an image is required.')

        if len(field.data.replace('\r\n', '\n')) > MAX_POST_LENGTH:
            raise ValidationError('Oh no! Posts can not be larger than '
                                  '{} characters'.format(MAX_POST_LENGTH))

If I do this through my test suite all is okay using the following code:

resp = self.client.post(
    url_for('posts.post'),
    data={
        'body': 'Test',
        'upload': '',
    },
    follow_redirects=True
)

The the post is successful. However if I make the request through Firefox (or Chrome) the validation is triggered saying I have an invalid format.

Request body:

Content-Type: multipart/form-data; boundary=---------------------------138689464934064453715802001
Content-Length: 651

-----------------------------138689464934064453715802001
Content-Disposition: form-data; name="body"

Test
-----------------------------138689464934064453715802001
Content-Disposition: form-data; name="csrf_token"

IjY4YTZmYjNmNWM4MWJlNmVjMjc3Y2Y4YjBiMTM1Nzk3YTdhMGZkNjci.C1Vusg.RCp4SBZKai60nRqMGYP63i8JfXM
-----------------------------138689464934064453715802001
Content-Disposition: form-data; name="permission"

0
-----------------------------138689464934064453715802001
Content-Disposition: form-data; name="upload"; filename=""
Content-Type: application/octet-stream


-----------------------------138689464934064453715802001--

This was happening before I fixed the deprecation warnings that FileField is getting removed and to use the built-in WTForms FileField and after I changed the code to use this.

I believe it's something to do with this change but can't find any documentation.

I have had to revert the change as it stopped my site being usable.

Thanks in advance

@docapotamus docapotamus changed the title Validation issue upgrading 0.13.1 -> 0.14 Validation (FileAllowed) issue upgrading 0.13.1 -> 0.14 Jan 9, 2017
@davidism davidism added the bug label Jan 9, 2017
@davidism davidism added this to the v0.14.1 milestone Jan 9, 2017
@docapotamus
Copy link
Author

docapotamus commented Jan 9, 2017

@ThiefMaster on #pocoo suggested trying wtforms.validators.Optional which didn't help.

@davidism davidism self-assigned this Jan 9, 2017
@davidism
Copy link
Member

Turns out my assumption was wrong: empty file fields still populate request.files, just with empty FileStorage instances. I think I'll un-deprecate FileField along with updating the two validators.

@davidism
Copy link
Member

davidism commented Jan 10, 2017

Optional doesn't work for the same reason: raw_data is not an empty list, it's a list with an empty FileStorage. But the change to FileField didn't affect that. If I make FileField only populate if the data is non-empty, that would solve this problem.

@davidism
Copy link
Member

Fixed in release 0.14.1 on PyPI.

@docapotamus
Copy link
Author

@davidism Thank you very much for getting this sorted so fast. It's a massive help.
Sorry I didn't have the time to investigate further myself 😀

docapotamus added a commit to pjuu/pjuu that referenced this issue Jan 10, 2017
* Upgraded Flask-WTF to 0.14.1 to fix issue with empty fields.

Thanks to @davidism in issue for resolving so quickly.
wtforms/flask-wtf#276

* Moved back to flask_wtf.file.FileField as deprecation has been removed.
jsonn pushed a commit to jsonn/pkgsrc that referenced this issue Feb 20, 2017
Version 0.14.2
--------------

Released 2017-01-10

- Fix bug where ``FlaskForm`` assumed ``meta`` argument was not ``None`` if it
  was passed. (`#278`_)

.. _#278: wtforms/flask-wtf#278

Version 0.14.1
--------------

Released 2017-01-10

- Fix bug where the file validators would incorrectly identify an empty file as
  valid data. (`#276`_, `#277`_)

    - ``FileField`` is no longer deprecated. The data is checked during
      processing and only set if it's a valid file.
    - ``has_file`` *is* deprecated; it's now equivalent to ``bool(field.data)``.
    - ``FileRequired`` and ``FileAllowed`` work with both the Flask-WTF and
      WTForms ``FileField`` classes.
    - The ``Optional`` validator now works with ``FileField``.

.. _#276: wtforms/flask-wtf#276
.. _#277: wtforms/flask-wtf#277

Version 0.14
------------

Released 2017-01-06

- Use itsdangerous to sign CSRF tokens and check expiration instead of doing it
  ourselves. (`#264`_)

    - All tokens are URL safe, removing the ``url_safe`` parameter from
      ``generate_csrf``. (`#206`_)
    - All tokens store a timestamp, which is checked in ``validate_csrf``. The
      ``time_limit`` parameter of ``generate_csrf`` is removed.

- Remove the ``app`` attribute from ``CsrfProtect``, use ``current_app``.
  (`#264`_)
- ``CsrfProtect`` protects the ``DELETE`` method by default. (`#264`_)
- The same CSRF token is generated for the lifetime of a request. It is exposed
  as ``g.csrf_token`` for use during testing. (`#227`_, `#264`_)
- ``CsrfProtect.error_handler`` is deprecated. (`#264`_)

    - Handlers that return a response work in addition to those that raise an
      error. The behavior was not clear in previous docs.
    - (`#200`_, `#209`_, `#243`_, `#252`_)

- Use ``Form.Meta`` instead of deprecated ``SecureForm`` for CSRF (and
  everything else). (`#216`_, `#271`_)

    - ``csrf_enabled`` parameter is still recognized but deprecated. All other
      attributes and methods from ``SecureForm`` are removed. (`#271`_)

- Provide ``WTF_CSRF_FIELD_NAME`` to configure the name of the CSRF token.
  (`#271`_)
- ``validate_csrf`` raises ``wtforms.ValidationError`` with specific messages
  instead of returning ``True`` or ``False``. This breaks anything that was
  calling the method directly. (`#239`_, `#271`_)

    - CSRF errors are logged as well as raised. (`#239`_)

- ``CsrfProtect`` is renamed to ``CSRFProtect``. A deprecation warning is issued
  when using the old name. ``CsrfError`` is renamed to ``CSRFError`` without
  deprecation. (`#271`_)
- ``FileField`` is deprecated because it no longer provides functionality over
  the provided validators. Use ``wtforms.FileField`` directly. (`#272`_)

.. _`#200`: wtforms/flask-wtf#200
.. _`#209`: wtforms/flask-wtf#209
.. _`#216`: wtforms/flask-wtf#216
.. _`#227`: wtforms/flask-wtf#227
.. _`#239`: wtforms/flask-wtf#239
.. _`#243`: wtforms/flask-wtf#243
.. _`#252`: wtforms/flask-wtf#252
.. _`#264`: wtforms/flask-wtf#264
.. _`#271`: wtforms/flask-wtf#271
.. _`#272`: wtforms/flask-wtf#272

Version 0.13.1
--------------

Released 2016/10/6

- Deprecation warning for ``Form`` is shown during ``__init__`` instead of immediately when subclassing. (`#262`_)
- Don't use ``pkg_resources`` to get version, for compatibility with GAE. (`#261`_)

.. _`#261`: wtforms/flask-wtf#261
.. _`#262`: wtforms/flask-wtf#262

Version 0.13
------------

Released 2016/09/29

- ``Form`` is renamed to ``FlaskForm`` in order to avoid name collision with WTForms's base class.  Using ``Form`` will show a deprecation warning. (`#250`_)
- ``hidden_tag`` no longer wraps the hidden inputs in a hidden div.  This is valid HTML5 and any modern HTML parser will behave correctly. (`#217`_, `#193`_)
- ``flask_wtf.html5`` is deprecated.  Import directly from ``wtforms.fields.html5``. (`#251`_)
- ``is_submitted`` is true for ``PATCH`` and ``DELETE`` in addition to ``POST`` and ``PUT``. (`#187`_)
- ``generate_csrf`` takes a ``token_key`` parameter to specify the key stored in the session. (`#206`_)
- ``generate_csrf`` takes a ``url_safe`` parameter to allow the token to be used in URLs. (`#206`_)
- ``form.data`` can be accessed multiple times without raising an exception. (`#248`_)
- File extension with multiple parts (``.tar.gz``) can be used in the ``FileAllowed`` validator. (`#201`_)

.. _`#187`: wtforms/flask-wtf#187
.. _`#193`: wtforms/flask-wtf#193
.. _`#201`: wtforms/flask-wtf#201
.. _`#206`: wtforms/flask-wtf#206
.. _`#217`: wtforms/flask-wtf#217
.. _`#248`: wtforms/flask-wtf#248
.. _`#250`: wtforms/flask-wtf#250
.. _`#251`: wtforms/flask-wtf#251
@WTRipper
Copy link

Seems that this bug still exists using following setup:
Versions

Flask==0.12
Flask-Bootstrap==3.3.7.1
Flask-WTF==0.14.2
WTForms==2.1

form class defintion

from flask_wtf import FlaskForm
from flask_wtf.file import FileField
from wtforms import SubmitField

class NewExample(FlaskForm):
    datei = FileField("Dateiupload",
                      render_kw={'multiple': 'True'}
                      )
    submit = SubmitField("Store this")

route

@main.route('/example', methods=["POST"])
def new_snippet():
    form = NewExample()
    if form.validate_on_submit():
        files = request.files.getlist("datei")
        if files:
            for f in files:
                print(f)
        return redirect(url_for("main.other_page"))
    else:
        return abort(404)

rendering

{% extends "bootstrap/base.html" %}
{% import "bootstrap/wtf.html" as wtf %}
{{ wtf.quick_form(form) }}

Using this setup and submitting an empty form will print

<FileStorage: '' ('application/octet-stream')>

@davidism
Copy link
Member

What's the problem in Flask-WTF? Werkzeug adds empty file objects for each field received, regardless of if the file contained data. You're not using Flask-WTF to process the form data, so it won't check that the object is empty.

@WTRipper
Copy link

But there is no other way to process multiple files in Flask-WTF at the moment, right?
Using a route like this:

@main.route('/example', methods=["POST"])
def new_snippet():
    form = NewExample()
    if form.validate_on_submit():
        files = form.datei.data
        if files:
            for f in files:
                print(f)
        return redirect(url_for("main.other_page"))
    else:
        return abort(404)

Gives you None when leaving the FileField empty but only gives the first FileStorage object if multiple files are selected.

@WTRipper
Copy link

Probably something that would be fixed with your patch right?

@davidism
Copy link
Member

Yes, I was about to link that. As it says at the bottom, just copy those fields into your own code, there's nothing requiring them to be in WTForms.

@WTRipper
Copy link

form_fix.py

from wtforms.widgets import Input
from wtforms.fields.core import Field


class FileInput(Input):
    """Render a file chooser input.
    :param multiple: allow choosing multiple files
    """

    input_type = 'file'

    def __init__(self, multiple=False):
        super(FileInput, self).__init__()
        self.multiple = multiple

    def __call__(self, field, **kwargs):
        # browser ignores value of file input for security
        kwargs['value'] = False

        if self.multiple:
            kwargs['multiple'] = True

        return super(FileInput, self).__call__(field, **kwargs)


class FileField(Field):
    """Renders a file upload field.
    By default, the value will be the filename sent in the form data.
    WTForms **does not** deal with frameworks' file handling capabilities.
    A WTForms extension for a framework may replace the filename value
    with an object representing the uploaded data.
    """

    widget = FileInput()

    def _value(self):
        # browser ignores value of file input for security
        return False


class MultipleFileField(FileField):
    """A :class:`FileField` that allows choosing multiple files."""

    widget = FileInput(multiple=True)

    def process_formdata(self, valuelist):
        self.data = valuelist

form class definition

from flask_wtf import FlaskForm
from .form_fix import MultipleFileField
from wtforms import SubmitField

class NewExample(FlaskForm):
    datei = MultipleFileField("Dateiupload")
    submit = SubmitField("Store this")

This renders something that looks like a StringInput behind the FileInput when using {{ wtf.quick_form(form) }}. And form.datei.data seems to be a list of strings (the file names I uploaded). What's wrong? That's probably some mistake I made.. Hope I waste not your time. I really appreciate your fast responses!

The rendered html:

<div class="form-group "><label class="control-label" for="datei">Dateiupload</label>
        
          <input class="form-control" id="datei" multiple name="datei" type="file">
        
  </div>

@WTRipper
Copy link

By manually calling {{ form.datei }} fixes the rendering problem but still form.datei.data is a list of filename strings.

@davidism
Copy link
Member

Please use Stack Overflow for questions about your own code. If you have a new bug report, after ensuring it is a bug with Flask-WTF, please open a new issue.

@wtforms wtforms locked and limited conversation to collaborators Sep 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

3 participants