Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved encoding for url parameters that are objects or arrays #141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Suez321
Copy link

@Suez321 Suez321 commented Oct 23, 2015

I would like to ask to merge my changes to the master.

The problem I had with the library was that if I send some object as URL parameter to crossroads, it encodes every property of that object using encodeURI method. EncodeURI method is javascript method that does not encode characters like: ~!@#$&*()=:/,;?+' becuase it is used to encode whole url so for example '/' should not be encoded because it would change meaning of url. In crossroads it is used to encode just url parameters.

In my case, I have to put some parameters in url that contains # character but this character is reserved and has significant meaning in url so it needs to be encoded. Then, it is impossible to use crossroads for that case (because encodeURI does not encode #) unless I encode this parameter before sending it to crossroads. But when I encode # to %23, then crossroads encodes % again, ending with %2523 for # instead of %23.

It is possible to change the code (as followed in commit) to encode only keys and values of url parameters with use of javascript encodeURIComponent method that encodes more characters (all without ~!*()') and is supposed to encode url parameters. This method is standard method of javascript and solves problem of using special characters as url parameters.

Additionally, when I was building crossroads.js file from sources, crossroads.js.min has been deleted and it has not been created because some file to do this was missing. This is why this commit removes this crossroads.js.min file. It should be restored based on new crossroad.js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant