Django

Code

Ticket #7635 (closed: fixed)

Opened 5 months ago

Last modified 5 months ago

File uploads sometimes incorrectly detect an infinite loop.

Reported by: lakin.wecker@gmail.com Assigned to: axiak
Milestone: 1.0 beta Component: HTTP handling
Version: SVN Keywords: 2070-fix
Cc: axiak, brosner, christian Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

just updated my django trunk install and am not getting a http://dpaste.com/60810/ that wasn't there before http://code.djangoproject.com/changeset/7814 ... now I realize that this presents a backwards incompatible change, but the error is coming from the admin interface (one that does have a file field) but I'm not uploading a file

Will upload the traceback from the paste on this ticket as well.

The models used:

from django.db import models 
from django.conf import settings


#-------------------------------------------------------------------------------
class File(models.Model):
    """
    """
    path = models.CharField(max_length=255, unique=True)

    folder = 'protected_files'
    file = models.FileField(upload_to=folder,)
    number_of_codes_to_create = models.PositiveIntegerField(
        help_text = "Use this to generate new codes on save."
    )
    max_uses_for_created_codes =  models.PositiveIntegerField(
        default=2,
        help_text = "New codes will have this many uses on generation."
    )

    def __unicode__(self) :
        return self.filename()

    class Admin:
        pass



#-------------------------------------------------------------------------------
class FileCode(models.Model):
    """
    """

    file = models.ForeignKey(
        File,
        edit_inline=models.TABULAR,
        related_name='codes'
    )
    code = models.CharField(max_length=10, core=True)

    uses = models.IntegerField(default=0)
    max_uses = models.PositiveIntegerField(default=2)


    class Admin:
        pass

Attachments

r7814_backtrace.txt (2.5 kB) - added by lakin.wecker@gmail.com on 07/04/08 21:18:04.
backtrace_from_error.txt
second_patch.diff (6.8 kB) - added by axiak on 07/04/08 23:08:22.
Increase counter from 500 to 64000.
increase_limit.diff (0.6 kB) - added by axiak on 07/04/08 23:08:54.
Increase counter from 500 to 64000.
multipartparser_recusion_check.diff (3.5 kB) - added by axiak on 07/08/08 13:13:34.
Different way to check recursion

Change History

07/04/08 21:18:04 changed by lakin.wecker@gmail.com

  • attachment r7814_backtrace.txt added.

backtrace_from_error.txt

07/04/08 21:37:53 changed by mtredinnick

  • cc set to axiak.
  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

@axiak: This looks like your department. Any ideas?

07/04/08 21:55:51 changed by axiak

Hello,

Can you post more information? Specifically, the following would be extremely helpful:

  1. What browser were you using?
  2. How were you deploying Django (test server, mod_python, mod_wsgi, fastcgi, ...)?

I'm trying to replicate this now.

07/04/08 21:57:10 changed by lakin.wecker@gmail.com

Firefox 3. It happened with both the dev server and apache/mod_python deployment.

07/04/08 22:48:07 changed by axiak

Okay, after working with lakin online for about an hour I found the issue.

The issue is that in order to prevent us from entering an infinite loop while parsing, we have a counter that essentially counts how many times we enter a segment of code.

We set the limit to something that is "high", like 500. Problem is, this counter gets incremented whenever there is an additional piece of form data.

In your example, you had a file field, so the form encoding was multipart/form-data, and you had 815 form fields because of the edit inline.

I'm working on coming up with a better way to detect an infinite loop. If there is no better place, then could we make the limit something incredibly high? (100000 comes to mind) or remove it altogether?

The reason for it is that in early development we had cases where we struck an infinite loop, and it was a bad failure mode. We figured this error would be better than that failure.

07/04/08 23:08:22 changed by axiak

  • attachment second_patch.diff added.

Increase counter from 500 to 64000.

07/04/08 23:08:54 changed by axiak

  • attachment increase_limit.diff added.

Increase counter from 500 to 64000.

07/04/08 23:11:02 changed by axiak

Disregard second_patch.diff :-).

I'm opting for increasing the limit to 64,000 for now. I think we should have some more battle testing before we remove this check outright.

07/05/08 22:01:47 changed by mtredinnick

  • has_patch set to 1.
  • summary changed from r7814 introduces bug into the admin interface. to File uploads sometimes incorrectly detect an infinite loop..
  • stage changed from Unreviewed to Accepted.

(Changed description to something approaching the real problem.)

07/06/08 19:49:44 changed by anonymous

  • cc changed from axiak to axiak, brosner.

07/07/08 18:43:58 changed by axiak

  • component changed from Uncategorized to HTTP handling.

I actually use the component...

07/07/08 18:49:30 changed by jacob

  • milestone set to 1.0 beta.

07/08/08 12:45:05 changed by christian

  • cc changed from axiak, brosner to axiak, brosner, christian.

07/08/08 13:13:15 changed by axiak

I'm uploading an attempt at a different recursion metric that should work much better.

07/08/08 13:13:34 changed by axiak

  • attachment multipartparser_recusion_check.diff added.

Different way to check recursion

07/08/08 13:24:57 changed by axiak

  • keywords set to 2070-fix.
  • owner changed from nobody to axiak.
  • status changed from new to assigned.

Adding keywords...

07/08/08 15:29:00 changed by axiak

  • stage changed from Accepted to Ready for checkin.

I think we can be happy with this fix.

07/12/08 15:43:16 changed by jacob

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [7905]) Fixed #7635: do a better job checking for infinite loops in multi-part MIME parsing. Thanks, Mike Axiak.


Add/Change #7635 (File uploads sometimes incorrectly detect an infinite loop.)




Change Properties
Action