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

Prepend slash to service ID in DELETE #211

Merged
merged 2 commits into from
Jun 22, 2016

Conversation

xperimental
Copy link
Contributor

There is a small discrepancy in the current bamboo version in handling the ID between PUT and DELETE.

Create service /some/service using PUT:

$ http -v PUT http://localhost:8000/api/services/some/service Id=/some/service "Acl=path_beg /some/service"
PUT /api/services/some/service HTTP/1.1
Accept: application/json
Accept-Encoding: gzip, deflate
Connection: keep-alive
Content-Length: 56
Content-Type: application/json
Host: localhost:8000
User-Agent: HTTPie/0.9.3

{
    "Acl": "path_beg /some/service",
    "Id": "/some/service"
}

HTTP/1.1 200 OK
Content-Length: 67
Content-Type: application/json
Date: Wed, 22 Jun 2016 16:47:50 GMT

{
    "Acl": "path_beg /some/service",
    "Config": null,
    "Id": "/some/service"
}

Delete same service:

> http -v DELETE http://localhost:8000/api/services/some/service
DELETE /api/services/some/service HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Content-Length: 0
Host: localhost:8000
User-Agent: HTTPie/0.9.3



HTTP/1.1 400 Bad Request
Content-Length: 24
Content-Type: text/plain; charset=utf-8
Date: Wed, 22 Jun 2016 16:48:50 GMT
X-Content-Type-Options: nosniff

zk: node does not exist

This is because PUT and POST read the service-ID from the JSON payload and prepend a slash if the ID does not start with one. This fix just also adds the slash when using the DELETE request.

I noticed that there is also no input validation on the ID read from JSON, so if an empty string is passed it is converted to a single slash, which should probably also not happen.

@swagatha-christie
Copy link

By analyzing the blame information on this pull request, we identified @bluepeppers, @activars and @njuettner to be potential reviewers

@xperimental xperimental changed the title Prepend slash Prepend slash to service ID in DELETE Jun 22, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 39.609% when pulling 21d8f39 on xperimental:prepend-slash into 7539277 on QubitProducts:master.

@j1n6
Copy link
Contributor

j1n6 commented Jun 22, 2016

LGTM. Thank you

@j1n6 j1n6 merged commit e08654b into QubitProducts:master Jun 22, 2016
@xperimental xperimental deleted the prepend-slash branch June 22, 2016 19:17
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

Successfully merging this pull request may close these issues.

4 participants