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

Body of 412 responses not sent since ETag handling was added #1231

Closed
lafrech opened this issue Jan 3, 2018 · 11 comments
Closed

Body of 412 responses not sent since ETag handling was added #1231

lafrech opened this issue Jan 3, 2018 · 11 comments

Comments

@lafrech
Copy link
Contributor

@lafrech lafrech commented Jan 3, 2018

On HTTP error, the response includes a message such as:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>413 Request Entity Too Large</title>
<h1>Request Entity Too Large</h1>
<p>The data value transmitted exceeds the capacity limit.</p>

Since Etag handling was added, 412 error response does not include this message anymore.

Below is the test script I used.

#!/usr/bin/env python3

from werkzeug.exceptions import default_exceptions
from flask import Flask, abort


for http_exc in default_exceptions:

    app = Flask('test')
    client = app.test_client()

    @app.route('/')
    def test():
        abort(http_exc)

    response = client.get('/')
    assert response.status_code == http_exc

    if not response.get_data(as_text=True):
        raise ValueError('Missing data for HTTP error {}'.format(http_exc))

Feeding it to git bisect pointed me to 092e59a.

@mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented Jan 3, 2018

This is currently intentional because this is a side effect of how conditional requests are implemented.

@lafrech lafrech changed the title Missing response payload on 412 error since Etag handling was added Body of 412 responses not sent since ETag handling was added Jan 4, 2018
lafrech added a commit to marshmallow-code/flask-smorest that referenced this issue Jan 4, 2018
Since Werkzeug 0.14, the body of 412 responses if not sent
pallets/werkzeug#1231
eruvanos pushed a commit to eruvanos/openbrokerapi that referenced this issue Jan 4, 2018
@eruvanos
Copy link

@eruvanos eruvanos commented Jan 12, 2018

Any update on this? I had to skip some requirements of a spec because of this.

Edited:
Reference to spec: https://github.com/openservicebrokerapi/servicebroker/blob/v2.13/spec.md#api-version-header

@mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented Jan 12, 2018

I did not find a good way to fix this yet. The main issue is that from what I understand we must not send a response for precondition failed when triggered by an if-match.

LowieHuyghe added a commit to LowieHuyghe/edmunds-python that referenced this issue Jan 20, 2018
LowieHuyghe added a commit to LowieHuyghe/edmunds-python that referenced this issue Jan 20, 2018
@chenyosef
Copy link

@chenyosef chenyosef commented Feb 13, 2018

The current behavior is not compatible with w3c https status code standard (https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html) - which allow sending a body with 412 response code.

@mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented Feb 13, 2018

We can just generally send bodies again :-/

@pallets pallets deleted a comment from ravidbro Feb 13, 2018
@chenyosef
Copy link

@chenyosef chenyosef commented Feb 13, 2018

@mitsuhiko Any ETA for that ?

@mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented Feb 13, 2018

Not atm. If someone sends a pr it can be faster.

@davidism
Copy link
Member

@davidism davidism commented May 22, 2018

@lafrech doesn't indicate why they think 412 should have a body. The page linked by @chenyosef-stratoscale doesn't say anything about the body. @eruvanos doesn't say what their spec is or why it requires a body, or if that spec is compliant with the HTTP spec. @mitsuhiko says that If-Match can't send a body, but that doesn't appear to be true according to RFC 7232. I'll merge the PR since nothing seems to disallow it, but the discussion on this issue isn't very clear. 😕

@mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented May 22, 2018

I would just merge #1255. I'm not sure myself what's correct or not but since it broke people's stuff i would propose to undo this.

@davidism
Copy link
Member

@davidism davidism commented May 22, 2018

Yeah, I'm rebasing and adding a changelog now.

@mnowotka

This comment was marked as off-topic.

lafrech added a commit to marshmallow-code/flask-smorest that referenced this issue May 3, 2019
Fixes "no content on 412" issue (pallets/werkzeug#1231)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants