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

Docs: re-create pages for removed modules to document their removal. #126622

Merged
merged 9 commits into from
Nov 11, 2024

Conversation

nedbat
Copy link
Member

@nedbat nedbat commented Nov 9, 2024

This shows just two modules as an example. Will flesh it out with the rest of PEP 594 if people like the approach.
This adds a "Removed modules" page that lists modules which have been removed. Each module gets a page (with the original URL) that explains why the module is gone.

Will also need to change the redirects that were created here: https://github.com/python/psf-salt/pull/521/files

Things to notice:

  • A new "Removed modules" section, but it doesn't list all the modules in the main table of contents.
  • Links are provided to PyPI alternatives
  • Where existing suggested replacements text was available, I've put it on the removed module page.
  • The modules are still listed in the index, but as: "Deprecated: Removed in Python 3.13"
  • When the modules were first removed, existing mentions in What's New pages were changed to not try to link to the missing pages. I haven't restored those links, and am torn about whether it is worthwhile.

📚 Documentation preview 📚: https://cpython-previews--126622.org.readthedocs.build/

This shows just two modules as an example.  Will flesh it out with the
rest of PEP 594 if people like the approach.

Will also need to change the redirects that were created here:
https://github.com/python/psf-salt/pull/521/files
@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news labels Nov 9, 2024
@defnull
Copy link

defnull commented Nov 9, 2024

Please re-evaluate if recommending email for parsing multipart/form-data is really a good idea. It has many problems, some of which may impact security of web applications:

  • There is no documentation how to actually parse multipart/form-data (e.g. from a WSGI environment) with email. The parser expects messages to begin with MIME-Version: 1.0 and a matching Content-Type header, which is not the case for an HTTP request. The header with the boundary is part of the HTTP headers, not the request payload. Getting this to work with a WSGI or ASGI payload is not obvious and hard to get right.
  • email is way to forgiving and accepts way more than just RFC7578 multipart/form-data. For example: nested multipart, invalid line breaks, invalid content disposition types, segments with missing required headers, and a ton of other stuff that may be required for email parsing but should immediately be rejected when parsing multipart/form-data streams. That makes the email parser complex and slow (compared to a more focused parser) and intorduces a lot of surface area for potential security issues when used to parse web application user input.
  • The email parser holds everything in memory by default, which is fine for emails but very problematic when parsing potentially large file uploads.
  • The email parser is dangerously slow for specifically crafted input. So slow that it can be used as a denial of service attack against websites using it to parse user input. I won't go into details here, for obvious reasons.

Disclaimer: I'm the author of the multipart library.

@nineteendo
Copy link
Contributor

I think a warning would stand out more than this:

Screenshot 2024-11-09 at 18 04 16

@nedbat
Copy link
Member Author

nedbat commented Nov 9, 2024

Please re-evaluate if recommending email for parsing multipart/form-data is really a good idea. It has many problems, some of which may impact security of web applications:

Thanks. I copied those recommendations from the What's New section when the module was removed. I can omit this part of the page and let people find other advice to follow.

We should also make clear (as someone mentioned in Discourse) that the PyPI packages are third-party, not maintained by the CPython team.

I think a warning would stand out more than this:

I'm not sure how else you would like it to be mentioned. The first sentence (and soon one of the only sentences) is very clear.

@nineteendo
Copy link
Contributor

I'm not sure how else you would like it to be mentioned.

I would just like it to stand out more by using the versionremoved, warning directive or another admonition.

@nedbat nedbat force-pushed the nedbat/document-dead-batteries branch from 77a7a09 to 7fcf162 Compare November 9, 2024 20:15
@nedbat
Copy link
Member Author

nedbat commented Nov 9, 2024

I'm not sure how else you would like it to be mentioned.

I would just like it to stand out more by using the versionremoved, warning directive or another admonition.

I've added the .. deprecated-removed: directive to each page.

@nedbat
Copy link
Member Author

nedbat commented Nov 10, 2024

I've added the rest of the PEP 594 modules. Any last concerns?

@nedbat nedbat marked this pull request as ready for review November 10, 2024 16:47
@nedbat nedbat requested review from ethanfurman and a team as code owners November 10, 2024 16:47
@effigies
Copy link
Contributor

Any chance of adding distutils (PEP 632)?

@nedbat
Copy link
Member Author

nedbat commented Nov 10, 2024

I guess also imp?

@effigies
Copy link
Contributor

Ah, yeah, imp would be good, too. I don't think it got a PEP, so I didn't see it grepping through the list.

@nedbat
Copy link
Member Author

nedbat commented Nov 10, 2024

I've added distutils and imp.


This module is no longer part of the Python standard library.
It was :ref:`removed in Python 3.12 <whatsnew312-removed>` after
being deprecated in Python 3.4.
Copy link
Member

Choose a reason for hiding this comment

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

No mention of importlib as replacement?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I added a mention that the removal notice on the What's New page includes the migration advice.

@nedbat nedbat force-pushed the nedbat/document-dead-batteries branch from 66784f8 to 002be87 Compare November 11, 2024 13:19
@nedbat nedbat changed the title Docs: re-create cgi and cgitb pages to document their removal. Docs: re-create pages for removed modules to document their removal. Nov 11, 2024
@nedbat nedbat merged commit 036930d into python:main Nov 11, 2024
25 checks passed
@nedbat nedbat deleted the nedbat/document-dead-batteries branch November 11, 2024 22:54
@nedbat nedbat added the needs backport to 3.13 bugs and security fixes label Nov 12, 2024
@miss-islington-app
Copy link

Thanks @nedbat for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 12, 2024
…ythonGH-126622)

Will also need to change the redirects that were created here:
https://github.com/python/psf-salt/pull/521/files
(cherry picked from commit 036930d)

Co-authored-by: Ned Batchelder <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Nov 12, 2024

GH-126709 is a backport of this pull request to the 3.13 branch.

@AA-Turner
Copy link
Member

@nedbat do you intend to backport this to 3.12? Otherwise we shall need to retain the redirects for asynchat, asyncore, and smtpd in the 3.12 documentation only.

A

@nedbat
Copy link
Member Author

nedbat commented Nov 12, 2024

Most of the modules were removed in 3.13, so it's better to leave 3.12 as it is.

@AA-Turner
Copy link
Member

There are ~5 from 3.12, so I'd say it's worth considering if the 'version switcher' case is to be useful -- but if not I shall update the redirects PR accordingly.

A

@nedbat
Copy link
Member Author

nedbat commented Nov 13, 2024

The modules removed in 3.12 as asynchat, asyncore, distutils, imp, and smtpd. To make those behave correctly, we'd need a new PR for the docs that we'd only apply to the 3.12 branch, correct?

@AA-Turner
Copy link
Member

I don't think you'd need a new PR, just backporting this one and deleting the PEP 594 changes should work, I believe. Though maybe avoiding cherry picker would be less work, I'm not sure.

A

@nedbat
Copy link
Member Author

nedbat commented Nov 13, 2024

#126781 has the changes needed for 3.12.

nedbat added a commit that referenced this pull request Nov 14, 2024
…moval, based on GH-126622 (#126781)

[3.12] Docs: re-create pages for removed modules to document their removal, based on #126622
@dmitrijiks

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants