Django

Code

Ticket #5192 (assigned)

Opened 3 years ago

Last modified 4 months ago

Flatpages should be able to handle more characters (e.g. ~)

Reported by: marco.giusti@gmail.com Assigned to: ctrochalakis (accepted)
Milestone: Component: Contrib apps
Version: SVN Keywords:
Cc: yatiohi@ideopolis.gr Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

url validator is wrong and flatpages uses core.isAlphaNumericUrl in his model. expecially chars like `~' are not accepted.

Attachments

5192a.diff (1.0 kB) - added by simeon on 12/01/07 22:47:26.
Alter django.core.validators.py
5192b.diff (0.6 kB) - added by simeon on 12/01/07 22:50:21.
Alter django/contrib/flatpages/models.py
0001-contrib.flatpages-URL-can-now-contain-tilde.patch (1.5 kB) - added by ctrochalakis on 03/18/08 05:45:53.
tilde-v1
0001-add-support-for-dots-and-tildes-in-flatpages.patch (1.2 kB) - added by ctrochalakis on 08/10/09 07:57:07.
+dots+tildes, new patch on top of trunk

Change History

(follow-up: ↓ 2 ) 08/18/07 09:42:09 changed by Simon G. <dev@simon.net.nz>

  • status changed from new to closed.
  • needs_better_patch changed.
  • resolution set to invalid.
  • needs_tests changed.
  • needs_docs changed.

How is the url validator wrong? is it just the flatpages app that you're having issues with? can you give some details please?

(in reply to: ↑ 1 ) 08/20/07 07:58:39 changed by anonymous

  • status changed from closed to reopened.
  • resolution deleted.

Replying to Simon G. <dev@simon.net.nz>:

How is the url validator wrong? is it just the flatpages app that you're having issues with? can you give some details please?

this is the re used to validate urls:

url_re = re.compile(r'^https?://\S+$')

but urls can have different schemas, addictionally urls have some restrictions for unsafe chars (rfc1738).

also working whid unquoted chars instead of quoted unsafe ones it's a good thing for url dispatch sanity.

flatpages uses the following regex to check urls:

alnumurl_re = re.compile(r'^[-\w/]+$')

so how can i use unsafe (quoted) chars like tilde `~'? in rfc2396, which revises and replaces rfc1738, adds tilde to unreserved characters "...since it is extensively used on the Internet in spite of the difficulty to transcribe it with some keyboards."

12/01/07 20:24:15 changed by Simon G <dev@simon.net.nz>

  • summary changed from wrong url validator to Flatpages should be able to handle more characters (e.g. ~).
  • component changed from Validators to Contrib apps.
  • stage changed from Unreviewed to Accepted.

12/01/07 22:46:02 changed by simeon

  • owner changed from nobody to simeon.
  • status changed from reopened to new.
  • has_patch set to 1.

Simon G: Are we wanting to change the isAlphaNumericURL validator in django/core/validators.py to also accept a tilde char? If so see patch 5192a.diff... You've changed the component and summary which leads me to think that you don't want to and would rather isolate the changes to the flatpages module (although I'd argue that a url filter should allow tildes. Of course than we have to update the translations of the validation error message). If you'd rather just touch the flatpages module and not worry about the slightly misleading error message we could monkeypatch the appropriate validators re (see 5192b.diff) and call it good.

12/01/07 22:47:26 changed by simeon

  • attachment 5192a.diff added.

Alter django.core.validators.py

12/01/07 22:50:21 changed by simeon

  • attachment 5192b.diff added.

Alter django/contrib/flatpages/models.py

12/02/07 00:05:58 changed by Simon G <dev@simon.net.nz>

  • stage changed from Accepted to Ready for checkin.

Hmm.. good question. We should probably do what the RFC's say, but that involves modifying isAlphaNumericURL into something that's isAlphaNumericURLAndSomeExtraThingsLikeTilde.

I'll offload it to one of the core developers to decide what's best :)

12/02/07 06:39:03 changed by marco giusti <marco.giusti@gmail.com>

maybe isRFC2396Compliant is a good choice or simply isValidUrl

12/04/07 13:20:09 changed by jacob

  • needs_better_patch set to 1.
  • stage changed from Ready for checkin to Accepted.

This is the wrong change: it makes the alpha-numeric regular expression match "~". The right way to change this is by modifying the url field on FlatPage? itself.

01/14/08 11:31:06 changed by ctrochalakis

  • owner changed from simeon to ctrochalakis.
  • status changed from new to assigned.

Jacob, do you mean using a RegexField? for example?

01/14/08 12:12:28 changed by ctrochalakis

Oops, sorry for that. RegexField? is newforms not models.fields!

Is the correct way creating a new validator and appending it to validator_list?

03/18/08 05:45:53 changed by ctrochalakis

  • attachment 0001-contrib.flatpages-URL-can-now-contain-tilde.patch added.

tilde-v1

03/18/08 05:49:38 changed by ctrochalakis

  • needs_better_patch deleted.

Modified the URL field on flatpage application. (tilde-v1 patch)

04/04/08 02:18:10 changed by Kevin McCarthy <me@kevinmccarthy.org>

Also: attempting to add a url like /test.html as a flatpage is rejected with the following error:

  • This value must contain only letters, numbers, underscores, dashes or slashes.

I suspect it is the . (period), although the lack of trailing slash might be throwing it for a loop as well?

07/07/08 15:58:02 changed by jacob

#7663 was a duplicate.

08/17/08 13:42:07 changed by idangazit

Following conversation with malcolmt on IRC, leaving this alone for now and removing it from SprintIdeas as it is post-1.0.

I think the question here is how to validate the URLs entered for flatpages now that oldforms/validation isn't around anymore. It can go one of two ways:

  • A simplistic regex that tries to conform to "average case" usage of flatpages, coupled with docs saying what URLs are supported in flatpages. Benefits: short(er) regex, easier to maintain, secure, and debug. Downsides: people creating flatpages in unusual locations will run into trouble.
  • RFC2396-compliant regex. Benefits: flatpages can work with any valid URL, "standards compliance". Downsides: monsterously complex regex, difficult to secure, difficult to understand and maintain.

Hope this helps focus future work on this ticket!

08/17/08 13:46:50 changed by idangazit

  • milestone set to post-1.0.

Correction, didn't remove from sprintideas, but retargeted for post-1.0 and noted as such in SprintIdeas.

10/28/08 21:25:48 changed by nealmcb

I ran into the same problem while trying to verify my site to google's "webmaster tools". They tell you the url to create and you have to create it to prove that you control the site. They asked for a random url ending with ".html" (google29988dbab2fb15d3.html), which generated the error noted above from flatpages.

So certainly adding "." to the legal characters sounds to me like a good and easy idea.

Or can the validation step be made optional so the user can override it?

02/25/09 13:51:44 changed by

  • milestone deleted.

Milestone post-1.0 deleted

06/19/09 01:24:09 changed by bugmenot

It is rather old ticket and it annoys sometimes. May be we can use this way for now?

A simplistic regex that tries to conform to "average case" usage of flatpages, coupled with docs saying
what URLs are supported in flatpages. Benefits: short(er) regex, easier to maintain, secure, and debug.
Downsides: people creating flatpages in unusual locations will run into trouble.

Just add some common symbols ("~", ".", may be some more) to the regexp. It should be easy and should not make troubles for anybody. And someday, where it will be time, think about more powerful regexp, may be checking all RFC2396-valid URLs.

08/10/09 07:57:07 changed by ctrochalakis

  • attachment 0001-add-support-for-dots-and-tildes-in-flatpages.patch added.

+dots+tildes, new patch on top of trunk

08/10/09 08:01:12 changed by ctrochalakis

  • cc set to yatiohi@ideopolis.gr.

The old patch couldn't be applied anymore. Now the validation code is located in admin.py, I wrote a new patch adding support for both dots(.) and tildes(~)

11/16/09 15:00:07 changed by SmileyChris

  • stage changed from Accepted to Ready for checkin.

Add/Change #5192 (Flatpages should be able to handle more characters (e.g. ~))




Change Properties
Action