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

Kinto creates blank record on Content-Type:text/plain #461

Closed

Conversation

Natim
Copy link
Member

@Natim Natim commented Feb 25, 2016

As Kinto is JSON Datastore and whenever we POST text/plain instead of JSON it creates a blank record in DB instead of rejecting text/plain requests.

Request:

echo '{"data": {"description": "xyz"}}' | http POST http://localhost:8080/v1/buckets/default/collections/test-collection/records Content-Type:text/plain  -v --auth 'token:my-secret'
POST /v1/buckets/default/collections/test-collection/records HTTP/1.1
Accept: application/json
Accept-Encoding: gzip, deflate
Authorization: Basic dG9rZW46bXktc2VjcmV0
Connection: keep-alive
Content-Length: 33
Content-Type: text/plain
Host: localhost:8080
User-Agent: HTTPie/0.9.2

{"data": {"description": "xyz"}}

Response :

HTTP/1.1 201 Created
Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff
Connection: keep-alive
Content-Length: 187
Content-Type: application/json; charset=UTF-8
Date: Thu, 25 Feb 2016 10:46:57 GMT
Server: nginx/1.8.0

{
    "data": {
        "id": "1227cf0c-66a5-4620-a178-255e7f479f28",
        "last_modified": 1456417017195
    },
    "permissions": {
        "write": [
            "basicauth:157beded4ea96ad4a21ed7fd91fff2a92a5daf47f054877d2e9768259e61600c"
        ]
    }
}

@Natim Natim added the bug label Feb 25, 2016
@Natim
Copy link
Member

Natim commented Feb 25, 2016

You are right we should return a 415 Unsupported Media Type error

@Natim
Copy link
Member

Natim commented Feb 25, 2016

Surprisingly, there are tests about that in cornice: https://github.com/mozilla-services/cornice/blob/master/cornice/tests/test_validation.py#L192-L228

Natim pushed a commit that referenced this pull request Feb 25, 2016
@Natim Natim self-assigned this Feb 25, 2016
@Natim
Copy link
Member

Natim commented Feb 25, 2016

I have added failing tests so that we can investigate why cornice doesn't catch the matter.

@Natim
Copy link
Member

Natim commented Feb 25, 2016

Tests are passing with this branch: https://github.com/mozilla-services/cliquet/pull/667/files

headers=headers,
status=415)

def test_records_should_reject_unaccepted_client_accept(self):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also duplicate this code in groups/buckets and collection if you think it is a good idea to do it.

Since the code is in Cliquet it is unlikely that they will fail if the other one passes.
It is just some functional tests to make sure that what we want for Kinto is working regardless of cornice or cliquet changes.

@ayusharma
Copy link
Member Author

I got it working.

If we define the content_type while registering a service in cornice, the corince would take it as a default validator. link : http://cornice.readthedocs.org/en/latest/validation.html#content-type-validation

so I have changed code in https://github.com/mozilla-services/cliquet/blob/master/cliquet/resource/__init__.py#L70-L79


service = Service(name, path, depth=depth,content_type="application/json",
                          **viewset.get_service_arguments())

and it gives the response on content-type: text/plain as :


{"errno":107,"message":"Content-Type header should be one of ['application\/json']","code":400,"details":[{"location":"header","name":"Content-Type","description":"Content-Type header should be one of ['application\/json']"}],"error":"Invalid parameters"}

I came across this conversation Cornices/cornice#192

@Natim
Copy link
Member

Natim commented Feb 25, 2016

Waiting on mozilla-services/cliquet#667 to get merged

@Natim Natim force-pushed the 461-text-plain-body-should-be-rejected-with-an-error branch from 283cfc7 to d9ab9db Compare February 26, 2016 15:21
@Natim
Copy link
Member

Natim commented Feb 26, 2016

mozilla-services/cliquet#667 has been merged.

@Natim
Copy link
Member

Natim commented Feb 26, 2016

Test are passing on cliquet master: https://travis-ci.org/Kinto/kinto/jobs/112033653

We need a release before merging this.

@leplatrem leplatrem closed this in 97cab99 Mar 7, 2016
leplatrem added a commit that referenced this pull request Mar 7, 2016
Upgrade to Cliquet 3 (fixes #345, fixes #fixes #369, fixes #421, fixes #424, fixes #433, fixes #434, fixes #461)
leplatrem pushed a commit that referenced this pull request Mar 7, 2016
@leplatrem leplatrem modified the milestone: 2.0 Mar 10, 2016
lavish205 pushed a commit to lavish205/kinto that referenced this pull request Jun 20, 2016
…default

Do not instantiate backends if not configured (fixes Kinto#386)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants