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

204 No Content should throw error when body is sent instead of parsing it #196

Closed
neonichu opened this issue May 21, 2015 · 11 comments
Closed
Labels

Comments

@neonichu
Copy link

If a 204 HTTP response contains a content-type header, dredd tries to parse the empty response and fails, thus breaking the test. As 204 response must not have a body, this doesn't seem like a good behaviour.

Blueprint:

### Delete a Webhook [DELETE]

+ Request
    + Headers

            Authorization: Bearer b4c0n73n7fu1

+ Response 204 (application/vnd.contentful.management.v1+json)

Dredd output:

fail: DELETE /spaces/5smsq22uwt4m/webhook_definitions/yolo duration: 673ms
fail: headers: At '/content-type' Missing required property: content-type
body: Expected body: Content-Type is application/vnd.contentful.management.v1+json but body is not a parseable JSON: Parse error on line 1:

^
Expecting 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '[', got 'EOF'
body: No validator found for real data media type 'text/plain' and expected data media type 'null'.

request: 
body: 

headers: 
    Authorization:  Bearer XXX
    User-Agent: Dredd/0.5.2 (Darwin 14.4.0; x64)

uri: /spaces/5smsq22uwt4m/webhook_definitions/yolo
method: DELETE


expected: 
headers: 
    Content-Type: application/vnd.contentful.management.v1+json

body: 

statusCode: 204


actual: 
statusCode: 204
headers: 
    accept-ranges: bytes
    access-control-allow-headers: Accept,Accept-Language,Authorization,Cache-Control,Content-Length,Content-Range,Content-Type,DNT,Destination,Expires,If-Match,If-Modified-Since,If-None-Match,Keep-Alive,Last-Modified,Origin,Pragma,Range,User-Agent,X-Http-Method-Override,X-Mx-ReqToken,X-Requested-With,X-Contentful-Version,X-Contentful-Content-Type,X-Contentful-Organization,X-Contentful-Skip-Transformation
    access-control-allow-methods: DELETE,GET,HEAD,POST,PUT,OPTIONS
    access-control-allow-origin: *
    access-control-expose-headers: Etag
    access-control-max-age: 1728000
    cache-control: max-age=0
    date: Thu, 21 May 2015 12:13:03 GMT
    server: nginx
    status: 204 No Content
    x-contentful-request-id: c0e-1770848933
    connection: keep-alive

body: 
@netmilk
Copy link
Contributor

netmilk commented May 21, 2015

Hi @neonichu,

thanks for the report and for opening this issue and for the feedback! I'm excited you're using Dredd!

I'm sorry, but I'm afraid this is not a bug. My intention here was when you declare body should be a parseable JSON in the content-type header, Dredd tries to parse it regardless the status code 204 No Content.

My opinion is that it makes no sense to send any content-type header, because there is no content. :)

See [RFC2616, §7.2.1]http://tools.ietf.org/search/rfc2616#section-7.2.1()

Any HTTP/1.1 message containing an entity-body SHOULD include a Content-Type header field defining the media type of that body.

@neonichu
Copy link
Author

Dredd is awesome, thanks a lot for making it 👍

I guess this is an edge case :)

Section "10.2.5 204 No Content" of the same RFC states:

The 204 response MUST NOT include a message-body, and thus is always terminated by the first empty line after the header fields.

Feel free to close this, though, if this works as you intended.

@netmilk
Copy link
Contributor

netmilk commented May 22, 2015

Thanks for pointing this out. Do you think that Dredd should not parse body on 204 status code and throw an error if the body is present?

@neonichu
Copy link
Author

I think so, as "MUST NOT" is defined as "absolute prohibition" in RFCs, sending a body with a 204 response is clearly an error.

@netmilk
Copy link
Contributor

netmilk commented May 22, 2015

Well, I'll rename this issue if you don't mind :) Thank you!

@netmilk netmilk changed the title 204 response with content-type header breaks test 204 No Content should throw error when body is sent instead of parsing it May 22, 2015
@milch
Copy link

milch commented May 25, 2015

I ran into a similar issue, because I am using rails for my API. rails is using rack, which is always emitting the Content-Type header, even if there is no body. To solve this, I put this script I found on StackOverflow (and adapted it) into my application.rb:

class RemoveContentType
  def initialize(app)
    @app = app
  end

  def call(env)
    status, headers, body = @app.call(env)
    bbody = body.body

    #Remove Content-Type if there is no body
    headers.delete('Content-Type') if bbody.length == 0
    return [status, headers, body] 
  end
end

And then called config.middleware.use "::RemoveContentType" in the config part of the same file.

@netmilk
Copy link
Contributor

netmilk commented May 26, 2015

@milch Thanks! 🍬

@ungrim97
Copy link
Contributor

ungrim97 commented Jul 4, 2016

I am hitting this issue as well. Returning from an OPTIONS request is failing in Dredd as it attempts to parse the JSON when there is no content.

I would argue that there doesn't have to be a Body if there is a Content-Type header, but there must be a Content-type header if there is a body.

Certainly the RFC indicates that in the event of a 204 then the Body should be ignored.

@netmilk
Copy link
Contributor

netmilk commented Jul 15, 2016

Just letting know I proposed a solution for this issue here #556 with a temporary workaround solution with use of hooks.

@honzajavorek
Copy link
Contributor

Hi folks, please check out a new proposed behavior and let me know if that would work for you: #556 (comment)

@honzajavorek
Copy link
Contributor

I believe this is already addressed by v4.7.1 and further, I just forgot to close this one. Thanks everyone involved and I'm sorry it took so long to fix this.

artem-zakharchenko pushed a commit that referenced this issue Oct 9, 2019
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

5 participants