Skip to content

bpo-27777: cgi.FieldStorage can't parse simple body with Cont…#11764

Closed
ar45 wants to merge 3 commits into
python:masterfrom
ar45:issue27777
Closed

bpo-27777: cgi.FieldStorage can't parse simple body with Cont…#11764
ar45 wants to merge 3 commits into
python:masterfrom
ar45:issue27777

Conversation

@ar45
Copy link
Copy Markdown

@ar45 ar45 commented Feb 5, 2019 •
edited by bedevere-bot
Loading

…ent-Length and no Content-Disposition

  • If content length is present, read at most len bytes.
  • When reading read_lines_to_eof use read instead of readline.
  • Use self.__write even when content length is present and > 1000.

https://bugs.python.org/issue27777

@the-knights-who-say-ni
Copy link
Copy Markdown

the-knights-who-say-ni commented Feb 5, 2019

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@ar45
Fixes issue 27777: cgi.FieldStorage can't parse simple body with Cont…
1567431 
…ent-Length and no Content-Disposition

- If content length is present, read at most len bytes.
- When reading `read_lines_to_eof` use `read` instead of `readline`.
- Use `self.__write` even when content length is present and > 1000.
auvipy
auvipy suggested changes Jun 2, 2019
View reviewed changes
Copy link
Copy Markdown

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

can you check the failures?

@ar45
Updated test_cgi.py
530d87b 
Renamed `test_fieldstorage_readline` to `test_fieldstorage_read`
and test `read` instead of `readline` since we use `read`.
@ar45
Copy link
Copy Markdown
Author

ar45 commented Jun 2, 2019

@auvipy I updated. passes now.

ZackerySpytz
ZackerySpytz reviewed Jun 3, 2019
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2019-02-05-16-57-27.bpo-27777.AKUxDR.rst Outdated
@GuillaumeDerval
Copy link
Copy Markdown

GuillaumeDerval commented Apr 8, 2020 •
edited
Loading

The test added in PR #10771 does not pass with this PR:

    def test_content_length_no_content_disposition(self):
        body = b'{"test":123}'
        env = {
            'CONTENT_LENGTH': len(body),
            'REQUEST_METHOD': 'POST',
            'CONTENT_TYPE': 'application/json',
            'wsgi.input': BytesIO(body),
        }

        form = cgi.FieldStorage(fp=env['wsgi.input'], environ=env)
        self.assertEqual(form.file.read(), body.decode(form.encoding))

The tests fails with

Traceback (most recent call last):
  File "/.../myfile.py", line 17, in test_content_length_no_content_disposition
    form = cgi.FieldStorage(fp=env['wsgi.input'], environ=env)
  File "/mypydistrib/cgi.py", line 474, in __init__
    self.read_single()
  File "/mypydistrib/cgi.py", line 665, in read_single
    self.read_content()
  File "/mypydistrib/cgi.py", line 687, in read_content
    self.__write(data)
  File "/mypydistrib/cgi.py", line 702, in __write
    if self.__file is not None:
  File "/mypydistrib/cgi.py", line 498, in __getattr__
    raise AttributeError(name)
AttributeError: _FieldStorage__file

The attr self.__file must be defined before calling self.__write. Adding self.__file = self.file to read_content (as it is done in read_lines) fixes the issue:

    def read_content(self):
        if self.length > 1000:
            self.file = self.make_file()
        else:
            self.file = self.io_object()
        self.__file = self.file #line added here

@GuillaumeDerval GuillaumeDerval mentioned this pull request Apr 8, 2020
Closed
@GuillaumeDerval GuillaumeDerval mentioned this pull request Jul 2, 2020
Closed
@ar45 ar45 closed this Jul 13, 2020
@ar45
Copy link
Copy Markdown
Author

ar45 commented Jul 13, 2020

Superseded by #21457

@rr rr mannequin mentioned this pull request Apr 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@ZackerySpytz ZackerySpytz ZackerySpytz left review comments

@auvipy auvipy auvipy requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting core review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ar45 @the-knights-who-say-ni @GuillaumeDerval @auvipy @ZackerySpytz @bedevere-bot