Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Liam Morland’s picture

Project: Link » Clientside Validation
Version: 7.x-1.0 » 7.x-1.x-dev

Clientside Validation uses the jQuery URL validator, which only considers full URLs to be valid. Clientside Validation needs to validate internal paths as well.

attiks’s picture

Good point, we clientside_validation will need a way to know what kind of URL is expected (internal or external), for external we can use the rule as it's defined right know, for internal validation will be tricky: we can check against the menu system, but it will require an AJAX callback, or we can just try to validate it using the current domain name.

Liam Morland’s picture

Yes, doing it properly would require an Ajax call. It depends how good the validation needs to be. For example, you could accept any URL like ^node/[0-9]+$, but that would accept node numbers that don't exist. Of course, internal paths might not be to node number URLs. This would be an easy quick fix that doesn't require Ajax.

kpaxman’s picture

Unfortunately, right now my solution is to disable clientside validation for URL fields.

To your point about knowing what kind of URL is expected...I actually want to allow both internal and external URLs.

attiks’s picture

#3 limiting this to only node/n is a quick fix, but it's possible people want to link to other pages like front, views, ....

An ajax callback should be pretty easy to implement, the menu_router knows if a path exists or not, so there's probably not much code needed.

lathan’s picture

I have started work on this patch I require this functionality urgently. Can someone please point me in the direction of where this "jQuery URL validator" is used in the code so that i can implement the call back to make this check.

attiks’s picture

I think the easiest will be to create a new rule, see line 1269 in clientside_validation.js for an example, name the new rule 'drupalURL'

To add the right rule (url or drupalurl) you have to edit http://drupalcode.org/project/clientside_validation.git/blob/refs/heads/... and see which url types are allowed and set the right rule.

If you want to support both internal and external URL's in the same field, you have to alter your callback so it checks for a valid URL as well.

lathan’s picture

Status: Active » Needs review
FileSize
2.51 KB

Initially I was assuming that we would just implement the callback in the patch above if the external url validation failed. But I have gone ahead and adjusted the callback to also check external urls... ive made the changes that I could understand however im not sure what needs to be done in clientside_validation_html5.module I see the call in to _clientside_validation_set_url($el_name, $el_title, $js_rules); however that seems only setting the error message.

Please can I get some more feed back here to take this further.

Thanks.

lathan’s picture

understood a few more things that i included.

lathan’s picture

yay here is a working patch.

Jelle_S’s picture

Status: Needs review » Needs work

I think we're nearly there with this patch. The only thing I see now is that for every url an ajax callback is fired (including for absolute urls), while we can validate absolute urls on the client side (as we do now).

So here are my remarks:

+++ b/clientside_validation.jsundefined
@@ -1264,6 +1264,25 @@
     // Support for phone
