Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

+ (Plus) in URL parameter converted to %2B #3042

Closed
nlwillia opened this issue Jun 24, 2013 · 33 comments
Closed

+ (Plus) in URL parameter converted to %2B #3042

nlwillia opened this issue Jun 24, 2013 · 33 comments

Comments

@nlwillia
Copy link

The + character is valid in url query strings as a substitute for space or its hex-coded equivalent, %20.

When Angular bootstraps, $LocationProvider.$get initializes $location. $location.$$parse uses parseKeyValue (from inside matchAppURL) to decompose the query string into parameters. It calls the built-in decodeURIComponent which replaces hex escape sequences, but it does not replace "+" with space.

As a consequence, the plus character remains for $location.$$compile to encode as %2B which changes the unescaped, actual value of the parameter (as returned from $location.search().paramName) from "a b" to "a+b". This is incorrect.

The parseKeyValue function should be changed to scan the decoded value for '+' and replace it with space. This function is also used for manipulation of $location.search(), and in that scenario, '+' replacement is not correct, so the change would need to be parameterized so that '+' is replaced when parsing a parameter from the browser and left alone when parsing a parameter from the application.

Verified in angular 1.0.7 and 1.1.5.

@czerwingithub
Copy link

We ran into this problem as well. For now, I put a hack in place to get around this by overriding $browser.url() to change all '+' signs in the query string to '%20'. Otherwise, as the original bug poster noted, we incorrectly get plus signs returned in the values for our query string parameters.

At the very least, there should be a fix to respect the plus signs in the original URLs, before they are rewritten by Angular. We cannot prevent users of our API from using plus signs to encode spaces when they are constructing URLs to send to us.

@BrainBacon
Copy link

@czerwingithub we are having a similar issue. How did you override $browser.url()?

@czerwingithub
Copy link

I just used a decorator on the $browser service and overrode the url method. It does depend a little on the implementation details of $browser, but I know this works for 1.1.5, and for the limited use case we have. Luckily, the watch functions for the $location service invoke $browser.url(), so search param lookups through $location work. Just a note though, the mockBrowser service is implemented slightly differently, so this fix won't work for it.. which means you can't end up testing this fix in unit tests.. just real production usage.

Here's the sample code. I don't know if this exact code compiles/works since I had to cut and paste it from several different files to give you one coherent view of it.. but it should be enough to at least give you the idea.

Oh, to use it, you just have to be sure to load the '$browserFixer' module in your app.

angular.module('$browserFixer', [])
.config(['$provide', function($provide) {
// We use a decorator to agument the behavior of the default $browser
// service. This is guaranteed to be called before anyone uses
// $browser which is great.
$provide.decorator('$browser',
[$delegate', function ($browser) {
// Copy the old url method to superUrl.
$browser.superUrl = $browser.url;

// Our new url method which does the conversion.
$browser.url = function fixedUrl() {
  // We do not do anything when used as a setter.  In this
  // case, the returned value is the $browser object itself.
  if (arguments.length > 0)
    return this.superUrl.apply(this, arguments);
  // Just convert on the way out.  We do not convert the value
  // before we invoke url() as a setter since that could cause
  // other problems.
  return convertQueryPlusSigns(this.superUrl());
};

return $browser;

function convertQueryPlusSigns(originalUrl) {
// Grab the query portion of the URL and do the rewrite.
var questionMark = originalUrl.indexOf('?');

if (questionMark < 0)
  return originalUrl;

// Split the string up into three components.  Base
// which is everything up to the first questionMark,
// query which is from questionMark to the pound sign,
// and then tail which is pound sign to the end.
var base = originalUrl.substring(0, questionMark);
var query = originalUrl.substring(questionMark);

if (query.indexOf('+') < 0)
  return originalUrl;

var tail = '';
var pound = originalUrl.indexOf('#');
if (pound > questionMark) {
  query = originalUrl.substring(questionMark, pound);
  tail = originalUrl.substring(pound);
}

query = query.replace(/\+/g,"%20");
return base + query + tail;
}

}]);
}]);

@BrainBacon
Copy link

Thanks @czerwingithub I modified the fix a bit and made it vastly simpler. Seems to work for my purposes since I don't anticipate a + anywhere else besides the search section.

    $provide.decorator('$browser', function($delegate) {
        var superUrl = $delegate.url;
        $delegate.url = function(url, replace) {
            if(url !== undefined) {
                return superUrl(url.replace(/\%20/g,"+"), replace);
            } else {
                return superUrl().replace(/\+/g,"%20");
            }
        }
        return $delegate;
    });

@czerwingithub
Copy link

Glad it helped. Yup, that works as well. This is part of our generic URL
handling libraries, so we try to follow spec as much as possible.

On Thu, Aug 8, 2013 at 3:34 PM, Brian Jesse notifications@github.comwrote:

Thanks @czerwingithub https://github.com/czerwingithub I modified the
fix a bit and made it vastly simpler. Seems to work for my purposes since I
don't anticipate a + anywhere else besides the search section.

$provide.decorator('$browser', function($delegate) {
    var superUrl = $delegate.url;
    $delegate.url = function(url, replace) {
        if(url !== undefined) {
            return superUrl(url.replace(/\%20/g,"+"), replace);
        } else {
            return superUrl().replace(/\+/g,"%20");
        }
    }
    return $delegate;
});


Reply to this email directly or view it on GitHubhttps://github.com//issues/3042#issuecomment-22363310
.

@jrabbe
Copy link

jrabbe commented Dec 9, 2013

Just FYI, you may need to use the array notation to prevent unknown provider exception after minification. That is:

    $provide.decorator('$browser', ['$delegate', function($delegate) {
        var superUrl = $delegate.url;
        $delegate.url = function(url, replace) {
            if(url !== undefined) {
                return superUrl(url.replace(/\%20/g,"+"), replace);
            } else {
                return superUrl().replace(/\+/g,"%20");
            }
        }
        return $delegate;
    }]);

@mjj1409
Copy link

mjj1409 commented Feb 15, 2014

Has anyone noticed that $routeChangeStart/$routeChangeSuccess events are fired twice in these situations, one time for the original query string containing the '+' and then again when it is converted to '%2B'? The net effect is that the associated controllers are also instantiated twice. This is happening for me in 1.2.10.

@Furizaa
Copy link

Furizaa commented Apr 24, 2014

In addition to @BrainBacon fix I also use the following snippet to fix links that have an "+" encoded whitespace in the href:

  $provide.decorator('$location', function($delegate) {
    var superRewrite = $delegate.$$rewrite;
    $delegate.$$rewrite = function(url) {
      return superRewrite(url.replace(/\+/g, '%20'));
    };
    return $delegate;
  });

Don't forget to use the array syntax if you need minification.

@bpossolo
Copy link

bpossolo commented May 7, 2014

any idea why this issue hasn't been addressed in 11 months? seems like it affects everyone and is an easy fix

@gkop
Copy link

gkop commented Jun 17, 2014

+1. For what it's worth I am getting around this by using commas instead of pluses.

@arkarkark
Copy link

also +1

@honzajde
Copy link

+1

1 similar comment
@altmind
Copy link

altmind commented Jun 30, 2014

+1

@kirka121
Copy link

are you typing +1's just to increase your comment count?

@altmind
Copy link

altmind commented Jun 30, 2014

its ok as long as github dont have any other means to vote vor an issue. google code had stars, jira had votes, github has nothing for that.

@arkarkark
Copy link

@jrabbe I really liked your workaround, thank you.

My karma tests failed with it (as forewarned by @czerwingithub) but this small change made it work both in prod and my tests.

superUrl = angular.bind($delegate, $delegate.url)

I'd still really like to see valid url parameters supported by angular though.

@IgorMinar
Copy link
Contributor

@caguillen214 can you take a look at this one please?

Write a test for both initializing $location and setting $location.search with a search query containing + characters.

The fix will most likely be to just replace all + with %20 in the parseKeyValue fn

@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.16, Backlog Jul 16, 2014
@caguillen214
Copy link

@IgorMinar will do

@caitp
Copy link
Contributor

caitp commented Jul 17, 2014

@caguillen214 I could take this one if you don't have time for it

@LorDOniX
Copy link

LorDOniX commented Sep 4, 2014

Hi,
I need help with these two cases:

If I use url parameter, for example "search", which will contain a text from a user.
User will be searching "one + one" and then in the url there will be
"?search=one%20%20%20one", which will be decoded as "one one".

I think this change breaks many peoples codes, because in this case plus sign should be encoded as %2B. Also, what about SEO ? What If I use plus sign to distinguish one unique URL from another ?

Thanks for reply.

@firlaj
Copy link

firlaj commented Sep 4, 2014

Hello,
This fix causes me a problem. In my app I have a value in search param, which can contain a plus sign, for example "flat-size=2+1". This is a common way how to describe flat size in my country and it is important for SEO that the value is exactly like this. The fix changes the parameter to "flat-size=2 1", which is nonsense (and broke my app :-( ).
Also, as @LorDOniX mentioned above, it can change values from search text inputs, which does not seem right.

In the opening post to this issue, @nlwillia writes:

"The parseKeyValue function should be changed to scan the decoded value for '+' and replace it with space. This function is also used for manipulation of $location.search(), and in that scenario, '+' replacement is not correct, so the change would need to be parameterized so that '+' is replaced when parsing a parameter from the browser and left alone when parsing a parameter from the application."

The fix does not do anything like that. Maybe it should?

@nlwillia
Copy link
Author

nlwillia commented Sep 5, 2014

Take a look at this demo. So far as I can tell, this is now being handled correctly. If your application assigns a value containing "+" via the $location.search API, then angular encodes it with %2B in the URL and preserves the literal plus character for the application. But "+" in a URL whether manually in the browser's address bar or from an internal link gets interpreted the same as %20 (space) at the API level, and that's how it should be. I verified this in both hashbang and HTML5 modes.

@LorDOniX If a search for "one + one" is being encoded in the URL as "one%20%20%20one", then there is something wrong with your encoding process. An entered "+" character should encode as %2B because "+" is a reserved character in URL parameters.

@bpossolo
Copy link

bpossolo commented Sep 6, 2014

The behaviour should be configurable either on the locationProvider or as an argument to $location.search({}).
Some people want $location.search( { "fl": "foo bar' } ) to become /path?fl=foo+bar others want /path?fl=foo%20bar

@LorDOniX
Copy link

Hi,
yes $location.search working well, but problem is with $location.url.
Url:

/en/search/for-sale/apartments/praha-9?disposition=1+kt

is encoded to:

/en/search/for-sale/apartments/praha-9?disposition=1%20kt

Please take a look at "disposition" parametr, which is encoded as a space, not a plus sign.

@nlwillia
Copy link
Author

@LorDOniX I get where you're coming from, but the use of %20 vs. + in an encoded URL is not really a bug. From a browser or robot perspective, the two encodings are interchangeable. Or put differently this may be a change, but it isn't really a regression...at least not the sort of thing that would require that this issue be reopened. This is just my opinion as a fellow angular user, but I'd suggest that you'll get more traction by logging a new issue asking for an enhancement to $locationProvider allowing you to configure which encoding style is used.

@micmmakarov
Copy link

Solution that was the best for me:

  $provide.decorator "$browser", [
    "$delegate"
    ($delegate) ->
      superUrl = $delegate.url
      $delegate.url = (url, replace) ->
        superUrl().replace /\+/g, "%2B" if url is 'undefined'
      $delegate
  ]

Just replaces +es to %2B which fix everything and makes them as "+" in the url

@dinofx
Copy link

dinofx commented Jan 21, 2016

This bug is invalid. A '+' in a URL always means '+'. It's a valid character in the path and query. Space is always encoded as %20.

The reporter has made the common mistake of confusing the encoding used for "application/x-www-form-urlencoded" content in the BODY of an HTTP request, with the encoding used for URLs.

@jrabbe
Copy link

jrabbe commented Jan 22, 2016

From W3:

Within the query string, the plus sign is reserved as shorthand notation for a space. Therefore, real plus signs must be encoded. This method was used to make query URIs easier to pass in systems which did not allow spaces.

So while you are correct that + is a valid character in path and query, the fact remains that many of us have to interact with legacy systems and support the world as it looks, not like we want it to look.

@dinofx
Copy link

dinofx commented Jan 22, 2016

That link is far from being a meaningful specification and just shows another confused developer who probably doesn't know the difference between a URL and URI. The fact that it says some systems allow spaces in URIs should say something about its accuracy.

Web browsers use URLs. Try pasting this into your web browser:
github.com//issues/3042?encode=this query

Every browser in the world will automatically convert the SPACE to %20.

The URL specification has been around for over 20 years (RFC1738). They all agree that the only way to encode characters is using %.

@nlwillia
Copy link
Author

Probably better to have this discussion under #13815.

@jrabbe
Copy link

jrabbe commented Jan 22, 2016

@dinofx it may not be a meaningful specification, but I'm sure you're aware of what W3 is, and the about this document page specifically says:

This document defines the syntax used by the World-Wide Web initiative to encode the names and addresses of objects on the Internet.

That said, the problem seems to be not so much the decoding, but that $location.$$compile encodes a plus sign as %2B which is perfectly valid, but shouldn't happen since + is a valid character in a URI and doesn't need to be percent encoded.

@bpossolo
Copy link

@jrabbe is correct here. the plus symbol is perfectly valid in the query portion of the string as an alternative to %20

@vijaybabu4589
Copy link

vijaybabu4589 commented Aug 24, 2021

use new HttpUrlEncodingCodec().encodeValue("+91")

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.