Skip to content
This repository has been archived by the owner on Oct 1, 2020. It is now read-only.

URL encoding for dots in file paths required #7

Open
m-mohr opened this issue Mar 2, 2018 · 9 comments
Open

URL encoding for dots in file paths required #7

m-mohr opened this issue Mar 2, 2018 · 9 comments

Comments

@m-mohr
Copy link
Member

m-mohr commented Mar 2, 2018

The R server needs the dot in file paths (e.g. /users/me/files/image.png) to be URL encoded. This doesn't follow the RFC standard for characters that need to be URL encoded. Either the R back-end should not need the dot to be URL encoded or - in case this is not possible at all - an issue should be opened for the Core API to request the dot to be URL encoded.

@flahn
Copy link
Member

flahn commented Mar 2, 2018

The underlying server implementation (plumber) assumes that if the URL path has a dot and a suffix, it is a static file and not a service. Unless the plumber implementation changes in this regard, the backend won't change in this regard.
I guess it the dot-encoding won't be a problem if the file path is provided as query parameter, but i would have to test that to be sure.
By the way, the either server needs to URL decode the path argument. Even with the "%2E" replacement for dots, the parsed argument on all servers should be fine. Its really just cosmetics, which the client needs to handle.

@m-mohr
Copy link
Member Author

m-mohr commented Mar 13, 2018

The JS client can't send %2E in the path in a browser, see https://stackoverflow.com/a/3857067 . This is because all browsers (except Firefox) convert %2E back to a dot during the request. Not sure what we can do here as it all depends on external software. Therefore we might need to change the Core API to have the path in the query.

Edit: Additionally, RFC 3986 says in 2.3 that dots in URLs should not be encoded. Therefore we should not have that specified in the API that dots need to be URL encoded.

@flahn
Copy link
Member

flahn commented Mar 19, 2018

Alternatively, we can remove the file feature from the R-Backend and provide it with an alternative Webservice, which also needs to include authentication and authorization via the bearer token. But it will be quite a bit of work and I would post-pone this.

@GreatEmerald
Copy link
Member

Why would /users/me/files/image.png be a service and not a static file to begin with? I'd imagine that if it's a service that generates a file on demand, then don't give it an extension to begin with.

@m-mohr
Copy link
Member Author

m-mohr commented Jun 7, 2018

You probably need to run it as a service as you still need to check the authorization for example. You also still need to accept uploads, like PUT /users/me/files/path/image.png...

@GreatEmerald
Copy link
Member

You can have authorisation for regular files (though that's using HTTP authentication, not sure if OpenID Connect would be different). In case of PUT, the file you are uploading doesn't yet exist on the server altogether, and is a static file on your computer; would that still be an issue for plumber?

@m-mohr
Copy link
Member Author

m-mohr commented Jun 7, 2018

But it could exist. PUT is also for replacing files. And there is also DELETE.
Anyway, there is also an issue for NodeJS, see #57. We might want to change it anyway. I think we just need to wait what other back-end providers to try it out. There was not a clear opinion today.

@GreatEmerald
Copy link
Member

Yea, I'm just trying to understand why (and to what extent) this is an issue for the R backend...

@flahn
Copy link
Member

flahn commented Jun 11, 2018

The issue would be that we are not fully compliant to the API in this regard. And I think we also raised an issue in the API whether or not to treat files as static routes (direct file access using a real dot) or describing the path information as a query parameter, where we would use URL encoding (no dot) (Open-EO/openeo-api#57)

flahn pushed a commit that referenced this issue Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants