jaystack / jaydata

Notice: this library isn't maintained anymore
http://jaydata.org
GNU General Public License v2.0
352 stars 95 forks source link

OData provider: Server root issue #134

Open t-nelis opened 11 years ago

t-nelis commented 11 years ago

Hello,

Because HTTP/1.1 specifies that e.g. http://example.com and http://example.com/ are equivalent, there is no way to tell if the resource that the API user wants to access for the OData service root is the HTTP server root or the resource referenced by the empty segment after the forward-slash (which is technically a valid resource).

RFC 2616, Section 5.1.2: "Note that the absolute path cannot be empty; if none is present in the original URI, it MUST be given as "/" (the server root)."

httpbis-p1 (Draft 24), Section 5.3: "If the target URI's path component is empty, then the client MUST send "/" as the path within the origin-form of request-target."

This is significant because the JayData OData provider builds URIs by appending sub-segments after the base service root URI. For example, depending on how the URI is interpreted (which is unspecified AFAICT, since they are considered "equivalent"), the result URI for the metadata resource might be (1) http://example.com/$metadata or (2) http://example.com//$metadata.

Currently, the query compiler of the JayData OData provider chooses the latter. However, since practically nobody uses such URIs, I believe it would be beneficial to change that behavior to the former solution for the server root only.

Notes:

agdolla commented 11 years ago

you could strip the double slashes in prepareRequest

context.prepareRequest = function(r) { r[0].requestUri = r[0].requestUri.replace('//', '/'); // or something similar };

t-nelis commented 11 years ago

Yep, as I've mentioned in the second note that's essentially what I do (although I do it outside of JayData). However there is no guarantee that this is always the correct solution since I don't control the servers.

prepareRequest helps in that I can manipulate the full requests instead of passing a mostly-equivalent service root to JayData (thanks). Doing what I think is the better way involves a little more than replacing // with / though, one would have to parse the URI to be able to special-case the server root segment separator (first slash after authority), and not forget to take relative URIs into account to. See RFC 3986.

I'd rather not parse the URI that way in application code though (not just for runtime performance, that's negligible), and I wouldn't expect all users of the API to do it. It'd be better if the OData provider would build the intended request URI to begin with, simply by having good defaults.

agdolla commented 11 years ago

on second thought it might not be such a brilliant idea to replace // with / as it'd turn http:// to http:/ we've used our odata provider in many projects and many different backends like Microsoft EntityFramework, WebApi, Microsoft CRM, SAP, etc. So far this behaviour has not created any problems whatsoever so I'm reluctant to change it.

t-nelis commented 11 years ago

Many servers ignore redundant slashes so that http://example.com/ == http://example.com//, which is why you've probably never witnessed any bad effects. One can verify this by inspecting the requests URIs generated by JayData for a http://example.com/ service, which indeed shows that they look like http://example.com//$metadata, http://example.com//EntitySet1, etc.

I guess the change could be rightfully refused if it is assumed that servers that don't ignore redundant forward slashes are rare enough that it is the responsibility of the API user to either (a) use server root URIs without trailing slash for these servers (even though that's not the recommended way per the HTTP standard) or (b) to parse and rebuild every request URI with prepareRequest. I would suggest that this should then be noted somewhere accessible if that's reasonable, possibly with some sample code.

To be entirely fair, it's not anyone's fault and the current behavior is not technically incorrect, but I would still lean toward what I think is a "better" default in the OData provider, per the POLA.