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

Only keep already uploaded image when it is a POST request #38

Merged
merged 3 commits into from
Mar 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions news/2628.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Only use the new feature for keeping an already uploaded image when it is a POST request.
Fixes auto csrf error in `site-controlpanel <https://github.com/plone/Products.CMFPlone/issues/2628>`_
and `personal-information <https://github.com/plone/Products.CMFPlone/issues/2709>`_ page.
[maurits]
15 changes: 13 additions & 2 deletions plone/formwidget/namedfile/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,19 @@
class Py23DocChecker(doctest.OutputChecker):
def check_output(self, want, got, optionflags):
if six.PY2:
got = re.sub('zope.publisher.interfaces.NotFound', 'NotFound', got)
got = re.sub("u'(.*?)'", "'\\1'", want)
got = re.sub('NotFound', 'zope.publisher.interfaces.NotFound', got)
got = re.sub('InvalidState', 'plone.formwidget.namedfile.validator.InvalidState', got)
got = re.sub('RequiredMissing', 'zope.schema._bootstrapinterfaces.RequiredMissing', got)
got = re.sub('IOError: cannot identify image file', 'OSError: cannot identify image file', got)
got = re.sub('IO instance', 'IO object', got)
got = re.sub("u'(.*?)'", "'\\1'", got)
got = re.sub('u"(.*?)"', '"\\1"', got)
got = re.sub("b'(.*?)'", "'\\1'", got)
got = re.sub('b"(.*?)"', '"\\1"', got)
want = re.sub("u'(.*?)'", "'\\1'", want)
want = re.sub('u"(.*?)"', '"\\1"', want)
want = re.sub("b'(.*?)'", "'\\1'", want)
want = re.sub('b"(.*?)"', '"\\1"', want)
return doctest.OutputChecker.check_output(self, want, got, optionflags)


Expand Down
17 changes: 16 additions & 1 deletion plone/formwidget/namedfile/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,21 @@ def file_upload_id(self):
"""Temporary store the uploaded file contents with a file_upload_id key.
In case of form validation errors the already uploaded image can then
be reused.

This is only useful on a POST request:
forms should not be using GET,
especially when you save something to the database.

Note that if we want this on a GET request,
we should add a safeWrite call in the code below:
plone.protect.utils.safeWrite(up.upload_map, self.request)
Otherwise plone.protect auto csrf will complain for example
when getting @@site-controlpanel or @@personal-information
See https://github.com/plone/Products.CMFPlone/issues/2628
and https://github.com/plone/Products.CMFPlone/issues/2709
"""
if self.request.method != 'POST':
return ''
if self._file_upload_id:
# cache this property for multiple calls within one request.
# This avoids storing a file upload multiple times.
Expand Down Expand Up @@ -270,7 +284,8 @@ def extract(self, default=NOVALUE):
data = fileinfo.get('data')

if filename or data:
filename = safe_basename(filename)
if filename:
filename = safe_basename(filename)
if (
filename is not None
and not isinstance(filename, six.text_type)
Expand Down
85 changes: 62 additions & 23 deletions plone/formwidget/namedfile/widget.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,31 @@ There are also more specific interfaces for each widget::
The widgets can be instantiated only using the request::

>>> from z3c.form.testing import TestRequest
>>> def make_get_request(**kwargs):
... # GET is actually the default.
... # return TestRequest(**kwargs)
... req = TestRequest(**kwargs)
... req.method = 'GET'
... return req
>>> def make_request(**kwargs):
... return TestRequest(**kwargs)
... req = TestRequest(**kwargs)
... req.method = 'POST'
... return req
>>> request = make_request()

>>> file_widget = NamedFileWidget(request)
>>> image_widget = NamedImageWidget(request)

Before rendering a widget, one has to set the name and id of the widget::

>>> file_widget.id = 'widget.id.file'
>>> file_widget.name = 'widget.name.file'

>>> image_widget.id = 'widget.id.image'
>>> image_widget.name = 'widget.name.image'
>>> def make_file_widget(request):
... file_widget = NamedFileWidget(request)
... # In some versions, before rendering a widget, one has to set the name and id of the widget:
... file_widget.id = 'widget.id.file'
... file_widget.name = 'widget.name.file'
... return file_widget
>>> def make_image_widget(request):
... image_widget = NamedImageWidget(request)
... # In some versions, before rendering a widget, one has to set the name and id of the widget:
... image_widget.id = 'widget.id.image'
... image_widget.name = 'widget.name.image'
... return image_widget
>>> file_widget = make_file_widget(request)
>>> image_widget = make_image_widget(request)

We also need to register the templates for the widgets::

Expand Down Expand Up @@ -89,24 +100,24 @@ We can extract simple file data from the widget like this::
>>> import six
>>> myfile = six.BytesIO(b'My file contents.')

>>> file_widget.request = make_request(form={'widget.name.file': myfile})
>>> file_widget = make_file_widget(make_request(form={'widget.name.file': myfile}))
>>> file_widget.update()
>>> file_widget.extract()
<...IO object at ...>

>>> image_widget.request = make_request(form={'widget.name.image': myfile})
>>> image_widget = make_image_widget(make_request(form={'widget.name.image': myfile}))
>>> image_widget.update()
>>> image_widget.extract()
<...IO object at ...>

If nothing is found in the request, the default is returned::

>>> file_widget.request = make_request()
>>> file_widget = make_file_widget(make_request())
>>> file_widget.update()
>>> file_widget.extract()
<NO_VALUE>

>>> image_widget.request = make_request()
>>> image_widget = make_image_widget(make_request())
>>> image_widget.update()
>>> image_widget.extract()
<NO_VALUE>
Expand All @@ -130,17 +141,45 @@ Now build a FileUpload::
>>> aFieldStorage = FieldStorageStub(myfile)
>>> myUpload = FileUpload(aFieldStorage)

>>> file_widget.request = make_request(form={'widget.name.file': myUpload})
First use a GET request::

>>> file_widget = make_file_widget(make_get_request(form={'widget.name.file': myUpload}))
>>> file_widget.update()
>>> file_widget.extract()
<ZPublisher.HTTPRequest.FileUpload ...>

>>> image_widget.request = make_request(form={'widget.name.image': myUpload})
>>> image_widget = make_image_widget(make_get_request(form={'widget.name.image': myUpload}))
>>> image_widget.update()
>>> image_widget.extract()
<ZPublisher.HTTPRequest.FileUpload ...>

The rendering is unchanged:

>>> print(file_widget.render())
<span id="widget.id.file" class="named-file-widget">
<input type="file" id="widget.id.file-input"
name="widget.name.file" />
</span>

>>> print(image_widget.render())
<span id="widget.id.image" class="named-image-widget">
<input type="file" id="widget.id.image-input"
name="widget.name.image" />
</span>

Now use a POST request (the default in our make_request helper function)::

>>> file_widget = make_file_widget(make_request(form={'widget.name.file': myUpload}))
>>> file_widget.update()
>>> file_widget.extract()
<ZPublisher.HTTPRequest.FileUpload ...>

>>> image_widget = make_image_widget(make_request(form={'widget.name.image': myUpload}))
>>> image_widget.update()
>>> image_widget.extract()
<ZPublisher.HTTPRequest.FileUpload ...>

The rendering is unchanged::
The rendering contains data about the file upload id::

>>> print(file_widget.render())
<span id="widget.id.file" class="named-file-widget">
Expand Down Expand Up @@ -170,12 +209,12 @@ Empty, unnamed FileUploads are treated as having no value::
>>> aFieldStorage = FieldStorageStub(emptyfile, filename='')
>>> myEmptyUpload = FileUpload(aFieldStorage)

>>> file_widget.request = make_request(form={'widget.name.file': myEmptyUpload})
>>> file_widget = make_file_widget(make_request(form={'widget.name.file': myEmptyUpload}))
>>> file_widget.update()
>>> file_widget.extract()
<NO_VALUE>

>>> image_widget.request = make_request(form={'widget.name.image': myEmptyUpload})
>>> image_widget = make_image_widget(make_request(form={'widget.name.image': myEmptyUpload}))
>>> image_widget.update()
>>> image_widget.extract()
<NO_VALUE>
Expand Down Expand Up @@ -456,7 +495,7 @@ content type::
>>> file_obj = file_converter.toFieldValue(FileUpload(aFieldStorage))
>>> file_obj.data
b'File upload contents.'
>>> print(file_obj.filename.encode('utf8'))
>>> file_obj.filename.encode('utf8')
b'rand\xc3\xb8m.txt'

Content type from headers sent by browser should be ignored::
Expand Down Expand Up @@ -692,7 +731,7 @@ Check that we have a good image that PIL can handle::

>>> content.image_field = bytes_image_converter.toFieldValue(uploaded)
>>> content.image_field
b'filenameb64:aW1hZ2UuanBn;datab64:/9j/4AAQSkZJRgABAQEAYABgAAD/...
b'filenameb64:aW1hZ2UuanBn;datab64:/9j/4AAQSkZJRgABAQEAYABgAAD/...'

Note that PIL cannot open this bytes image, so we cannot scale it::

Expand Down