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

Can not import modules if sys.prefix contains DELIM #75393

Closed
cedk mannequin opened this issue Aug 15, 2017 · 17 comments
Closed

Can not import modules if sys.prefix contains DELIM #75393

cedk mannequin opened this issue Aug 15, 2017 · 17 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@cedk
Copy link
Mannequin

cedk mannequin commented Aug 15, 2017

BPO 31210
Nosy @brettcannon, @pfmoore, @ncoghlan, @vstinner, @tjguk, @cedk, @bitdancer, @ericsnowcurrently, @zware, @eryksun, @zooba

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-09-26.00:55:34.746>
created_at = <Date 2017-08-15.12:29:44.531>
labels = ['interpreter-core', 'invalid', 'type-bug', '3.7']
title = 'Can not import modules if sys.prefix contains DELIM'
updated_at = <Date 2019-09-26.00:55:34.745>
user = 'https://github.com/cedk'

bugs.python.org fields:

activity = <Date 2019-09-26.00:55:34.745>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-26.00:55:34.746>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2017-08-15.12:29:44.531>
creator = 'ced'
dependencies = []
files = []
hgrepos = []
issue_num = 31210
keywords = []
message_count = 17.0
messages = ['300293', '300294', '300295', '300296', '300302', '300304', '300307', '300308', '300329', '300360', '300361', '300370', '300374', '300375', '300388', '300403', '353247']
nosy_count = 11.0
nosy_names = ['brett.cannon', 'paul.moore', 'ncoghlan', 'vstinner', 'tim.golden', 'ced', 'r.david.murray', 'eric.snow', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31210'
versions = ['Python 3.7']

@cedk
Copy link
Mannequin Author

cedk mannequin commented Aug 15, 2017

If the path on which Python is installed contains the DELIM, the resulted sys.path is split.
I think maybe there should be a escape mechanism for the PYTHONPATH.

@cedk cedk mannequin added type-crash A hard crash of the interpreter, possibly with a core dump interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Aug 15, 2017
@bitdancer
Copy link
Member

By DELIM, you mean the shell ':'? As far as I've been able to determine there is no way defined in posix to escape that character. If you can find one, please let us know. (I think the same is true for the Windows semicolon but I'm not sure.)

@cedk
Copy link
Mannequin Author

cedk mannequin commented Aug 15, 2017

Yes I mean ':' for posix.
Indeed I do not known a posix way to escape the colon (it was discussed here: https://stackoverflow.com/questions/14661373/how-to-escape-colon-in-path-on-unix).
But it does not mean that the method makepathobject could not support one. I'm thinking about path single-quoted.

@bitdancer
Copy link
Member

I'm not sure there is anything we should do here, then, because we are conforming to the posix parsing for PATH in our PYTHONPATH implementation.

I think if you want to pursue this further you should take it to the python-ideas mailing list. I'm going to close the issue, but you can reopen it if python-ideas thinks it is worth doing something (and there is something we can reasonably do :)

@bitdancer bitdancer added invalid type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Aug 15, 2017
@cedk
Copy link
Mannequin Author

cedk mannequin commented Aug 15, 2017

A last comment, I do not think it is an issue to follow posix way to parse PATH. But for me, the problem is that Python adds without sanitation the sys.prefix to the PYTHONPATH. So I think internally Python should not use PATH notation to extend the PYTHONPATH because it is not a robust notation.

@bitdancer
Copy link
Member

You mean to create the entries on sys.path that do not come from the PYTHONPATH?

@cedk
Copy link
Mannequin Author

cedk mannequin commented Aug 15, 2017

On 2017-08-15 17:32, R. David Murray wrote:

You mean to create the entries on sys.path that do not come from the PYTHONPATH?

Yes because such path could contain ':'.

@bitdancer
Copy link
Member

OK, that seems reasonable to me. I'll reopen the issue. Assuming other developers agree that this should be changed, I'm not sure if it will qualify as a bug or an enhancement, so I'm leaving versions unselected for now :)

@bitdancer bitdancer reopened this Aug 15, 2017
@bitdancer bitdancer removed the invalid label Aug 15, 2017
@ncoghlan
Copy link
Contributor

I'm marking this as Python 3.7 only, not because I don't think it's a bug, but because getpath.c is a maintainability nightmare and I strongly prefer we avoid going anywhere near it in maintenance releases :)

Targeting Python 3.7+ also means we may be able to take advantage of the initial phase of PEP-432 changes and look at switching getpath.c to use higher level CPython data structures (specifically list objects) internally.

I've also added the Windows folks to the nosy list to check if Windows might have a similar problem with semi-colons appearing in sys.prefix.

@ncoghlan ncoghlan added the 3.7 (EOL) end of life label Aug 16, 2017
@zooba
Copy link
Member

zooba commented Aug 16, 2017

We don't. Semicolons are not valid path characters, so you can't have them in a single path (unless you're deliberately trying to break your entire machine...)

@bitdancer
Copy link
Member

Is this post wrong then?:

https://superuser.com/questions/584870/how-can-i-add-a-folder-containing-a-semicolon-to-the-windows-path

("I noticed that the semicolon ; is a valid character for Windows (NTFS) file and directory names.")

@eryksun
Copy link
Contributor

eryksun commented Aug 16, 2017

Is this post wrong then?

No, it's not wrong that semicolon is a valid character in Windows NTFS and FAT32 filesystems.

However, the answer by Kevin Edwards that recommends using double quotes needs qualification. It only works in CMD. It doesn't work in PowerShell nor the runtime library functions used by CreateProcess or SearchPath (i.e. RtlDosSearchPath_U[str]) or the loader's LdrpSearchPath function. These all split PATH on semicolon, with no supported escape character or quoting. In fact they retain double quotes in the computed filename when testing for existence, so quoting paths in PATH is wrong in general.

@bitdancer
Copy link
Member

All right. So the challenge here for windows is: if python is installed on a path that has a semicolon in one of the directory names, is it even possible to populate sys.path such that modules can be imported from the lib directory that is under that path? (IMO we shouldn't jump through big hoops to support this, but if it can be done and the refactoring Nick is contemplating makes it *relatively* easily, it would probably be good to do, to be consistent).

@bitdancer bitdancer changed the title Can not import site from sys.prefix containing DELIM Can not import modules if sys.prefix contains DELIM Aug 16, 2017
@eryksun
Copy link
Contributor

eryksun commented Aug 16, 2017

It's simple to work around using a junction or symbolic link, so is this worth pursuing?

@cedk
Copy link
Mannequin Author

cedk mannequin commented Aug 16, 2017

I'm wondering if it could have security implications and be used to fool user by changing the PYTHONPATH.

@ncoghlan
Copy link
Contributor

If you have access to modify PYTHONPATH at all, you can already shadow almost all standard library modules:

$ PYTHONPATH=/MY_CHOSEN_DIRECTORY python3 -m site
sys.path = [
    '/home/ncoghlan',
    '/MY_CHOSEN_DIRECTORY',
    '/usr/lib64/python36.zip',
    '/usr/lib64/python3.6',
    '/usr/lib64/python3.6/lib-dynload',
    '/home/ncoghlan/.local/lib/python3.6/site-packages',
    '/usr/lib64/python3.6/site-packages',
    '/usr/lib/python3.6/site-packages',
]

The only ones you can't shadow that way are builtin and frozen modules, and any modules that get imported even before PYTHONPATH is processed. So no, this doesn't open up any new attack vectors that weren't already present by design.

As far as whether or not it's worth fixing goes, yes, I think so - one of my original motivations for writing PEP-432 was to allow the use of CPython data structures when calculating the initial value of sys.path, and this is a nice concrete example of a bug arising from the current implementation.

@vstinner
Copy link
Member

I consider that the current behavior is correct. They are other ways to put a path which contains ":" into sys.path. I close the issue.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants