Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16812 closed Bug (fixed)

FieldsTests.test_urlfield_10 fails since r16760

Reported by: aaugustin Owned by: nobody
Component: Forms Version: master
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This failure happen in Python 2.5 and 2.6 but it apparently doesn't show up on all OSes.

The CI server reports it:

Here's what I get under OS X Lion, with Python from Mac Ports:

myk@mYk tests % PYTHONPATH=.. python2.5 runtests.py --settings=test_sqlite forms                                                            ~/Documents/dev/django-trunk/tests
Creating test database for alias 'default'...
Creating test database for alias 'other'...
...................................................................................................................................E...................................................................................................................................................................................
======================================================================
ERROR: test_urlfield_10 (regressiontests.forms.tests.fields.FieldsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/myk/Documents/dev/django-trunk/tests/regressiontests/forms/tests/fields.py", line 697, in test_urlfield_10
    self.assertEqual(url, f.clean(url))
  File "/Users/myk/Documents/dev/django-trunk/django/forms/fields.py", line 153, in clean
    self.run_validators(value)
  File "/Users/myk/Documents/dev/django-trunk/django/forms/fields.py", line 142, in run_validators
    raise ValidationError(errors)
ValidationError: [u'Enter a valid URL.']

----------------------------------------------------------------------
Ran 311 tests in 2.924s

FAILED (errors=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
myk@mYk tests % PYTHONPATH=.. python2.6 runtests.py --settings=test_sqlite forms                                                            ~/Documents/dev/django-trunk/tests
Creating test database for alias 'default'...
Creating test database for alias 'other'...
...................................................................................................................................E...................................................................................................................................................................................
======================================================================
ERROR: test_urlfield_10 (regressiontests.forms.tests.fields.FieldsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/myk/Documents/dev/django-trunk/tests/regressiontests/forms/tests/fields.py", line 697, in test_urlfield_10
    self.assertEqual(url, f.clean(url))
  File "/Users/myk/Documents/dev/django-trunk/django/forms/fields.py", line 153, in clean
    self.run_validators(value)
  File "/Users/myk/Documents/dev/django-trunk/django/forms/fields.py", line 142, in run_validators
    raise ValidationError(errors)
ValidationError: [u'Enter a valid URL.']

----------------------------------------------------------------------
Ran 311 tests in 1.941s

FAILED (errors=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
myk@mYk tests % PYTHONPATH=.. python2.7 runtests.py --settings=test_sqlite forms                                                            ~/Documents/dev/django-trunk/tests
Creating test database for alias 'default'...
Creating test database for alias 'other'...
.......................................................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 311 tests in 2.666s

OK
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

Attachments (2)

16812.patch (753 bytes) - added by aaugustin 3 years ago.
16812.2.patch (673 bytes) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by julien

  • Triage Stage changed from Unreviewed to Accepted

Yes, I can confirm that this error appears on my CI server too (Ubuntu 10.04.2 LTS with Python 2.6).

comment:2 Changed 3 years ago by apollo13

Here's what I get under OS X Lion, with Python from Mac Ports

Can't see it under 10.6 with python2.5 or python2.6 from macports.

comment:3 Changed 3 years ago by aaugustin

  • Has patch set

Django masks the exception, to get the original traceback you can edit django/core/validators.py as follows:

Index: django/core/validators.py
===================================================================
--- django/core/validators.py	(révision 16831)
+++ django/core/validators.py	(copie de travail)
@@ -126,6 +126,7 @@
                 else:
                     opener.open(req)
             except ValueError:
+                raise
                 raise ValidationError(_(u'Enter a valid URL.'), code='invalid')
             except: # urllib2.URLError, httplib.InvalidURL, etc.
                 raise broken_error

Django converts unicode url to utf-8, which is correct. However, it doesn't URLencode it. At least the path should be urlencoded. Attached patch works under python 2.5. 2.6 and 2.7.

Since this fixes a broken test, it doesn't need an additional test.

Changed 3 years ago by aaugustin

comment:4 Changed 3 years ago by aaugustin

  • Patch needs improvement set

Note that this probably isn't a perfect way to quote an URL (I haven't checked the RFC yet).

Other components should be quoted too, but with some exclusions. For instance, don't quote & and = in query: query = urllib.quote(query, '&=').

comment:5 Changed 3 years ago by aaugustin

  • Patch needs improvement unset

A quick search turns up: http://en.wikipedia.org/wiki/Percent-encoding#Current_standard:

The generic URI syntax mandates that new URI schemes that provide for the representation of character data in a URI must, in effect, represent characters from the unreserved set without translation, and should convert all other characters to bytes according to UTF-8, and then percent-encode those values. This requirement was introduced in January 2005 with the publication of RFC 3986. URI schemes introduced before this date are not affected.


New patch implements this.

Note that this piece of code will be removed in Django 1.5, I'd be OK with just skipping the test on Python < 2.7.

Changed 3 years ago by aaugustin

comment:6 Changed 3 years ago by carljm

  • Resolution set to fixed
  • Status changed from new to closed

In [16838]:

Fixed #16812 -- Percent-encode URLs in verify_exists, to fix test failures on Python 2.5 and 2.6.

comment:7 Changed 3 years ago by aaugustin

In [17803]:

[1.3.X] Fixed #16812 -- Percent-encode URLs in verify_exists, to fix test failures on Python 2.5 and 2.6. Backport of r16838 from trunk.

Note: See TracTickets for help on using tickets.
Back to Top