Grails

URL Domain Field Validation Fails

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.0-RC1
  • Fix Version/s: 1.0-RC2
  • Component/s: Persistence
  • Labels:
    None
  • Environment:
    Linux; Sun JVM 1.6.0_02

Description

A domain field constraint specified as:

originURI(nullable: true, url: true)

Is causing failed validation for both empty fields and for perfectly acceptable URLs.

Issue Links

Activity

Hide
Randall R Schulz added a comment -

Accoding to Sergey Nebolsin (see http://www.nabble.com/forum/ViewPost.jtp?post=13144042&framed=y), an empty field requires the "blank: true" constraint but that the valid URL "http://localhost:8080/tau_gwi_00/clif/cb/19" improperly fails URL validation.

RRS

Show
Randall R Schulz added a comment - Accoding to Sergey Nebolsin (see http://www.nabble.com/forum/ViewPost.jtp?post=13144042&framed=y), an empty field requires the "blank: true" constraint but that the valid URL "http://localhost:8080/tau_gwi_00/clif/cb/19" improperly fails URL validation. RRS
Hide
Graeme Rocher added a comment -

I can't reproduce this. I created a class like:

class Site {
	URL url
	
	static constraints = {
		url(nullable:true, url:true)
	}
}

And it worked as expected

Show
Graeme Rocher added a comment - I can't reproduce this. I created a class like:
class Site {
	URL url
	
	static constraints = {
		url(nullable:true, url:true)
	}
}
And it worked as expected
Hide
Sergey Nebolsin added a comment -

I tested it with:

    String url

According to supports() method in UrlConstraint, it supports String properties.

Show
Sergey Nebolsin added a comment - I tested it with:
    String url
According to supports() method in UrlConstraint, it supports String properties.
Hide
Graeme Rocher added a comment -

Unfortunately it appears to be a bug in apache commons validator. Your example URL doesn't work, but if you type http://grails.org it does work. I recommend you change to a type such as java.net.URL or use a regex of your own with the matches constraint

Show
Graeme Rocher added a comment - Unfortunately it appears to be a bug in apache commons validator. Your example URL doesn't work, but if you type http://grails.org it does work. I recommend you change to a type such as java.net.URL or use a regex of your own with the matches constraint
Hide
Randall R Schulz added a comment -

Sure. Kindly supply me with the regular expression that properly validates an arbitrary URL.

Seriously, this is a problem that from the (Grails') user's perspective is in Grails. It's not fair to shift the burden of bugs in components you choose to integrate onto the shoulders of your users.

RRS

Show
Randall R Schulz added a comment - Sure. Kindly supply me with the regular expression that properly validates an arbitrary URL. Seriously, this is a problem that from the (Grails') user's perspective is in Grails. It's not fair to shift the burden of bugs in components you choose to integrate onto the shoulders of your users. RRS
Hide
Sergey Nebolsin added a comment -

I finally found the problem here. It's commons-validator's UrlValidator which rejects urls with 'localhost' completely since 'localhost' is not listed by IANA as a top-level domain. I filed a Jira for the commons-validator project VALIDATOR-248 with the suggested patch.

Show
Sergey Nebolsin added a comment - I finally found the problem here. It's commons-validator's UrlValidator which rejects urls with 'localhost' completely since 'localhost' is not listed by IANA as a top-level domain. I filed a Jira for the commons-validator project VALIDATOR-248 with the suggested patch.
Hide
Sergey Nebolsin added a comment -

We finally fixed the corresponding issue in the commons-validator, and the fix will be included in 1.4 version of it. However, as far as I know there is no planned time for 1.4 release in commons-validator project.

Since the current 1.4-SNAPSHOT looks stable enough (only 7 open issues most of which are around credit card validation), one way would be include it for 1.0-RC2 version of Grails, and hope that 1.4 final will be out till Grails 1.0.

Another way is to wait until 1.4 final will be released.

Thoughts?

Show
Sergey Nebolsin added a comment - We finally fixed the corresponding issue in the commons-validator, and the fix will be included in 1.4 version of it. However, as far as I know there is no planned time for 1.4 release in commons-validator project. Since the current 1.4-SNAPSHOT looks stable enough (only 7 open issues most of which are around credit card validation), one way would be include it for 1.0-RC2 version of Grails, and hope that 1.4 final will be out till Grails 1.0. Another way is to wait until 1.4 final will be released. Thoughts?
Hide
Graeme Rocher added a comment -

since its apache license maybe we should just fork the UrlValidator from the snapshot then when 1.4 is out we can remove our fork

Show
Graeme Rocher added a comment - since its apache license maybe we should just fork the UrlValidator from the snapshot then when 1.4 is out we can remove our fork
Hide
Sergey Nebolsin added a comment - - edited

Forked UrlValidator from commons-validator project for now, and implemented the following constraint behavior:

    someUrl( url: true )
- behaves the same as previous, so only URLs with IANA-registered top-level domains are allowed (those ends with .com, .org, .us, .uk, etc. You can find the full list at IANA's site). So "http://www.google.com" is valid, and "http://localhost/" is not.

    someUrl( url: ".*\\.localdomain" )
- allows IANA TLD's as in previous case, but also allows URLs which authority parts are recognized with the provided regexp. So now "http://www.google.com", "http://aaa.localdomain", "http://bbb.localdomain" are allowed, and "http://bbb.localdomain:8080", "http://another-local-domain" are not.

    someUrl( url: ["localhost(:(\\d{1,5}))?", "my-machine"] )
- same as previous, but allows a list of regexps to recognize authority part of the URL. Now "http://www.google.com", "http://localhost/aaa/bbb", "http://localhost:8080/", "http://my-machine/" are allowed.

I think that now it's flexible enough for us.

Show
Sergey Nebolsin added a comment - - edited Forked UrlValidator from commons-validator project for now, and implemented the following constraint behavior:
    someUrl( url: true )
- behaves the same as previous, so only URLs with IANA-registered top-level domains are allowed (those ends with .com, .org, .us, .uk, etc. You can find the full list at IANA's site). So "http://www.google.com" is valid, and "http://localhost/" is not.
    someUrl( url: ".*\\.localdomain" )
- allows IANA TLD's as in previous case, but also allows URLs which authority parts are recognized with the provided regexp. So now "http://www.google.com", "http://aaa.localdomain", "http://bbb.localdomain" are allowed, and "http://bbb.localdomain:8080", "http://another-local-domain" are not.
    someUrl( url: ["localhost(:(\\d{1,5}))?", "my-machine"] )
- same as previous, but allows a list of regexps to recognize authority part of the URL. Now "http://www.google.com", "http://localhost/aaa/bbb", "http://localhost:8080/", "http://my-machine/" are allowed. I think that now it's flexible enough for us.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: