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

Trailing Slashes in REST API endpoints Results in 405 #972

Closed
salekinsirajus opened this issue Sep 25, 2018 · 6 comments
Closed

Trailing Slashes in REST API endpoints Results in 405 #972

salekinsirajus opened this issue Sep 25, 2018 · 6 comments

Comments

@salekinsirajus
Copy link

Problem

When used the OPA REST API to upload data or policies, a slash at the endpoint causes the API to reject the request and results in a 405 (Method Not Allowed) response.

I am not sure if it is the case for every endpoint, but that might very well be the case.

How to Reproduce

Contents of data.json:

{
  "users": [
    {
      "user_id": "343",
      "role": "admin"
    },
    {
      "user_id": "124",
      "role": "superuser"
    }
   ]
}

(Optional) The above file can be validated with ./opa run data.json to make sure it's formatted correctly.

Now if we want to upload this data with the Data API, like this:

curl -X PUT localhost:8181/v1/data/ --data-binary @data.json -v

*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8181 (#0)
> PUT /v1/data/ HTTP/1.1
> Host: localhost:8181
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Length: 153
> Content-Type: application/x-www-form-urlencoded
> 
* upload completely sent off: 153 out of 153 bytes
< HTTP/1.1 405 Method Not Allowed
< Date: Tue, 25 Sep 2018 22:09:41 GMT
< Content-Length: 0
< 
* Connection #0 to host localhost left intact

However, if we remove the trailing slash (localhost:8181/v1/data), the API works as it's intended:

curl -X PUT localhost:8181/v1/data --data-binary @data.json

*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8181 (#0)
> PUT /v1/data HTTP/1.1
> Host: localhost:8181
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Length: 153
> Content-Type: application/x-www-form-urlencoded
> 
* upload completely sent off: 153 out of 153 bytes
< HTTP/1.1 204 No Content
< Date: Tue, 25 Sep 2018 22:18:57 GMT
< 
* Connection #0 to host localhost left intact

# query the data api
curl -X GET localhost:8181/v1/data | python -mjson.tool
{
    "result": {
        "users": [
            {
                "policy_name": "admin",
                "user_id": "343"
            },
            {
                "policy_name": "superuser",
                "user_id": "124"
            }
        ]
    }
}
@srenatus
Copy link
Contributor

srenatus commented Oct 1, 2018

@salekinsirajus Would you say that this needs a documentation improvement -- e.g. "Don't do this: ..." or "Do it this way... " -- or do the docs currently claim that /data/ would work; or would you expect a REST API to not care for the trailing slash and propose changing the behaviour to match that expectation?

@tsandall
Copy link
Member

tsandall commented Oct 1, 2018

405 is the wrong status code in this case.

We should review the current APIs and document the current behaviour (just in this issue would be a good start.) Then we can decide how to handle trailing slashes in a way that's consistent across all of the APIs.

@BenderScript
Copy link
Contributor

This is a well-known issue with HTTP REST frameworks. Normally a redirect is sent if there is no trailing slashes to the URL with slashes (and vice-versa). If enabled it should just work

https://godoc.org/github.com/gorilla/mux#Router.StrictSlash

BTW, some clients (such as curl?) do not follow redirects, POSTMAN does.

@BenderScript
Copy link
Contributor

I guess I can fix this one

@tsandall
Copy link
Member

tsandall commented Oct 11, 2018

Stripping the trailing slash or redirecting trailing slash URIs on the V1 APIs should be fine

@BenderScript
Copy link
Contributor

okay, I will set Router.StrictSlash and see how it plays out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants