Fix manage_edit of File (#312)

re #240:

* Ensure working file upload.
* Render the file content in textarea if it can be decoded.
* Edit content of text file with Python 3.
* Add changelog and documentation.

CHANGES.rst

@@ -19,6 +19,10 @@ New features
 - Add a minimum ``buildout.cfg`` suggestion in the docs for creating ``wsgi``
   instances.
 
+- Inlcude the ``zmi.styles`` repository in this package to break a circular
+  dependency.
+  (`#307 <https://github.com/zopefoundation/Zope/pull/307>`_)
+
 Bugfixes
 ++++++++
 
@@ -29,6 +33,9 @@ Bugfixes
   native ``str`` on both Python versions.
   (`#265 <https://github.com/zopefoundation/Zope/pull/265>`_)
 
+- Fix upload and rendering of text files.
+  (`#240 <https://github.com/zopefoundation/Zope/pull/240>`_)
+
 - Include the ``zmi.styles`` repository in this package to break a circular
   dependency.
   (`#307 <https://github.com/zopefoundation/Zope/pull/307>`_)

docs/WHATSNEW.rst

@@ -151,3 +151,12 @@ ZMI overhaul
 The ZMI (Zope Management Interface) is now styled with Bootstrap.
 See :ref:`ZMI-label` for details how to adapt Zope add-on packages to the new
 styling.
+
+
+Unified encoding
+----------------
+
+As it is reasonable to have one unified encoding in ZMI and frontend,
+``management_page_charset`` (as property of a folder) and ``default-zpublisher-
+encoding`` in `zope.conf` have to define the same encoding, which is `utf-8`
+by default for both.

src/OFS/Image.py

@@ -46,6 +46,7 @@
 from OFS.SimpleItem import Item_w__name__
 from ZPublisher import HTTPRangeSupport
 from ZPublisher.HTTPRequest import FileUpload
+import ZPublisher.HTTPRequest
 
 try:
     from html import escape
@@ -470,6 +471,11 @@ def update_data(self, data, content_type=None, size=None):
         self.ZCacheable_set(None)
         self.http__refreshEtag()
 
+    def _get_encoding(self):
+        """Get the canonical encoding for ZMI."""
+        return getattr(self, 'management_page_charset',
+                       ZPublisher.HTTPRequest.default_encoding)
+
     security.declareProtected(change_images_and_files, 'manage_edit')
     def manage_edit(self, title, content_type, precondition='',
                     filedata=None, REQUEST=None):
@@ -486,6 +492,8 @@ def manage_edit(self, title, content_type, precondition='',
         elif self.precondition:
             del self.precondition
         if filedata is not None:
+            if isinstance(filedata, text_type):
+                filedata = filedata.encode(self._get_encoding())
             self.update_data(filedata, content_type, len(filedata))
         else:
             self.ZCacheable_invalidate()
@@ -624,9 +632,11 @@ def getContentType(self):
     def __bytes__(self):
         return bytes(self.data)
 
-    if PY2:
-        def __str__(self):
+    def __str__(self):
+        if PY2:
             return str(self.data)
+        else:
+            return self.data.decode(self._get_encoding())
 
     def __bool__(self):
         return True

src/OFS/dtml/fileEdit.dtml

@@ -41,11 +41,16 @@
 		<dtml-let ct=getContentType>
 			<dtml-if "(ct.startswith('text') or ct.endswith('javascript')) and this().get_size() < 65536">
 				<div class="form-group">
-					<textarea id="content" <dtml-if content_type>data-contenttype="&dtml-content_type;"</dtml-if>
-						class="form-control zmi-file zmi-code col-sm-12"
-						name="filedata:text" wrap="off" rows="20"><dtml-var __str__ html_quote></textarea>
+					<dtml-try>
+						<textarea id="content" <dtml-if content_type>data-contenttype="&dtml-content_type;"</dtml-if>
+							class="form-control zmi-file zmi-code col-sm-12"
+							name="filedata:text" wrap="off" rows="20"><dtml-var __str__ html_quote></textarea>
+					<dtml-except UnicodeDecodeError>
+						<div class="alert alert-warning" role="alert">
+							The file could not be decoded with '<dtml-var "error_value.encoding">'. Setting a different encoding as `management_page_charset` might help.
+						</div>
+					</dtml-try>
 				</div>
-
 			<dtml-else>
 				<div class="form-group row">
 					<label for="size" class="form-label	col-sm-3 col-md-2">File Size</label>
@@ -68,15 +73,15 @@
 </form>
 
 	<dtml-unless wl_isLocked>
-		<form action="<dtml-var "REQUEST.URL1" html_quote>" action="manage_upload" method="post" enctype="multipart/form-data" class="zmi-upload mt-4">
+		<form action="<dtml-var "REQUEST.URL1" html_quote>" method="post" enctype="multipart/form-data" class="zmi-upload mt-4">
 			<div class="input-group" title="Select Local File for Uploading">
 				<div class="custom-file">
 					<input type="file" name="file" class="custom-file-input" id="file-data" value=""
 						onchange="$('.custom-file label span').html($(this).val().replace(/^.*(\\|\/|\:)/, ''));" />
 					<label class="custom-file-label" for="file-data"><span>Choose file</span></label>
 				</div>
 				<div class="input-group-append">
-					<input class="btn btn-outline-secondary" type="submit" value="Upload File" />
+					<input class="btn btn-outline-secondary" type="submit" name="manage_upload:method" value="Upload File" />
 				</div>
 			</div>
 		</form>

src/OFS/tests/testFileAndImage.py

@@ -1,7 +1,9 @@
+# -*- coding: utf-8 -*-
 import unittest
 
 import Zope2
 
+import codecs
 import os
 import sys
 import time
@@ -19,6 +21,8 @@
 from App.Common import rfc1123_date
 from Testing.makerequest import makerequest
 from zExceptions import Redirect
+import Testing.ZopeTestCase
+import Testing.testbrowser
 import transaction
 
 import OFS.Image
@@ -292,8 +296,8 @@ def test_interfaces(self):
     def testUnicode(self):
         val = u'some unicode string here'
 
-        self.assertRaises(TypeError, self.file.manage_edit,
-                          'foobar', 'text/plain', filedata=val)
+        self.assertRaises(TypeError, self.file.update_data,
+                          data=val, content_type='text/plain')
 
 
 class ImageTests(FileTests):
@@ -335,3 +339,71 @@ def test_text_representation_is_tag(self):
         self.assertEqual(six.text_type(self.file),
                          '<img src="http://nohost/file"'
                          ' alt="" title="" height="16" width="16" />')
+
+
+class FileEditTests(Testing.ZopeTestCase.FunctionalTestCase):
+    """Browser testing ..Image.File"""
+
+    def setUp(self):
+        super(FileEditTests, self).setUp()
+        uf = self.app.acl_users
+        uf.userFolderAddUser('manager', 'manager_pass', ['Manager'], [])
+        self.app.manage_addFile('file')
+
+        transaction.commit()
+        self.browser = Testing.testbrowser.Browser()
+        self.browser.addHeader(
+            'Authorization',
+            'basic {}'.format(codecs.encode(
+                b'manager:manager_pass', 'base64').decode()))
+
+    def test_Image__manage_main__1(self):
+        """It shows the content of text files as text."""
+        self.app.file.update_data(u'hällo'.encode('utf-8'))
+        self.browser.open('http://localhost/file/manage_main')
+        text = self.browser.getControl(name='filedata:text').value
+        self.assertEqual(text, 'hällo')
+
+    @unittest.skipIf(six.PY2, "feature not supported on Python 2")
+    def test_Image__manage_main__2(self):
+        """It shows the content of text files.
+
+        It respects the encoding in `management_page_charset`.
+        """
+        self.app.management_page_charset = 'latin-1'
+        self.app.file.update_data(u'hällo'.encode('latin-1'))
+        self.browser.open('http://localhost/file/manage_main')
+        text = self.browser.getControl(name='filedata:text').value
+        self.assertEqual(text, 'hällo')
+
+    @unittest.skipIf(six.PY2, "feature not supported on Python 2")
+    def test_Image__manage_main__3(self):
+        """It shows an error message if the file content cannot be decoded."""
+        self.app.file.update_data(u'hällo'.encode('latin-1'))
+        self.browser.open('http://localhost/file/manage_main')
+        self.assertIn(
+            "The file could not be decoded with 'utf-8'.",
+            self.browser.contents)
+
+    def test_Image__manage_upload__1(self):
+        """It uploads a file, replaces the content and sets content type."""
+        self.browser.open('http://localhost/file/manage_main')
+        self.browser.getControl(name='file').add_file(
+            b'test text file', 'text/plain', 'TestFile.txt')
+        self.browser.getControl('Upload File').click()
+        self.assertIn('Saved changes', self.browser.contents)
+        self.assertEqual(
+            self.browser.getControl('Content Type').value, 'text/plain')
+        text = self.browser.getControl(name='filedata:text').value
+        self.assertEqual(text, 'test text file')
+
+    def test_Image__manage_edit__1(self):
+        """It it possible to change the file's content via browser."""
+        self.browser.open('http://localhost/file/manage_main')
+        text_1 = self.browser.getControl(name='filedata:text').value
+        self.assertEqual(text_1, '')
+        self.browser.getControl(name='filedata:text').value = u'hällo'
+        self.browser.getControl('Save Changes').click()
+        self.assertIn('Saved changes', self.browser.contents)
+        text_2 = self.browser.getControl(name='filedata:text').value
+        self.assertEqual(text_2, 'hällo')

src/ZPublisher/HTTPRequest.py

@@ -497,8 +497,13 @@ def processInputs(
             environ['QUERY_STRING'] = ''
 
         meth = None
+        fs_kw = {}
+        if PY3:
+            # In Python 3 we need the proper encoding to parse the input.
+            fs_kw['encoding'] = self.charset
+
         fs = ZopeFieldStorage(
-            fp=fp, environ=environ, keep_blank_values=1)
+            fp=fp, environ=environ, keep_blank_values=1, **fs_kw)
 
         # Keep a reference to the FieldStorage. Otherwise it's
         # __del__ method is called too early and closing FieldStorage.file.