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 — Fix TypeError when using subprocess shell=True under strict Java Secu… by akspi · Pull Request #419 · jython/jython

Conversation

@akspi

@akspi akspi commented Mar 31, 2026

Copy link
Copy Markdown

This change addresses the issue captured here: #418

Fixes #418

Sorry, something went wrong.

Uh oh!

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

All reactions
Akshay Pai
Fix TypeError when using subprocess shell=True under strict Java Secu…
6dedd1f
…rityManager
Stewori
Stewori reviewed Apr 1, 2026
View reviewed changes
Comment thread Lib/test/test_subprocess.py
if not jython:
return

import java.lang.SecurityManager as SecurityManager

@Stewori Stewori Apr 1, 2026

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.

Just a note: SecurityManager is deprecated for removal. Perhaps, it's premature to address this here, before it's actually removed. Once it's removed and the definitive JDK version of the removal is known, we can add a conditional skip.

Sorry, something went wrong.

Uh oh!

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

All reactions
jeff5
jeff5 reviewed Apr 3, 2026
View reviewed changes
Comment thread Lib/test/test_subprocess.py

class SecurityManagerTestCase(unittest.TestCase):
def test_subprocess_shell_with_security_manager(self):
if not jython:

@jeff5 jeff5 Apr 3, 2026 •
edited
Loading

Uh oh!

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

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.

A nicer way to do this is the decorator @unittest.skipIf(not jython, .... There's a @unittest.skipIf(jython somewhere in the file. It could be applied to the class.

Really appreciate you adding a test.

Sorry, something went wrong.

Uh oh!

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

All reactions
jeff5
jeff5 requested changes Apr 3, 2026
View reviewed changes

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

This seems like a valid bug and a sensible solution, thanks. I've only found points to criticise in the test. But thanks for adding one.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/test/test_subprocess.py

class MySecurityManager(SecurityManager):
def checkRead(self, file, context=None):
if file == "/bin/sh" or file == "cmd.exe" or file.endswith("sh"):

@jeff5 jeff5 Apr 3, 2026

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.

This needs a little more thought. Ok, it's only for the test and we're not proposing a real security manager. But it fails on my Windows machine because file contains "C:\Windows\System32\cmd.exe". And what is the intent of endswith("sh") anyway? So, probably endswith("cmd.exe").

It passes on the Windows CI because we do not enable the subprocess resource. (Seems like an omission right now.)

Fixed like that, and with checkRead printing what file it has be asked about, I find that subprocess._setup_platform() involves looking for an awful lot of files.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/test/test_subprocess.py
"-c", 1])


class SecurityManagerTestCase(unittest.TestCase):

@jeff5 jeff5 Apr 3, 2026

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.

Or on second thoughts, is this maybe happier in test_subprocess_jy.py, where everything is Jython-specific?

Sorry, something went wrong.

Uh oh!

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

All reactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Stewori Stewori Stewori left review comments
@jeff5 jeff5 jeff5 requested changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TypeError: unsupported operand type(s) for +: 'NoneType' and 'list' in subprocess.Popen when using shell=True under strict Java SecurityManager

Uh oh!

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

3 participants

@akspi @Stewori @jeff5
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.