GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-90949: Recommend `hasattr` with Expat security methods by hartwork · Pull Request #139800 · python/cpython

Conversation

@hartwork

@hartwork hartwork commented Oct 8, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Same idea (and approach) as #116278.

CC @picnixz

  • Issue: Expose Expat XML attack protection APIs #90949

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
hartwork added 2 commits October 8, 2025 20:07
@hartwork
Doc/library/pyexpat.rst: Recommend "hasattr" with Expat security methods
834a7e0
This mimics existing method SetReparseDeferralEnabled.
@hartwork
Doc/library/pyexpat.rst: Use ".. note::" with SetReparseDeferralEnabled
828f786
.. to make it consistent with the other four Expat security methods.
@bedevere-app bedevere-app Bot added docs Documentation in the Doc dir skip news labels Oct 8, 2025
@github-project-automation github-project-automation Bot added this to Docs PRs Oct 8, 2025
@github-project-automation github-project-automation Bot moved this to Todo in Docs PRs Oct 8, 2025
@bedevere-app bedevere-app Bot added the awaiting review label Oct 8, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Oct 8, 2025
Expose Expat XML attack protection APIs #90949
Closed
@hartwork hartwork changed the title gh-90949: Recommend "hasattr" with Expat security methods gh-90949: Recommend hasattr with Expat security methods Oct 8, 2025
@hartwork
Doc/library/xml.etree.elementtree.rst: Use ".. note::" for backport n…
c932512
…otes

.. to make it consistent with Doc/library/pyexpat.rst.
StanFromIreland
StanFromIreland reviewed Oct 8, 2025
View reviewed changes
Comment thread Doc/library/xml.etree.elementtree.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

StanFromIreland
StanFromIreland reviewed Oct 8, 2025
View reviewed changes
Comment thread Doc/library/xml.etree.elementtree.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@StanFromIreland
StanFromIreland requested a review from picnixz October 8, 2025 19:13
@hartwork
Doc/library/xml.etree.elementtree.rst: Add missing indent by 3
32d51e3
@hartwork
hartwork requested a review from StanFromIreland October 8, 2025 19:21
picnixz
picnixz reviewed Oct 9, 2025
View reviewed changes
Comment thread Doc/library/pyexpat.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@hartwork
Add an exclemation mark to suppress distracting links
67b5221
Related:
https://devguide.python.org/documentation/style-guide/#links

Idea by @picnixz
@hartwork
hartwork requested a review from picnixz October 9, 2025 13:36
@picnixz

picnixz commented Oct 9, 2025

Copy link
Copy Markdown
Member

I think I'm fine with this addition. However, because we talk about "backported" interface, we shouldn't push this until it's effectively backported (at least once). I'm actually unsure about the expanded docs. For instance:

image

I feel that the "note" boxes are way too intrusive... I think it's more important to have the note about the surprising values rather than the note on the availability (the versionadded should be enough). OTOH, if you ever add more protections, we'll have a huge block of notes, almost always the same. Here's what I can suggest: a simple .. seealso:: which refers to a section about "Availability testing" which indicates that some features are included in micro versions instead of .0 versions. That would reduce the vertical span of the page and be more readable for users in general. WDYT?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork

hartwork commented Oct 9, 2025

Copy link
Copy Markdown
Contributor Author

I think I'm fine with this addition. However, because we talk about "backported" interface, we shouldn't push this until it's effectively backported (at least once).

@picnixz I'm reading that as merging at least one of the unmerged backports of #139234 first — sure.

I feel that the "note" boxes are way too intrusive...

It's got a bit less loud in the meantime when adding the ! earlier — less bold and blue now:

note

I think it's more important to have the note about the surprising values rather than the note on the availability (the versionadded should be enough).

So there is a threshold where above a note gets its own box and below a box takes too much attention, makes sense.

OTOH, if you ever add more protections, we'll have a huge block of notes, almost always the same.

It would be multiple smaller blocks, not one huge block though. It's a bit less scary than that.

Btw I have no plans of adding more, new API like that will only appear if there is no way around it.

Here's what I can suggest: a simple .. seealso:: which refers to a section about "Availability testing" which indicates that some features are included in micro versions instead of .0 versions. That would reduce the vertical span of the page and be more readable for users in general. WDYT?

Personally I think that see also requires an additional click and additional energy and that alone will many users stop from ever noticing. (I also believe readability is not in danger currently.) Let me demo what we get when taking the .. note:: markers out and turning these notes back to text. Someone reading the whole method description will get the memo then, and it will not be as loud. Push coming up in a minute or two…

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork
Demo more quiet version without ".. note::"
5014423
@hartwork

hartwork commented Oct 9, 2025

Copy link
Copy Markdown
Contributor Author

@picnixz pushed.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork

hartwork commented Oct 13, 2025

Copy link
Copy Markdown
Contributor Author

@picnixz how do you like the new version?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Oct 13, 2025

Copy link
Copy Markdown
Member

I'm sorry but I don't have a lot of time to look at the Expat related issues/PRs. At first glance it looks fine (less visual distraction).

@hugovk As someone who has better insight on accessibility & co, what would you suggest here?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk
hugovk reviewed Oct 13, 2025
View reviewed changes
Comment thread Doc/library/pyexpat.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/pyexpat.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

hartwork added 3 commits October 14, 2025 00:54
@hartwork
Revert accidental addition of blank line
a4f79ad
@hartwork
Drop "Note that " at the start of the sentence
d5ecaa2
Requested by @hugovk
@hartwork
Resolve double self reference
6e397c4
As requested by @hugovk
@hartwork
hartwork requested a review from hugovk October 13, 2025 23:08
@hartwork

hartwork commented Oct 13, 2025

Copy link
Copy Markdown
Contributor Author

@hugovk thanks for the review! How about now?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk
hugovk approved these changes Oct 14, 2025
View reviewed changes

@hugovk hugovk left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hartwork reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Oct 14, 2025
@hugovk
hugovk merged commit 0c17473 into python:main Oct 14, 2025
4 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 14, 2025
@github-project-automation github-project-automation Bot moved this from Todo to Done in Docs PRs Oct 14, 2025
@hugovk

hugovk commented Oct 14, 2025

Copy link
Copy Markdown
Member

And backport this to 3.13?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Oct 14, 2025

Copy link
Copy Markdown
Member

Actually, we didn't backported the methods to prior versions yet. I am currently starting my new work so I didn'tt have much time to focus on those PRs (currently, only main contains these new APIs)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk

hugovk commented Oct 14, 2025

Copy link
Copy Markdown
Member

OK, so should this not have been merged yet? It's only in the 3.15 docs for now, so not such a big deal if they're coming soon-ish.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Oct 14, 2025

Copy link
Copy Markdown
Member

Yes, I should have added the DO-NOT-MERGE, my fault (we discussed it in the comments but it should have been mentioned more explicitly). It's not really important as I plan to backport the PRs next week (but my work may take me some time) (half of them are ready, the other half isn't backported yet)

hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk

hugovk commented Oct 14, 2025

Copy link
Copy Markdown
Member

OK, let's leave this in main for now and do the backports later. Good luck with your new work!

picnixz reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork

hartwork commented Oct 14, 2025

Copy link
Copy Markdown
Contributor Author

OK, let's leave this in main for now and do the backports later.

@hugovk good plan, there is no real damage done, no worries.

Good luck with your new work!

From me as well! 🥂

hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork hartwork mentioned this pull request Dec 17, 2025
gh-90949: expose Expat API to tune exponential expansion protections #139368
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk approved these changes
@StanFromIreland StanFromIreland Awaiting requested review from StanFromIreland
@picnixz picnixz Awaiting requested review from picnixz

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

Docs PRs
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@hartwork @picnixz @hugovk @StanFromIreland
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.