Skip to content

gh-90996: Fixed the issue that the xmlrpc module throws a ProtocolError which may leak the password #124051

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

gh-90996: Fixed the issue that the xmlrpc module throws a ProtocolError which may leak the password #124051

wants to merge 8 commits into from

Conversation

rruuaanng
Copy link
Contributor

@rruuaanng rruuaanng commented Sep 13, 2024 •
edited by bedevere-app bot
Loading

…ay cause password leakage

Fixed the security issue mentioned in #90996, could be leaked password due to a ProtocolError thrown

@bedevere-app bedevere-app bot mentioned this pull request Sep 13, 2024
Open
Wulian233
Wulian233 reviewed Sep 13, 2024
View reviewed changes
Copy link
Contributor

@Wulian233 Wulian233 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks your PR, but there has two FAILURES:

Docs

Error: new NEWS nits:

/home/runner/work/cpython/cpython/build/NEWS:129: WARNING: py:meth reference target not found: xmlrpc.client.Transport.single_request [ref.meth]
Process completed with exit code 255.

Tests:
test_fail_no_info (test.test_xmlrpc.FailingServerTestCase.test_fail_no_info) ... ERROR
test_fail_with_info (test.test_xmlrpc.FailingServerTestCase.test_fail_with_info) ... ERROR

1 test failed again:
test_xmlrpc

== Tests result: FAILURE then FAILURE ==

@Wulian233
Copy link
Contributor

Add security label?

@Wulian233
Copy link
Contributor

After the change, the current document test still fails. Can you modify the test to match the newer code? Thanks

https://github.com/python/cpython/actions/runs/10859924577/job/30139890135

@rruuaanng
Copy link
Contributor Author

After the change, the current document test still fails. Can you modify the test to match the newer code? Thanks

https://github.com/python/cpython/actions/runs/10859924577/job/30139890135

Okay， I'm ready change

Eclips4
Eclips4 reviewed Sep 15, 2024
View reviewed changes
Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

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

Please add a test case for this.

@rruuaanng rruuaanng closed this Sep 16, 2024
@rruuaanng rruuaanng reopened this Sep 16, 2024
@rruuaanng
Copy link
Contributor Author

Please add a test case for this.

Oh, test_fail_no_info of test_xmlrpc.py confuses me.

Wulian233
Wulian233 suggested changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@Wulian233 Wulian233 left a comment

Choose a reason for hiding this comment

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

Lint

@rruuaanng
Copy link
Contributor Author

rruuaanng commented Sep 16, 2024 •
edited
Loading

Lint

I need to modify test_fail_on_info, but I don't know where to start.

@rruuaanng rruuaanng closed this Sep 16, 2024
@rruuaanng rruuaanng deleted the dev6 branch September 16, 2024 10:37
@rruuaanng rruuaanng restored the dev6 branch September 16, 2024 10:39
@rruuaanng rruuaanng deleted the dev6 branch September 16, 2024 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Eclips4 Eclips4 Eclips4 left review comments

+1 more reviewer

@Wulian233 Wulian233 Wulian233 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.

3 participants

@rruuaanng @Wulian233 @Eclips4