+    jQuery.validator.addMethod("drupalURL", function(value, element) {

// Support for Drupal urls

+++ b/clientside_validation.jsundefined
@@ -1264,6 +1264,25 @@
+    jQuery.validator.addMethod("drupalURL", function(value, element) {

First validate the url on the client side, so this will become something like:

jQuery.validator.addMethod("drupalURL", function (value, element) {
  // Validate url on clientside.
  if (jQuery.validator.methods.url(value, element) {
    return true;
  }
  else {
    // Your current code.
  }
}, jQuery.format('Please fill in a valid url'));
+++ b/clientside_validation.moduleundefined
@@ -1569,3 +1577,33 @@ function clientside_validation_library_alter(&$libraries, $module) {
+  // Checks a path exists and the current user has access to it.
+  elseif (drupal_valid_path($path)) {

It seems a bit odd to me that users can only enter paths they have access to (even if they are valid otherwise). But since that's the way drupal_valid_path() works, I guess there's not much we can do about it.

lathan’s picture

jQuery.validator.addMethod("drupalURL", function (value, element) {
  // Validate url on clientside.
  if (jQuery.validator.methods.url(value, element)) {
    return true;
  }
  else {
    // Your current code.
  }
}, jQuery.format('Please fill in a valid url'));

when including your change I get this error.

TypeError: 'undefined' is not a function (evaluating 'this.optional(element)')

kaidawai’s picture

i spot some misplaced braces

1. jQuery.validator.addMethod("drupalURL", function (value, element)) {
->jQuery.validator.addMethod("drupalURL", function (value, element){
2. if (jQuery.validator.methods.url(value, element) {
-> if (jQuery.validator.methods.url(value, element)) {

hth

lathan’s picture

yeah that was a typo when typing this up.

    // Support for drupalURL
    jQuery.validator.addMethod("drupalURL", function(value, element) {
      // Validate url on clientside.
      if (jQuery.validator.methods.url(value, element)) {
        return true;
      }
      else {
        var result = false;
        jQuery.ajax({
          'url': Drupal.settings.basePath + 'clientside_validation/drupalURL',
          'type': "POST",
          'data': {
            'value': value
          },
          'dataType': 'json',
          'async': false,
          'success': function(res){
            result = res;
          }
        });
        return result.result;
      }
    }, jQuery.format('Please fill in a valid url'));

I still get.
TypeError: 'undefined' is not a function (evaluating 'this.optional(element)')

kaidawai’s picture

from the api:
----
// http://docs.jquery.com/Plugins/Validation/Methods/url
url: function(value, element) {
// contributed by Scott Gonzalez: http://projects.scottsplayground.com/iri/
return this.optional(element) || /^(
----
"this" is not bound correctly.
-> maybe
jQuery.validator.methods.url.call(this, value, element);
->or
jQuery.validator.methods.url.call(self, value, element);
->or..
some.object.url(value,element);

you need the object you are binding to..

kaidawai’s picture

a little more explanation because this is a common misconception in regard to js. js is an ai language.
it has a tree with nodes - almost everything is a node. now you can take almost every node and attach it whereever you want(or need it) in the tree. this is called binding.
your function 'url' is naturally a node and got bound to another node somewhere in the tree. now you call it, it in turn calls "this.optional(element)". now it complains that it can't execute it(the node called "optional"). (the errormessage is clear): there simply is no executable node called "optional" attached to the node('this") it looks for it. "this" of course is the node you attached the url function object.

so what you have to do, is bind the url function object to a node(object) with an executable childnode "optional" and js will be happy again. if necessary you can explicitely bind by passing the node as first parameter of the "call" childnode of the url function node and execute it; jQuery.validator.methods.url.call(the_object_goes_here, value, element);

but i guess there is an easier way once you know what object you are intend to deal with... find your object the one you are trying to do something with and you are most likely good.
hth

ps: there is another approach: every object can be an "actor" but at any given time there is just one active object. "this" is the alias of the single active object.
the actor is executing jQuery.validator.methods.url methode. there it finds that it has to execute the methode "this.optional". but "this"(that is the current actor) doesn't have "optional". error: this.optional is undefined. -> jQuery.validator.methods.url can only be executed by an object with the "optional" methode. the acting object has to ask another object with the necessary skills (in that case a methode "optional") to take over: binding.

Jelle_S’s picture

Ah of course, I didn't think about the context. Looking at the code in the check() function of jquery.validate (https://github.com/jzaefferer/jquery-validation/blob/master/jquery.valid...) you should do something like this:

jQuery.validator.addMethod("drupalURL", function (value, element) {
  // Validate url on clientside.
  if (jQuery.validator.methods.url.call(this, value, element) {
    return true;
  }
  else {
    // Your current code.
  }
}, jQuery.format('Please fill in a valid url'));

Which is basically what @kaidawai said in #15.

lathan’s picture

@Jelle_S even when i use this or self in

jQuery.validator.methods.url.call(this, value, element)

It still breaks and none of the validation is run on the url and I get "TypeError: 'undefined' is not a function (evaluating 'this.optional(element)')"

Maybe one of you guys could load the patch up and try?

kaidawai’s picture

Find the Object you need!

Without looking closer at it, i have a uncertain feeling that it might have the url function, all the other validator methodes and maybe some of them even overwritten and customized(in which case you might want to use the customized ones).... Can't look deeper into it now.

kaidawai’s picture

here you go:
this is all you ned just as @Jelle_S wrote in http://drupal.org/node/1666710#comment-7260218

  jQuery.validator.addMethod("drupalURL", function(value, element, param) {
       var checkInternalUrl = function (){
                return false;
       };
       return jQuery.validator.methods.url.call(this,value,element) || checkInternalUrl() ;
 }, jQuery.format('Please fill in a valid url'));

works fine.

kaidawai’s picture

and that might be the whole code:

jQuery.validator.addMethod("drupalURL", function(value, element, param) {
      var result = false;
      if(jQuery.validator.methods.url.call(this, value, element)) return true;
      jQuery.ajax({
        'url': Drupal.settings.basePath + 'clientside_validation/drupalURL',
        'type': "POST",
        'data': {
          'value': value
        },
        'dataType': 'json',
        'async': false,
        'success': function(res){
        result = res;
        }
      });
      return result.result;
    }, jQuery.format('Please fill in a valid url'));

remark: the callback can also be reduced: (but i don't see where the serverside check is) :

function _clientside_validation_url_validation_callback() {
  $path = check_plain($_POST['value']);
  $result = array(
      'result' => filter_var($path, FILTER_VALIDATE_URL) ||  //<- is this really necessary? js does this already!
                   drupal_valid_path($path) ||  //<- expensive: db query.
                   drupal_valid_path(drupal_lookup_path('source', $path)),  //<- 2nd time a dbquery
   );
  drupal_json_output($result);
}

also see: http://api.drupal.org/api/drupal/includes!common.inc/function/url_is_ext...
further there is some inconsistency: filter_var does a plausibility check(and it returns true even if it doesn't exist). drupal_valid_path however tests if user can actually acces the path.

Can you point me to the server side check that is done when js is turned off?

Jelle_S’s picture

I'm not sure what the server side check is in the OP's case, but looking at Drupal's valid_url(), relative paths are not even validated whether or not they exist..., makes me think if we even need an ajax callback...

Any thoughts?

kaidawai’s picture

Hi,
the way i see it is that everything that happenes clientside can be faked(turned off). So the crucial check is the serverside check. Clientside is for convinience and to reduce bandwith and resource demands... Once there is both Server and Clientside Check it is crucial that both are in sync otherwise the user gets a hint, does everything correctly according to clientside but when he submits he gets an error. Or clientside prevents a completely valid Form from being sent. Such behavior is at best confusing for the enduser and worst renders a webappliction unusable.

Hence the question regarding the serverside check(a reproducable usecase).

Lets craft an usecase. Some comunity portal has user profiles. The form provides a field for a link to a website: an url. For security reasons, this field needs checking but no necessity to check for a dead link let alone drupal access rights.
A clientside check that accepts internal URLs might break the security concerns and asking the drupal database for registered paths for the user makes no sence at all. I for one think that such kind of usecases are rather common. IMHO breaking this functionalty of an URL check might not be the best thing to do.

I can imagine a lot of usecases that require further checking(e.g. media upload/proxiing).

Looking at the OP, the issue was: internal URLs are rejected but should also be treated as valid. Fair enough. To solve that in an URL like way, relative paths need to be accepted like @Jelle_S says and maybe also those "public://" and "private://" ones. Maybe even as a separate check(internal/external like @attiks suggests in #2 ). Thats how i understand the OP's request. Such check can be imho easily be done via js without any ajax involved.

Checking for user acces however has commonly something to do with uploading. Otherwise the user submitting the url is different from the one following the link and hence the acces rights(possible paths). -> Besides upload( which requires more checking) like cases, checking for possible paths of the user submitting the url to me seems to do the wrong thing!

Thats where i am stuck. What use case? Wich serverside check produces an inconsitency with the clientside validation? Like @Jelle_S i can't see why ajax should be necessary to mimic the valid_url behavior.

mrfelton’s picture

Status: Needs work » Needs review
FileSize
3.67 KB

Updated patch here that adds the changes discussed earlier, as well as supporting links that contain query strings and fragment identifiers. Based quite a lot on this discussion http://drupal.stackexchange.com/questions/21782/validate-a-path-within-t.... The issue about wether or not access should be checked still stands (I think it should not be checked).

attiks’s picture

#24 looks good, so only thing left is access check or not. I think if we want to limit it to accessible URLs it has to be done by the module as well (and first)

Leaving this issue open for a couple of days, so people can react.

fastangel’s picture

Hi,

I extended the path created in #24. I exclude keyup for url. I think that is a good solutions for resolve this problem.

fastangel’s picture

Sorry the last patch is wrong. Now the right path.

fastangel’s picture

New patch. I fixed a little bug with twi drupalURL.

jaydee1818’s picture

Issue summary: View changes

Patch #28 causes my admin toolbar to randomly disappear. Some sort of js conflict going on I assume.

Jelle_S’s picture

Status: Needs review » Fixed

Fixed in latest dev version.

  • Jelle_S committed 6cc7558 on 7.x-1.x authored by fastangel
    Issue #1666710 by jucallme, fastangel, mrfelton | stano.lacko: Fixed...

  • Jelle_S committed 6cc7558 on 7.x-2.x authored by fastangel
    Issue #1666710 by jucallme, fastangel, mrfelton | stano.lacko: Fixed...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.