canjs / can-connect

Model layer utilities for every JavaScript framework! Assemble real-time, high performance, restful data connections.
https://canjs.com/doc/can-connect.html
MIT License
29 stars 16 forks source link

behavior data/url: encode URL parameters #374

Closed ivospinheiro closed 6 years ago

ivospinheiro commented 6 years ago

Using the behavior URL where the URL id parameter is a string that contains pound character (#) all the characters after the pound character are ignored.

Please check the following jsfiddle based on the ToDo example: https://jsbin.com/zurusiciqi/1/edit?js,console,output

If you edit a Todo and you check the logs from the console, we verify that the characters after the pound character are removed from the url, and they should be encoded. For instance when editing a todo item with ID "a#a" when saving the change on a todo the PUT is made to:

"PUT /api/todos/a {\"name\":\"mow lawn1\",\"complete\":\"false\",\"id\":\"a\"} -> handler(req,res)"

instead of something like:

"PUT /api/todos/a%23a {\"name\":\"mow lawn1\",\"complete\":\"false\",\"id\":\"a\"} -> handler(req,res)"
chasenlehara commented 6 years ago

This was brought up on the forums too (cc @pavankmv): https://gitter.im/canjs/canjs?at=5a0d5820ba39a53f1aa7af13

I think we should fix this, but in the meantime, you can work around it by making url an object and providing your own functions for making the requests. These docs have an example: https://canjs.com/doc/can-connect/data/url/url.url.html#Object

Here’s that same JS Bin with an updateData function: https://jsbin.com/rucanobemu/1/edit?js,console,output

That JS Bin works for me; let me know if that workaround doesn’t work for you @ivospinheiro.

andrejewski commented 6 years ago

It seems we can solve this by replacing the can-util/string.sub call with a new function replaceWith and perhaps have a default idPropEncoder option which defaults to encodeURIComponent. The replaceWith would essentially do:

function replaceWith (str, data, remove, replacer) { /* similar to string.sub */ }
// where
function replacer (dataValue) {
  if (dataValue) return (reqOptions.idPropEncoder || encodeURIComponent)(dataValue)
}

Edit: I said idPropEncoder but non-IDs can be serialized as url parameters, so a more appropriate override option would be urlParamEncoder (dataKey, dataValue) {}. This means the replacer also needs to pass the key of data being serialized to allow further customization.