Re: [Checkins] SVN: Zope/branches/2.10/ OFS Image: Image and File now both support simple unicode objects for data (they function the same as strings for data).
Has this change been discussed? I don't agree with it. Unicode strings aren't byte strings. File and Image should only work with byte strings. Code dealing with them should always know and assume that they use str. Anything using unicode with them is broken or has done insufficient input validation. Please revert, unless you can convince me otherwise. Florent On 28 Jun 2006, at 13:24, Rocky Burt wrote:
Log message for revision 68876: OFS Image: Image and File now both support simple unicode objects for data (they function the same as strings for data).
Changed: U Zope/branches/2.10/doc/CHANGES.txt U Zope/branches/2.10/lib/python/OFS/Image.py U Zope/branches/2.10/lib/python/OFS/tests/testFileAndImage.py
-=- Modified: Zope/branches/2.10/doc/CHANGES.txt =================================================================== --- Zope/branches/2.10/doc/CHANGES.txt 2006-06-28 11:20:16 UTC (rev 68875) +++ Zope/branches/2.10/doc/CHANGES.txt 2006-06-28 11:24:05 UTC (rev 68876) @@ -18,6 +18,9 @@
Bugs Fixed
+ - OFS Image: Image and File now both support simple unicode objects + for data (they function the same as strings for data). + - OFS Application: Updated deprecation warnings. Support for '__ac_permissions__' and 'meta_types' will be removed in Zope 2.11, 'methods' support might remain longer.
Modified: Zope/branches/2.10/lib/python/OFS/Image.py =================================================================== --- Zope/branches/2.10/lib/python/OFS/Image.py 2006-06-28 11:20:16 UTC (rev 68875) +++ Zope/branches/2.10/lib/python/OFS/Image.py 2006-06-28 11:24:05 UTC (rev 68876) @@ -43,7 +43,6 @@ from cgi import escape import transaction
-StringType=type('') manage_addFileForm=DTMLFile('dtml/imageAdd', globals (),Kind='File',kind='file') def manage_addFile(self,id,file='',title='',precondition='', content_type='', REQUEST=None): @@ -231,7 +230,7 @@ RESPONSE.setStatus(206) # Partial content
data = self.data - if type(data) is StringType: + if isinstance(data, basestring): RESPONSE.write(data[start:end]) return True
@@ -302,7 +301,7 @@ 'Content-Range: bytes %d-%d/%d\r\n\r \n' % ( start, end - 1, self.size))
- if type(data) is StringType: + if isinstance(data, basestring): RESPONSE.write(data[start:end])
else: @@ -401,7 +400,7 @@ self.ZCacheable_set(None)
data=self.data - if type(data) is type(''): + if isinstance(data, basestring): RESPONSE.setBase(None) return data
@@ -481,7 +480,7 @@ if headers and headers.has_key('content-type'): content_type=headers['content-type'] else: - if type(body) is not type(''): body=body.data + if not isinstance(body, basestring): body=body.data content_type, enc=guess_content_type( getattr(file, 'filename',id), body, content_type) return content_type @@ -490,7 +489,7 @@
n=1 << 16
- if type(file) is StringType: + if isinstance(file, basestring): size=len(file) if size < n: return file, size # Big string: cut it into smaller chunks @@ -617,7 +616,7 @@ return result
data = self.data - if type(data) is type(''): + if isinstance(data, basestring): RESPONSE.setBase(None) return data
Modified: Zope/branches/2.10/lib/python/OFS/tests/testFileAndImage.py =================================================================== --- Zope/branches/2.10/lib/python/OFS/tests/testFileAndImage.py 2006-06-28 11:20:16 UTC (rev 68875) +++ Zope/branches/2.10/lib/python/OFS/tests/testFileAndImage.py 2006-06-28 11:24:05 UTC (rev 68876) @@ -252,7 +252,15 @@ verifyClass(HTTPRangeInterface, File) verifyClass(WriteLockInterface, File)
- + def testUnicodeWithIndexHtml(self): + # Introduced to help test the fact that Image.py has been + # changed to be lenient towards any basestring type, not just str + + val = u'some unicode string here' + self.file.manage_edit('foobar', 'text/plain', filedata=val) + s = self.file.index_html(self.app.REQUEST, self.app.REQUEST.RESPONSE) + self.assertEquals(s, val) + class ImageTests(FileTests): data = open(filedata, 'rb').read() content_type = 'image/gif' @@ -285,7 +293,6 @@
verifyClass(WriteLockInterface, Image)
- def test_suite(): return unittest.TestSuite(( unittest.makeSuite(FileTests),
_______________________________________________ Checkins mailing list Checkins@zope.org http://mail.zope.org/mailman/listinfo/checkins
-- Florent Guillaume, Nuxeo (Paris, France) Director of R&D +33 1 40 33 71 59 http://nuxeo.com fg@nuxeo.com
Florent Guillaume wrote:
Has this change been discussed? I don't agree with it. Unicode strings aren't byte strings. File and Image should only work with byte strings. Code dealing with them should always know and assume that they use str. Anything using unicode with them is broken or has done insufficient input validation.
Please revert, unless you can convince me otherwise.
Right. I raised that issue on IRC already too. I think this should be reverted _and_ unicode should explicitly be denied to enter those objects. Christian -- gocept gmbh & co. kg - forsterstraße 29 - 06112 halle/saale - germany www.gocept.com - ct@gocept.com - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development
--On 28. Juni 2006 13:56:58 +0200 Florent Guillaume <fg@nuxeo.com> wrote:
Has this change been discussed? I don't agree with it. Unicode strings aren't byte strings. File and Image should only work with byte strings. Code dealing with them should always know and assume that they use str. Anything using unicode with them is broken or has done insufficient input validation.
I agree. This checkin must be reverted. In fact one raise a ValueError in case the data is a unicode string. -aj
On Wed, 2006-28-06 at 14:01 +0200, Andreas Jung wrote:
--On 28. Juni 2006 13:56:58 +0200 Florent Guillaume <fg@nuxeo.com> wrote:
Has this change been discussed? I don't agree with it. Unicode strings aren't byte strings. File and Image should only work with byte strings. Code dealing with them should always know and assume that they use str. Anything using unicode with them is broken or has done insufficient input validation.
I agree. This checkin must be reverted. In fact one raise a ValueError in case the data is a unicode string.
Alrighty, reverting change. Sorry about the confusion. The more I think about this the more I agree. I'll still change the logic to do isinstance(data, str) rather than type('') == type(data) and I'll also add the explicit check for unicode and raise a ValueError if found, does this all sound ok? - Rocky -- Rocky Burt ServerZen Software -- http://www.serverzen.com News About The Server (blog) -- http://www.serverzen.net
--On 28. Juni 2006 09:36:56 -0230 Rocky Burt <rocky@serverzen.com> wrote:
I'll still change the logic to do isinstance(data, str) rather than type('') == type(data) and I'll also add the explicit check for unicode and raise a ValueError if found, does this all sound ok?
sounds good
On Wed, 2006-28-06 at 14:09 +0200, Andreas Jung wrote:
--On 28. Juni 2006 09:36:56 -0230 Rocky Burt <rocky@serverzen.com> wrote:
I'll still change the logic to do isinstance(data, str) rather than type('') == type(data) and I'll also add the explicit check for unicode and raise a ValueError if found, does this all sound ok?
sounds good
Ok, all done on 2.10 branch and trunk. But I did use a TypeError instead of a ValueError. - Rocky -- Rocky Burt ServerZen Software -- http://www.serverzen.com News About The Server (blog) -- http://www.serverzen.net
Rocky Burt wrote:
On Wed, 2006-28-06 at 14:01 +0200, Andreas Jung wrote:
Has this change been discussed? I don't agree with it. Unicode strings aren't byte strings. File and Image should only work with byte strings. Code dealing with them should always know and assume that they use str. Anything using unicode with them is broken or has done insufficient input validation. I agree. This checkin must be reverted. In fact one raise a ValueError in case the data is a unicode string.
Alrighty, reverting change. Sorry about the confusion. The more I think about this the more I agree.
I'll still change the logic to do isinstance(data, str) rather than type('') == type(data) and I'll also add the explicit check for unicode and raise a ValueError if found, does this all sound ok?
Excellent, thanks. Florent -- Florent Guillaume, Nuxeo (Paris, France) Director of R&D +33 1 40 33 71 59 http://nuxeo.com fg@nuxeo.com
participants (4)
-
Andreas Jung -
Christian Theune -
Florent Guillaume -
Rocky Burt