solidusjs / solidus

A simple server that generates pages from JSON and Templates
MIT License
28 stars 7 forks source link

Encoding of commas in resource URLs #120

Closed pushred closed 9 years ago

pushred commented 9 years ago

In requesting a resource containing a comma in it's query string I noted that I wasn't getting the expected response. Trying the request from a separate HTTP client it worked however. So I tried URL encoding the commas in that client and noted the same result I'm getting in Solidus. Solidus is encoding the commas and they aren't being decoded on the other end.

Referencing RFC 3986 this particular API would appear to be in the wrong, as commas are a reserved character and should be encoded when used outside of path segments. But there's some ambiguity in regards to the query string:

However, as query components are often used to carry identifying information in the form of "key=value" pairs and one frequently used value is a reference to another URI, it is sometimes better for usability to avoid percent-encoding those characters.

Testing with some other APIs with some other examples of commas in the query string (Facebook, YouTube, WP-API) my requests work with and without encoding. So I'm not sure what examples out there might fail if we submit unencoded commas. Even if it did work though I'm not a fan of perpetuating a bad practice, so there's two other options:

As convenient as the first option would be, the right thing to do is outreach to API providers to push through fixes. That might not always be an option, but further instances of this should be rare. I've reached out to the team behind the API in question here to see what they have to say. But of course such changes rarely happen quickly. So I think we need an option for this behavior so as to not block use of such APIs.

Such an option would be a good fit for the resource config objects appearing either in the JSON frontmatter of views or the global auth.json config. Perhaps a simple string of characters that should be decoded. For example:

"http://example.com/api/*": {
    decode: ",;"
}
joanniclaborde commented 9 years ago

And a 3rd option: don't encode anything, use the resource URL directly, it's the developer's responsibility to format his URL properly...

pushred commented 9 years ago

I'm fine with that for now.. since most APIs handle commas encoded or not we shouldn't break any requests. But if we do at least there's a way to fix it on a case-by-case basis. I don't like that developers are forced to work with encoded characters, that really sucks from a usability perspective. But we can address that later on with some features around resource fetching in general. JSON config is probably not sufficient.

joanniclaborde commented 9 years ago

I'm thinking of only encoding ?&=, but I think we should wait for another feature for that, as you were saying...