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

stdlib wsgiref requires argument for read() #477

Merged
merged 4 commits into from
Feb 25, 2020
Merged

stdlib wsgiref requires argument for read() #477

merged 4 commits into from
Feb 25, 2020

Conversation

suola
Copy link

@suola suola commented Feb 21, 2020

PEP3333 says:

A server should allow read() to be called without an argument, and
return the remainder of the client's input stream.

However, Python standard library wsgiref does not allow calling read
without arguments. Falcon test client falcon.testing.TestClient uses
wsgiref - therefore content length should be passed an argument to
read.

PEP3333 says:

> A server should allow read() to be called without an argument, and
> return the remainder of the client's input stream.

However, Python standard library `wsgiref` does not allow calling `read`
without arguments. Falcon test client `falcon.testing.TestClient` uses
`wsgiref` - therefore content length should be passed an argument to
`read`.
@suola
Copy link
Author

suola commented Feb 21, 2020

Short example on how to reproduce:

import falcon
import falcon.testing
import webargs
from webargs.falconparser import use_kwargs
from marshmallow import Schema


class MySchema(Schema):
    name = webargs.fields.Str()


class Resource:
    @use_kwargs(MySchema)
    def on_post(self, req, resp):
        pass


api = falcon.API()
api.add_route("/", Resource())
client = falcon.testing.TestClient(api)
resp = client.simulate_post("/", json={"name": "richard feynman"})

@lafrech
Copy link
Member

lafrech commented Feb 24, 2020

I think you're right.

Could you please add a test for this?

And add yourself to AUTHORS file.

Thanks.

@suola
Copy link
Author

suola commented Feb 24, 2020

Added a test, please let me know if it's ok for webargs and/or if the patch needs anything else.

@@ -42,3 +43,10 @@ def test_invalid_json(self, testapp):
def test_parsing_headers(self, testapp):
res = testapp.get("/echo_headers", headers={"name": "Fred"})
assert res.json == {"NAME": "Fred"}

# `falcon.testing.TestClient.simulate_request` parses request with `wsgiref`
def test_body_parsing_works_with_simulate(self, testapp):
Copy link
Member

Choose a reason for hiding this comment

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

I don't think testapp is needed, here. Is it?

Copy link
Author

Choose a reason for hiding this comment

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

You're right, it's not - updated.

@lafrech lafrech merged commit 7ccb1ad into marshmallow-code:dev Feb 25, 2020
@lafrech
Copy link
Member

lafrech commented Feb 25, 2020

Thanks.

@suola suola deleted the falcon-parser-wsgi-ref-read-requires-argument branch February 26, 2020 10:25
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.

None yet

2 participants