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

Fixed content size parsing issue #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

g3rb3n
Copy link

@g3rb3n g3rb3n commented Nov 12, 2018

Added extra check for request_headers['content_length']

@prosperocu
Copy link

prosperocu commented Feb 28, 2019

Hi g3rb3n. Your solution don't work on Django 2.1.7 because request_headers["content_length"] can be an empty string too. One possible solution can be don't parsing to int this header:

    req_size = request_headers["content_length"]

Im looking on the code, and this var is not needed for other function, is only for info. But, in other future case this can be help:

    req_size = 0
    if "content_length" in request_headers and request_headers["content_length"]:
        req_size = int(request_headers["content_length"])

@g3rb3n
Copy link
Author

g3rb3n commented Mar 1, 2019

If request_headers["content_length"] is an empty string
"content_length" in request_headers will evaluate to True
request_headers["content_length"] will evaluate to False

So int(request_headers["content_length"]) won't be called and req_size will become 0 from the else clause.

Your proposal is logical equivalent. Or am I missing something?

@prosperocu
Copy link

prosperocu commented Mar 1, 2019 via email

@g3rb3n
Copy link
Author

g3rb3n commented Mar 1, 2019

I had the same problem when request_headers["content_length"] was returning an empty string. This PR fixes this issue. It is still not merged though by @elafarge, so you should indeed use my master version if you have the same issue.

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.

2 participants