Stibily
Stibily

Reputation: 73

Regex - Regular expression pattern failing on URL GET parameter

I'm trying to do a URL GET variable replace, however the regular expression for checking whether the variable exists in amongst other GET variables is returning true when I am expecting it to return false.

The pattern I am using is: &sort=.*&

Test URL: http://localhost/search?location=any&sort=asc

Am I right to believe that this pattern should be returning false on the basis that their is no ampersand character following the sort parameter's value?

Full code:

var sort = getOptionValue($(this).attr('id'));
var url = document.URL;

if(url.indexOf('?') == -1) {
    url = url+'?sort='+sort;
} else {
    if(url.search('/&sort=.*&/i')) {
        url.replace('/&sort=.*&/i','&sort='+sort+'&');
    }
    else if(url.search('/&sort=.*/i')) {
        url.replace('/&sort=.*/i','&sort='+sort);
    }
}

Upvotes: 0

Views: 720

Answers (6)

rodneyrehm
rodneyrehm

Reputation: 13557

The pattern I am using is: &sort=.*& Test URL: http://localhost/search?location=any&sort=asc Am I right to believe that this pattern should be returning false on the basis that their is no ampersand character following the sort parameter's value?

you are assuming right. But in your code you have else if(url.search('/&sort=.*/i')) which will match and thus still replace the value.

You should also note that your code would turn http://localhost/search?sort=asc&location=any&some=more into http://localhost/search?sort=asc&some=more. that's because because .* is greedy (trying to match as much as possible). You can avoid that by telling it to match as little as possible by appending a ? like so .*?.

That said, I believe you may be better off with a library that knows how URLs actually work. You're not compensating for parameter position, possible escaped values etc. I suggest you have a look at URI.js and replace your wicked regex with

var uri = URI(document.URL), 
    data = uri.query(true); 

data.foo = 'bazbaz'; 
uri.query(data); 

Upvotes: 0

James Kyburz
James Kyburz

Reputation: 14453

This can be done by using

url.replace(/([?&])(sort=)([^&?]*)/, "$1$2" + sort);

The match broken down

Group 1 matches for ? or & Group 2 matches sort= Group 3 matches anything that is not a & or ?

Then "$1$2" + sort will replace all 3 group matches with the first 2 + your variable

examples using string "REPLACE" instead of your sort variable

url = "http://localhost/search?location=any&sort=asc&a=z"
url.replace(/([?&])(sort=)([^&?]*)/, "$1$2" + "REPLACE");
// => "http://localhost/search?location=any&sort=REPLACE&a=z"

url = "http://localhost/search?location=any&sort=asc"
url.replace(/([?&])(sort=)([^&?]*)/, "$1$2" + "REPLACE");
// => "http://localhost/search?location=any&sort=REPLACE"

url = "http://localhost/search?sort=asc"
url.replace(/([?&])(sort=)([^&?]*)/, "$1$2" + "REPLACE");
// => "http://localhost/search?sort=REPLACE"

url = "http://localhost/search?sort=asc&z=y"
url.replace(/([?&])(sort=)([^&?]*)/, "$1$2" + "REPLACE");
// => "http://localhost/search?sort=REPLACE&z=y"

Upvotes: 0

Nicola Peluchetti
Nicola Peluchetti

Reputation: 76880

You could use this code

var url = 'http://localhost/search?location=any&sort=asc';
var vars = {};
  var parts = url.replace(/[?&]+([^=&]+)=([^&]*)/gi, function(m,key,value) {
        vars[key] = value;
    });
console.log(vars);
//vars is an object with two properties: location and sort

Upvotes: 0

user160820
user160820

Reputation: 15210

You should check

if(url.search('/&sort=.*&/i') >= 0) 

then it should work

Upvotes: 0

Linus Thiel
Linus Thiel

Reputation: 39223

Am I right to believe that this pattern should be returning false on the basis that their is no ampersand character following the sort parameter's value?

Well, you are using String.search, which, according to the linked documentation:

If successful, search returns the index of the regular expression inside the string. Otherwise, it returns -1.

So it will return -1, or 0 or greater when there is a match. So you should test for -1, not truthiness.

Also, there is no need to pass the regexes as strings, you might as well use:

url.replace(/&sort=.*&/i,'&sort='+sort+'&');

Further, keep in mind that replace will create a new string, not replace in the string (strings in Javascript are immutable).

Finally, I don't see the need for searching for the string, and then replacing it -- it seems that you always want to replace &sort=SOMETHING with &sort=SOMETHING_ELSE, so just do that:

if(url.indexOf('?') == -1) {
    url = url+'?sort='+sort;
} else {
    url = url.replace(/&sort=[^&]*/i, '&sort=' + sort);
}

Upvotes: 1

Ben Russell
Ben Russell

Reputation: 1423

The javascript string function search() returns -1 if not found, not false. Your code should read:

if(url.search('/&sort=.*&/i') != -1) {
    url.replace('/&sort=.*&/i','&sort='+sort+'&');
}
else if(url.search('/&sort=.*/i') != -1) {
    url.replace('/&sort=.*/i','&sort='+sort);
}

Upvotes: 0

Related Questions