[Checkins] SVN: Products.ExternalEditor/trunk/ - Added several comments about possible issues

Sidnei da Silva sidnei at enfoldsystems.com
Mon Oct 16 06:49:08 EDT 2006 

Log message for revision 70680:
  - Added several comments about possible issues
  - Added a test demonstrating issue with manage_FTPget having side-effects
  - Fixed possible side-effects by setting the content-type header as late as possible so it does not get overriden.
  - Bump version to 0.9.3
  

Changed:
  U   Products.ExternalEditor/trunk/CHANGES.txt
  U   Products.ExternalEditor/trunk/ExternalEditor.py
  U   Products.ExternalEditor/trunk/tests/edit.txt
  U   Products.ExternalEditor/trunk/tests/test_functional.py
  U   Products.ExternalEditor/trunk/version.txt
  U   Products.ExternalEditor/trunk/win32/setup.iss
  U   Products.ExternalEditor/trunk/zopeedit.py

-=-
Modified: Products.ExternalEditor/trunk/CHANGES.txt
===================================================================
--- Products.ExternalEditor/trunk/CHANGES.txt	2006-10-16 10:41:51 UTC (rev 70679)
+++ Products.ExternalEditor/trunk/CHANGES.txt	2006-10-16 10:49:02 UTC (rev 70680)
@@ -1,5 +1,10 @@
 External Editor Change Log
 
+  XX/XX/XXXX - 0.9.3 Release
+
+    - Fixed issue with 'manage_FTPget' overriding the 'Content-Type'
+      header.
+
   9/14/2006 - 0.9.2 Release
 
     - Added 'skip_data' option to make External Editor send out only

Modified: Products.ExternalEditor/trunk/ExternalEditor.py
===================================================================
--- Products.ExternalEditor/trunk/ExternalEditor.py	2006-10-16 10:41:51 UTC (rev 70679)
+++ Products.ExternalEditor/trunk/ExternalEditor.py	2006-10-16 10:49:02 UTC (rev 70680)
@@ -165,12 +165,6 @@
         metadata = join(r, '\n')
         metadata_len = len(metadata)
 
-        # Using RESPONSE.setHeader('Pragma', 'no-cache') would be better, but
-        # this chokes crappy most MSIE versions when downloads happen on SSL.
-        # cf. http://support.microsoft.com/support/kb/articles/q316/4/31.asp
-        RESPONSE.setHeader('Last-Modified', rfc1123_date())
-        RESPONSE.setHeader('Content-Type', 'application/x-zope-edit')
-
         # Check if we should send the file's data down the response.
         if REQUEST.get('skip_data'):
             # We've been requested to send only the metadata. The
@@ -180,10 +174,31 @@
 
         ob_data = getattr(Acquisition.aq_base(ob), 'data', None)
         if (ob_data is not None and isinstance(ob_data, Image.Pdata)):
-            # We have a File instance with chunked data, lets stream it
+            # We have a File instance with chunked data, lets stream it.
+            #
+            # Note we are setting the content-length header here. This
+            # is a simplification. Read comment below.
+            #
+            # We assume that ob.get_size() will return the exact size
+            # of the PData chain. If that assumption is broken we
+            # might have problems. This is mainly an optimization. If
+            # we read the whole PData chain just to compute the
+            # correct size that could cause the whole file to be read
+            # into memory.
             RESPONSE.setHeader('Content-Length', ob.get_size())
+            # It is safe to use this PDataStreamIterator here because
+            # it is consumed right below. This is only used to
+            # simplify the code below so it only has to deal with
+            # stream iterators or plain strings.
             body = PDataStreamIterator(ob.data)
         elif hasattr(ob, 'manage_FTPget'):
+            # Calling manage_FTPget *might* have side-effects. For
+            # example, in Archetypes it does set the 'content-type'
+            # response header, which would end up overriding our own
+            # content-type header because we've set it 'too
+            # early'. We've moved setting the content-type header to
+            # the '_write_metadata' method since, and any manipulation
+            # of response headers should happen there, if possible.
             try:
                 body = ob.manage_FTPget()
             except TypeError: # some need the R/R pair!
@@ -211,10 +226,26 @@
                 RESPONSE.write(data)
             return ''
 
-        # If we reached this point, body *must* be a string.
+        # If we reached this point, body *must* be a string. We *must*
+        # set the headers ourselves since _write_metadata won't get
+        # called.
+        self._set_headers(RESPONSE)
         return join((metadata, body), '\n')
 
+    def _set_headers(self, RESPONSE):
+        # Using RESPONSE.setHeader('Pragma', 'no-cache') would be better, but
+        # this chokes crappy most MSIE versions when downloads happen on SSL.
+        # cf. http://support.microsoft.com/support/kb/articles/q316/4/31.asp
+        RESPONSE.setHeader('Last-Modified', rfc1123_date())
+        RESPONSE.setHeader('Content-Type', 'application/x-zope-edit')
+
     def _write_metadata(self, RESPONSE, metadata, length):
+        # Set response content-type so that the browser gets hinted
+        # about what application should handle this.
+        self._set_headers(RESPONSE)
+
+        # Set response length and write our metadata. The '+1' on the
+        # content-length is the '\n' after the metadata.
         RESPONSE.setHeader('Content-Length', length + 1)
         RESPONSE.write(metadata)
         RESPONSE.write('\n')

Modified: Products.ExternalEditor/trunk/tests/edit.txt
===================================================================
--- Products.ExternalEditor/trunk/tests/edit.txt	2006-10-16 10:41:51 UTC (rev 70679)
+++ Products.ExternalEditor/trunk/tests/edit.txt	2006-10-16 10:49:02 UTC (rev 70680)
@@ -145,6 +145,32 @@
 
   >>> self.folder['some-file'].wl_clearLocks()
 
+
+Create a class that has a 'manage_FTPget' method with
+side-effects. Make sure that the content-type header is properly set
+to 'application/x-zope-edit':
+
+  >>> from Products.ExternalEditor.tests.test_functional import SideEffects
+  >>> _ = self.folder._setObject('another-file', SideEffects('another-file', 'some content'))
+
+  >>> print http(r"""
+  ... GET /test_folder_1_/externalEdit_/another-file HTTP/1.1
+  ... Authorization: Basic %s:%s
+  ... """ % (user_name, user_password))
+  HTTP/1.1 200 OK
+  Content-Length: 140
+  Content-Type: application/x-zope-edit; charset=iso-8859-15
+  Last-Modified:...
+  <BLANKLINE>
+  url:http://localhost/test_folder_1_/another-file
+  meta_type:Side Effects
+  title:
+  auth:...
+  cookie:
+  <BLANKLINE>
+  some content
+
+
 Callback Registry
 =================
 

Modified: Products.ExternalEditor/trunk/tests/test_functional.py
===================================================================
--- Products.ExternalEditor/trunk/tests/test_functional.py	2006-10-16 10:41:51 UTC (rev 70679)
+++ Products.ExternalEditor/trunk/tests/test_functional.py	2006-10-16 10:49:02 UTC (rev 70680)
@@ -20,6 +20,16 @@
 # Install our product
 ZopeTestCase.installProduct('ExternalEditor')
 
+from OFS.SimpleItem import SimpleItem
+class SideEffects(SimpleItem):
+    meta_type = 'Side Effects'
+    def __init__(self, id, content):
+        self.id = id
+        self.content = content
+    def manage_FTPget(self, REQUEST, RESPONSE):
+        RESPONSE.setHeader('Content-Type', 'text/plain')
+        return self.content
+
 def test_suite():
     import unittest
     suite = unittest.TestSuite()

Modified: Products.ExternalEditor/trunk/version.txt
===================================================================
--- Products.ExternalEditor/trunk/version.txt	2006-10-16 10:41:51 UTC (rev 70679)
+++ Products.ExternalEditor/trunk/version.txt	2006-10-16 10:49:02 UTC (rev 70680)
@@ -1 +1 @@
-0.9.2
+0.9.3

Modified: Products.ExternalEditor/trunk/win32/setup.iss
===================================================================
--- Products.ExternalEditor/trunk/win32/setup.iss	2006-10-16 10:41:51 UTC (rev 70679)
+++ Products.ExternalEditor/trunk/win32/setup.iss	2006-10-16 10:49:02 UTC (rev 70680)
@@ -3,10 +3,10 @@
 [Setup]
 DisableStartupPrompt=yes
 AppName=Zope External Editor Helper Application
-AppVerName=Zope External Editor 0.9.2
+AppVerName=Zope External Editor 0.9.3
 AppPublisher=Casey Duncan, Zope Corporation (maintained by Chris McDonough)
 AppPublisherURL=http://plope.com/software/ExternalEditor
-AppVersion=0.9.2
+AppVersion=0.9.3
 AppSupportURL=http://plope.com/software/ExternalEditor
 AppUpdatesURL=http://plope.com/software/ExternalEditor
 DefaultDirName={pf}\ZopeExternalEditor
@@ -14,7 +14,7 @@
 AllowNoIcons=yes
 LicenseFile=..\LICENSE.txt
 ChangesAssociations=yes
-OutputBaseFilename=zopeedit-win32-0.9.2
+OutputBaseFilename=zopeedit-win32-0.9.3
 
 [Registry]
 ; Register file type for use by helper app

Modified: Products.ExternalEditor/trunk/zopeedit.py
===================================================================
--- Products.ExternalEditor/trunk/zopeedit.py	2006-10-16 10:41:51 UTC (rev 70679)
+++ Products.ExternalEditor/trunk/zopeedit.py	2006-10-16 10:49:02 UTC (rev 70680)
@@ -16,7 +16,7 @@
 
 $Id$"""
 
-__version__ = '0.9.2'
+__version__ = '0.9.3'
 
 import sys

More information about the Checkins mailing list