Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  mitsuhiko
Status:  closed| Milestone:  1.1  
 Component:  File uploads/storage  |   Version:  SVN  
Resolution:  fixed |  Keywords:   
 Stage:  Ready for checkin | Has_patch:  1
Needs_docs:  0 |   Needs_tests:  0
Needs_better_patch:  0 |  
---+
Comment (by kmtracey):

 Replying to [comment:14 bthomas]:
 > This no longer seems to be an issue after r10737. #11032 is still
 broken, but for a different reason, and unrelated to this bug.

 Yes, I confirmed r10737 fixed 'mug' being kept open.  I've also checked in
 the fix for #11032 now.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  mitsuhiko
Status:  closed| Milestone:  1.1  
 Component:  File uploads/storage  |   Version:  SVN  
Resolution:  fixed |  Keywords:   
 Stage:  Ready for checkin | Has_patch:  1
Needs_docs:  0 |   Needs_tests:  0
Needs_better_patch:  0 |  
---+
Comment (by bthomas):

 This no longer seems to be an issue after r10737. #11032 is still broken,
 but for a different reason, and unrelated to this bug.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  mitsuhiko
Status:  closed| Milestone:  1.1  
 Component:  File uploads/storage  |   Version:  SVN  
Resolution:  fixed |  Keywords:   
 Stage:  Ready for checkin | Has_patch:  1
Needs_docs:  0 |   Needs_tests:  0
Needs_better_patch:  0 |  
---+
Comment (by mitsuhiko):

 I personally think the problem should better be solved by replacing the
 custom `NamedTemporaryFile` with the standard one from the `tempfile`
 module.  I can't really see what the one in django is used for.  The
 docstring says this:

 {{{
 The temp module provides a NamedTemporaryFile that can be re-opened on any
 platform. Most platforms use the standard Python tempfile.TemporaryFile
 class,
 but MS Windows users are given a custom class.

 This is needed because in Windows NT, the default implementation of
 NamedTemporaryFile uses the O_TEMPORARY flag, and thus cannot be reopened
 [1].

 1: http://mail.python.org/pipermail/python-list/2005-December/359474.html
 }}}

 The point of the temporary file is that it disappears if `close()` is
 called.  Why is there code doing something like `open(f.name, 'r')` on a
 temporary file in django?  Maybe that could be refactored that it does not
 open the file a second time.  If not it would probably make sense to open
 the file using the windows API and by passing all `FILE_SHARE_` flags to
 it and the `FILE_ATTRIBUTE_TEMPORARY` and `FILE_FLAG_DELETE_ON_CLOSE`
 flags.  That should give a posix like behavior.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  mitsuhiko
Status:  closed| Milestone:  1.1  
 Component:  File uploads/storage  |   Version:  SVN  
Resolution:  fixed |  Keywords:   
 Stage:  Ready for checkin | Has_patch:  1
Needs_docs:  0 |   Needs_tests:  0
Needs_better_patch:  0 |  
---+
Comment (by mitsuhiko):

 I'm looking into the test failure on windows right now.  I have a semi-
 finished patch for the temporary file that could fix that problem.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  mitsuhiko
Status:  closed| Milestone:  1.1  
 Component:  File uploads/storage  |   Version:  SVN  
Resolution:  fixed |  Keywords:   
 Stage:  Ready for checkin | Has_patch:  1
Needs_docs:  0 |   Needs_tests:  0
Needs_better_patch:  0 |  
---+
Comment (by kmtracey):

 The fix for this should have fixed #11032 but for some reason it hasn't.
 Using trunk r10707 on Windows with PIL installed, running the file_storage
 tests fails due to mug being open, therefore the attempt to delete the
 temp directory fails.  Removing lines 36-43 of
 regressiontests/file_storage/models.py (the lines that access the
 dimensions) makes the test pass.  Apparently the dimensions are read from
 an already-open file but the access of them is causing the file to not be
 subsequently closed?  I don't understand what's going on here, and
 unfortunately I've run out of time to look at this for several hours, so
 I'll settle for leaving this pesky comment.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  mitsuhiko
Status:  assigned  | Milestone:  1.1  
 Component:  File uploads/storage  |   Version:  SVN  
Resolution:|  Keywords:   
 Stage:  Ready for checkin | Has_patch:  1
Needs_docs:  0 |   Needs_tests:  0
Needs_better_patch:  0 |  
---+
Changes (by mitsuhiko):

  * owner:  nobody => mitsuhiko
  * status:  new => assigned

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  nobody
Status:  new   | Milestone:  1.1   
 Component:  File uploads/storage  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Ready for checkin | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Changes (by mitsuhiko):

  * stage:  Accepted => Ready for checkin

Comment:

 Here an updated version with a proper fix, no seek (which is against the
 common interface of functions accepting both strings and files) and a
 testcase.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  nobody
Status:  new   | Milestone:  1.1   
 Component:  File uploads/storage  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Changes (by simon29):

 * cc: si...@s29.com.au (added)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  nobody
Status:  new   | Milestone:  1.1   
 Component:  File uploads/storage  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Comment (by bthomas):

 I don't think that doing a seek(0) at the end is really the correct
 behavior. It would still modify the position on the stream if it was
 nonzero before. Getting image dimensions should have *no* side effects.
 However, this is really a separate bug from leaving the file handle open,
 and I think there is already a separate bug for doing a seek(0) before
 attempting to save.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  nobody
Status:  new   | Milestone:  1.1   
 Component:  File uploads/storage  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Comment (by bthomas):

 This bug is also causing the file storage regression tests to fail on
 Windows, because an open file handle will cause the
 {{{shutil.rmtree(temp_storage_dir)}}} at the end to fail.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  nobody
Status:  new   | Milestone:  1.1   
 Component:  File uploads/storage  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Changes (by bthomas):

 * cc: btho...@ncircle.com (added)
  * needs_tests:  1 => 0

Comment:

 Added unit tests. Easier than I thought, thanks to the existing tests for
 #8534

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  nobody
Status:  new   | Milestone:  1.1   
 Component:  File uploads/storage  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  1 
Needs_docs:  0 |   Needs_tests:  1 
Needs_better_patch:  0 |  
---+
Changes (by bthomas):

  * has_patch:  0 => 1
  * needs_tests:  0 => 1

Comment:

 I have a possible fix for this. There are several problems here that make
 a clean solution difficult. The patch I attached solves the problem by
 opening a new file handle to read the image dimensions, instead of reading
 from the open file-like object passed to it. This avoids the problem of
 the FileField.file attribute transparently opening the file, which can't
 be easily detected inside of get_image_dimensions. A method like
 get_image_dimensions shouldn't have side effects, but it will alter the
 file pointer's position by reading from it. This patch prevents any side
 effects in most cases, but it will still alter a file object that is not
 based on django.core.files.base.File (this should be fixed, but it's a
 slightly separate issue).

 No unit tests yet, but it passes the existing file_storage tests.

 Calling file.close() at the end may not be necessary (the interpreter
 should clean up when it drops out of scope), but explicit is probably
 better than implicit here.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #8817: Accessing ImageField's dimensions doesn't close file

[Django]

#8817: Accessing ImageField's dimensions doesn't close file
---+
  Reporter:  tripediac | Owner:  nobody
Status:  new   | Milestone:  1.1   
 Component:  File uploads/storage  |   Version:  SVN   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  0 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Changes (by jacob):

  * needs_better_patch:  => 0
  * needs_docs:  => 0
  * stage:  Unreviewed => Accepted
  * needs_tests:  => 0
  * milestone:  => 1.1

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---