gh-136061: IDLE - update code in editor.Editor.load_extension #134874

johnzhou721 · 2025-05-29T03:23:31Z

A DOS by Quadratic complexity issue is fixed in idlelib. Part of (but does not fix) #134873.

Issue: A Series of Simple DOS Vulnerabilities #134873

Issue: IDLE: Performance issue processing config-extensions.def #136061

terryjreedy · 2025-05-29T10:36:22Z

I believe that the 6 lines from 1205 to 1210 can be replaced by 2 lines -- an re.match and an f-string. I will submit an alternate proposal later. I believe that the input vevent name should have either no <>s or 2 of each, with maybe the latter for back compatibility (I will test). But I will may stick with the more general code to not break buggy extensions.

ZeroIntensity

Assuming this is the fix that we go with, let's add a test case.

Misc/NEWS.d/next/Security/2025-05-29-03-24-18.gh-issue-134873.dziqkQ.rst

johnzhou721 · 2025-05-29T13:57:52Z

@terryjreedy so should I leave the code for now, or should I go ahead and replace with the re.match thing you are going to propose? @ZeroIntensity so do you mean that active voice is preferred in release notes? I can replace this specific case with the change that you are suggesting, but I'm asking for advice in this aspect for future News.

Lib/idlelib/editor.py

johnzhou721 · 2025-05-29T18:22:14Z

Yes, I think it can be. Will fix.

…

Message ID: ***@***.***>

kexinoh · 2025-05-30T02:24:34Z

@johnzhou721
I would greatly appreciate it if you could kindly address the issue located at

cpython/Lib/idlelib/editor.py

Lines 1373 to 1378 in 5ab66a8

    
           while True: 
        
               chars = chars[:-1] 
        
               ncharsdeleted = ncharsdeleted + 1 
        
               have = len(chars.expandtabs(tabwidth)) 
        
               if have <= want or chars[-1] not in " \t": 
        
                   break

. I sincerely apologize for overlooking this in my previous message.

As an example, I successfully utilized Gemini 2.5 Pro to generate a reasonable fix for this problem. Could you give it a try?

johnzhou721 · 2025-05-30T02:40:25Z

@kexinoh Yes, I would give it a try once I have time; however, I am working on something else right now -- is it acceptable if I delay this by about a day or so?

(if anyone else has a fix ready before I get to this, feel free to make a pr onto the branch of my pr and I'll merge it into my PR)

…dziqkQ.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

johnzhou721 · 2025-05-30T03:29:49Z

@kexinoh I have a small amount of time not enough to work on anything else before I end my day so I attempted the issue you pointed out -- but can't test though.

johnzhou721 · 2025-05-30T04:06:28Z

Assuming this is the fix that we go with, let's add a test case.

Where? How? For what? Thanks! @ZeroIntensity

ZeroIntensity · 2025-05-30T11:41:01Z

Where? How? For what?

We need a test case in test_idlelib that results in DOS/extreme slowness off main. Basically, just do something to prove that this PR fixes it (probably just testing with large amounts of data).

terryjreedy · 2025-06-28T06:06:44Z

Still need to delete Security blurb and add IDLE blurb and news items. However

f:\dev\3x>git push https://github.com/johnzhou721/cpython pr_134874:idledos

remote: Permission to johnzhou721/cpython.git denied to terryjreedy.
fatal: unable to access 'https://github.com/johnzhou721/cpython/': The requested URL returned error: 403

This is my standard pr revision push template. with pr#, johnzhou721 idledos taken from top of this page.

@zware Do you have any idea what is wrong? Is the fact that johnzhou forked from somewhere else than python/cpython relevant?
@johnzhou721 Did you uncheck [] allow others to edit pr? The fact that I edited online suggests not.

Edit: deleted blurb on View files page. Unfortunately, that triggers retest, which will fail because no blurb.

Blurb-it failed also due to lack of access to PR.

…dziqkQ.rst

johnzhou721 · 2025-06-28T13:17:10Z

@terryjreedy Thanks for looking into this issue and resolving my suspicions. I will be blurbing it.

terryjreedy

Please make a change and an addition. Change the blurb to say
Update code in editor.Editor.load_extension. Patch by johnzhou and Zachary Ware.

Then add the same + the prefex gh-134873: to 'idlelib/News3.txt'. The result should be that the top 10 lines of the file looks like the following, with 2 blank lines above and 1 below the addition. (These blank lines matter as they enable clean backports!)

What's New in IDLE 3.14.0
(since 3.13.0)
Released on 2025-10-07
=========================


gh-134873: Update code in editor.Editor.load_extension.
Patch by johnzhou and Zachary Ware.

gh-112936: IDLE - Include Shell menu in single-process mode,

bedevere-app · 2025-06-29T04:34:42Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

serhiy-storchaka · 2025-06-29T04:38:56Z

Is it needed? This change is mainly cosmetic, it has almost no effect on users.

terryjreedy

After further review, I realize that we can replace 6 lines with one line to convert string vevent to string methodname in one expression that serves as a 'template' of the changes needed. See comment.

@serhiy-storchaka I don't think this change is needed as a security feature. To me, it makes the code a bit easier to read and understand.

If you veto merging the change (and close the issue as "won't fix"), I would instead want to add and merge a comment as to the equivalent expression, for whenever I or someone were to do a more thorough review of the extension functions. (I already noticed that load failure prints "Failed to import extension" to console twice, and aborts IDLE startup. Since essential IDLE functions were converted from 'extensions' to normal features years ago, stopping is no longer appropriate.)

Lib/idlelib/editor.py

serhiy-storchaka · 2025-06-29T06:45:39Z

Sorry for my unclear comment. I see now that it can be be interpreted incorrectly.

I questioned not the change (which LGTM), but necessary of the NEWS entry for it.

Lib/idlelib/editor.py

johnzhou721 · 2025-06-29T13:08:19Z

I have made the requested changes; please review again

FYI: I credited @terryjreedy as well since they came up with the approach for combining it into all one line.

(@terryjreedy: not sure about your pronouns, sorry for using they)

bedevere-app · 2025-06-29T13:08:24Z

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

zware · 2025-06-30T16:43:18Z

@zware Do you have any idea what is wrong? Is the fact that johnzhou forked from somewhere else than python/cpython relevant?

My suspicion would be that the push to https:// is rejected; I seem to remember something about GitHub no longer accepting HTTPS pushes at all anymore, but I don't have a reference for that assertion. I'd try git push git@github.com:johnzhou721/cpython pr_134874:idledos instead.

Lib/idlelib/editor.py

Misc/NEWS.d/next/IDLE/2025-06-28-13-29-52.gh-issue-136061.EQYuVW.rst

Lib/idlelib/News3.txt

…VW.rst

Co-authored-by: Zachary Ware <zachary.ware@gmail.com>

fix a dos

e5f4260

johnzhou721 requested a review from terryjreedy as a code owner May 29, 2025 03:23

bedevere-app bot added the awaiting review label May 29, 2025

bedevere-app bot mentioned this pull request May 29, 2025

A Series of Simple DOS Vulnerabilities #134873

Closed

📜🤖 Added by blurb_it.

aa5b5a8

terryjreedy added this to IDLE Issues May 29, 2025

terryjreedy moved this to In Progress in IDLE Issues May 29, 2025

terryjreedy self-assigned this May 29, 2025

ZeroIntensity reviewed May 29, 2025

View reviewed changes

Misc/NEWS.d/next/Security/2025-05-29-03-24-18.gh-issue-134873.dziqkQ.rst Outdated Show resolved Hide resolved

zware reviewed May 29, 2025

View reviewed changes

Lib/idlelib/editor.py Show resolved Hide resolved

Update Misc/NEWS.d/next/Security/2025-05-29-03-24-18.gh-issue-134873.…

edfa904

…dziqkQ.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

johnzhou721 added 2 commits May 29, 2025 22:31

another dos and also simplify

2090baf

grumble

d2838a5

johnzhou721 requested review from zware and ZeroIntensity May 30, 2025 04:06

terryjreedy changed the title ~~gh-134873: Avoid quadratic complexities in idlelib~~ gh-134873: IDLE - update code in editor.Editor.load_extension Jun 28, 2025

Delete Misc/NEWS.d/next/Security/2025-05-29-03-24-18.gh-issue-134873.…

1037bea

…dziqkQ.rst

kexinoh mentioned this pull request Jun 28, 2025

IDLE: Performance issue processing config-extensions.def #136061

Open

serhiy-storchaka changed the title ~~gh-134873: IDLE - update code in editor.Editor.load_extension~~ gh-136061: IDLE - update code in editor.Editor.load_extension Jun 28, 2025

📜🤖 Added by blurb_it.

e505938

johnzhou721 requested a review from terryjreedy June 28, 2025 13:30

terryjreedy requested changes Jun 29, 2025

View reviewed changes

bedevere-app bot removed the awaiting review label Jun 29, 2025

bedevere-app bot added the awaiting changes label Jun 29, 2025

terryjreedy reviewed Jun 29, 2025

View reviewed changes

Lib/idlelib/editor.py Outdated Show resolved Hide resolved

johnzhou721 commented Jun 29, 2025

View reviewed changes

Lib/idlelib/editor.py Outdated Show resolved Hide resolved

Lib/idlelib/editor.py Outdated Show resolved Hide resolved

Lib/idlelib/editor.py Outdated Show resolved Hide resolved

johnzhou721 added 3 commits June 29, 2025 08:03

Apply suggestions from code review

283a27a

update news

d58bd18

Update News3.txt

560e06b

bedevere-app bot added awaiting change review and removed awaiting changes labels Jun 29, 2025

bedevere-app bot requested a review from terryjreedy June 29, 2025 13:08

zware reviewed Jun 30, 2025

View reviewed changes

Lib/idlelib/editor.py Outdated Show resolved Hide resolved

Misc/NEWS.d/next/IDLE/2025-06-28-13-29-52.gh-issue-136061.EQYuVW.rst Outdated Show resolved Hide resolved

Lib/idlelib/News3.txt Outdated Show resolved Hide resolved

johnzhou721 and others added 2 commits June 30, 2025 11:55

Delete Misc/NEWS.d/next/IDLE/2025-06-28-13-29-52.gh-issue-136061.EQYu…

a09c458

…VW.rst

Update Lib/idlelib/News3.txt

a082fbf

Co-authored-by: Zachary Ware <zachary.ware@gmail.com>

zware added the skip news label Jun 30, 2025

Update Lib/idlelib/editor.py

4c1268f

Co-authored-by: Zachary Ware <zachary.ware@gmail.com>

Uh oh!

gh-136061: IDLE - update code in editor.Editor.load_extension #134874

Are you sure you want to change the base?

gh-136061: IDLE - update code in editor.Editor.load_extension #134874

Conversation

johnzhou721 commented May 29, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

terryjreedy commented May 29, 2025

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnzhou721 commented May 29, 2025 via email

Uh oh!

Uh oh!

johnzhou721 commented May 29, 2025 via email

Uh oh!

kexinoh commented May 30, 2025

Uh oh!

johnzhou721 commented May 30, 2025

Uh oh!

johnzhou721 commented May 30, 2025

Uh oh!

johnzhou721 commented May 30, 2025

Uh oh!

ZeroIntensity commented May 30, 2025

Uh oh!

terryjreedy commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnzhou721 commented Jun 28, 2025

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Jun 29, 2025

Uh oh!

serhiy-storchaka commented Jun 29, 2025

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serhiy-storchaka commented Jun 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johnzhou721 commented Jun 29, 2025

Uh oh!

bedevere-app bot commented Jun 29, 2025

Uh oh!

zware commented Jun 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johnzhou721 commented May 29, 2025 •

edited by bedevere-app bot

Loading

terryjreedy commented Jun 28, 2025 •

edited

Loading