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

Unable to access keys containing slashes under HTTP /v1/data #695

Closed
tsandall opened this issue Apr 9, 2018 · 3 comments
Closed

Unable to access keys containing slashes under HTTP /v1/data #695

tsandall opened this issue Apr 9, 2018 · 3 comments
Labels

Comments

@tsandall
Copy link
Member

tsandall commented Apr 9, 2018

If object keys under data contain slashes, callers cannot access them via the HTTP API. For example:

data.json:

{
  "foo/bar": "baz"
}
opa run -s data.json
curl localhost:8181/v1/data/foo%2Fbar

The 'problem' is that gorilla/mux is decoding URL parameters automatically. What OPA receives is foo/bar instead of foo%2Fbar. We need to investigate to see if gorrila/mux can be configured to skip decoding.

There is a related issue for repeated slashes. By default, gorrila/mux returns a 301 in cases like GET /v1/data/%2Ffoo.

srenatus added a commit to srenatus/opa that referenced this issue Apr 12, 2018
Now, GET /v1/data/foo%2fbar will construct a query string of

    data["foo/bar"]

allowing to retrieve the key mentioned in issue open-policy-agent#695.

However, I believe further work would be necessary, as the code path for
POST/PATCH/DELETE is different. (That has also blocked added a test for
this change, unfortunately.)

I also couldn't get this to work with POST /v0/data/foo%2fbar. (But I also
haven't tried very long, to be honest.)

Signed-off-by: Stephan Renatus <srenatus@chef.io>
@srenatus
Copy link
Contributor

☝️ took a quick stab at this in that branch -- it seems there's more to it than just GET...

However, the call you've mentioned would work:

$ curl 'localhost:8181/v1/data/foo%2Fbar'
{"result":"baz"}⏎

So, it's up to you -- I can't tell if this small improvement would help; or if maybe slashes in keys might not be a thing worth the hassle 🤔

@tsandall
Copy link
Member Author

This is great! This change fixes the read operations, i.e., GET v1/data, POST v1/data, and POST v1/data.

I'm going to do a quick test to see if we can easily fix the write operations (PUT, PATCH, and DELETE) as well. If you like go ahead and submit a PR for your change.

@srenatus
Copy link
Contributor

If you like go ahead and submit a PR for your change.

Done: #702. Sorry, didn't get around to add anything to that.

tsandall pushed a commit that referenced this issue Apr 12, 2018
Now, GET /v1/data/foo%2fbar will construct a query string of

    data["foo/bar"]

allowing to retrieve the key mentioned in issue #695.

However, I believe further work would be necessary, as the code path for
POST/PATCH/DELETE is different. (That has also blocked added a test for
this change, unfortunately.)

I also couldn't get this to work with POST /v0/data/foo%2fbar. (But I also
haven't tried very long, to be honest.)

Signed-off-by: Stephan Renatus <srenatus@chef.io>
@tsandall tsandall added the bug label Apr 12, 2018
tsandall added a commit to tsandall/opa that referenced this issue Apr 12, 2018
These changes extend open-policy-agent#702 to include writes. If the path segments are
escaped, the should be unescaped during parsing. This allows callers to
write keys like "foo/bar" into storage.

Fixes open-policy-agent#695

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
tsandall added a commit that referenced this issue Apr 13, 2018
These changes extend #702 to include writes. If the path segments are
escaped, the should be unescaped during parsing. This allows callers to
write keys like "foo/bar" into storage.

Fixes #695

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants