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

platform.py: _syscmd_file() can't handle target path with space or special shell character #47969

Closed
jfdp mannequin opened this issue Aug 29, 2008 · 11 comments
Closed

platform.py: _syscmd_file() can't handle target path with space or special shell character #47969

jfdp mannequin opened this issue Aug 29, 2008 · 11 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@jfdp
Copy link
Mannequin

jfdp mannequin commented Aug 29, 2008

BPO 3719
Nosy @malemburg
Files
  • platform.patch
  • 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 = 'https://github.com/malemburg'
    closed_at = <Date 2008-09-02.11:09:29.490>
    created_at = <Date 2008-08-29.00:40:12.829>
    labels = ['type-bug', 'library']
    title = "platform.py: _syscmd_file() can't handle target path with\tspace or special shell character"
    updated_at = <Date 2008-09-02.15:50:21.791>
    user = 'https://bugs.python.org/jfdp'

    bugs.python.org fields:

    activity = <Date 2008-09-02.15:50:21.791>
    actor = 'jfdp'
    assignee = 'lemburg'
    closed = True
    closed_date = <Date 2008-09-02.11:09:29.490>
    closer = 'lemburg'
    components = ['Library (Lib)']
    creation = <Date 2008-08-29.00:40:12.829>
    creator = 'jfdp'
    dependencies = []
    files = ['11342']
    hgrepos = []
    issue_num = 3719
    keywords = ['patch']
    message_count = 11.0
    messages = ['72118', '72225', '72321', '72322', '72324', '72325', '72327', '72328', '72329', '72330', '72347']
    nosy_count = 3.0
    nosy_names = ['lemburg', 'ocean-city', 'jfdp']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue3719'
    versions = ['Python 2.6', 'Python 2.5', 'Python 3.0']

    @jfdp
    Copy link
    Mannequin Author

    jfdp mannequin commented Aug 29, 2008

    If you install python in a location which has a space
    or shell character in the path then platform.platform()
    will generate an error and/or fail to get all platform
    information.

    For example

    $ pwd
    /disk0/tmp/foobar(2)/python2.4/bin
    $ ./python
    Python 2.4.4 (#8, Apr 11 2008, 11:42:39) [C] on sunos5
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import platform
    >>> print platform.platform()
    sh: syntax error at line 1: `(' unexpected
    SunOS-5.10-sun4u-sparc-32bit

    Note the error from 'sh' was well as the fact that "ELF"
    is missing from the platform string. If you are in a path
    with a space then it silently fails to identify "ELF".

    The problem is in platform.py: _syscmd_file(target,default='')
    in particular this line:

    f = os.popen('file %s 2> /dev/null' % target)

    This should be:

    f = os.popen('file "%s" 2> /dev/null' % target)

    Note the double quotes to protect the path from the shell.

    I've examined the 2.5, 2.6 and 3.0 source and they all
    have this same problem.

    @jfdp jfdp mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Aug 29, 2008
    @malemburg
    Copy link
    Member

    Is adding the double-quotes enough to solve the problem ?

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Sep 2, 2008

    Can you try this patch?

    1. used "file -b" option to eliminate file path. Otherwide, re.split
      won't work because file path will contain space like this. (I hope -b
      option can be used anywhere "file" command exists)

    a b/python.exe: MS-DOS executable PE for MS Windows (console) Intel
    80386 32-bit

    1. used subprocess module instead of popen. subprocess is useful when
      argument contains space or quote like this issue.

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Sep 2, 2008

    [off topic]
    I used cygwin to create this patch, but platform.architecture()[1]
    becomes empty string because sys.executable in cygwin points to
    directory. (at least before installing)

    Python 2.6b3+ (trunk:66142M, Sep  2 2008, 17:09:46)
    [GCC 3.4.4 (cygming special, gdc 0.12, using dmd 0.125)] on cygwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import sys
    >>> sys.executable
    '/home/WhiteRabbit/python-dev/trunk/python'

    I workarounded this by inserting "target = './python.exe'" in
    _syscmd_file, but sorry if this patch won't work.

    @malemburg
    Copy link
    Member

    Using subprocess is only possible conditionally, ie. if available.
    Please note that the module is intended to be usable with multiple
    Python versions and must at least support Python 2.1.

    We could consider an updated patch for 2.7 and 3.1, but not for 2.6/3.0,
    since it needs more testing.

    I think for 2.6/3.0, adding the extra quotes is enough to get things
    working.

    Please open a new patch ticket for your patch and update it to work with
    popen if subprocess is not available. The patch should also not remove
    the lines:

    • if sys.platform in ('dos','win32','win16','os2'):
    •    # XXX Others too ?
      
    •    return default
      

    from _syscmd_file().

    I'll checkin the extra quotes.

    @malemburg
    Copy link
    Member

    Updated the Python versions.

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Sep 2, 2008

    Well, if python was installed into directory named
    "foo ELF boo", python will say wrong architecture.

    >>> import platform
    >>> platform.architecture()
    ('32bit', 'ELF')

    Is it not good to use "file -b" option?

    @malemburg
    Copy link
    Member

    On 2008-09-02 12:55, Hirokazu Yamamoto wrote:
    > Hirokazu Yamamoto <ocean-city@m2.ccsnet.ne.jp> added the comment:
    > 
    > Well, if python was installed into directory named
    > "foo ELF boo", python will say wrong architecture.
    > 
    >>>> import platform
    >>>> platform.architecture()
    > ('32bit', 'ELF')
    > 
    > Is it not good to use "file -b" option?

    Well, I'd say that install path is rather unlikely to occur in real
    life ;-)

    I'm not sure how wide-spread support for the -b option is, so we'd
    have to check during the next release cycle.

    @malemburg malemburg changed the title platform.py: _syscmd_file() can't handle target path with space or special shell character platform.py: _syscmd_file() can't handle target path with space or special shell character Sep 2, 2008
    @malemburg
    Copy link
    Member

    Checked in as r66145 and r66146.

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Sep 2, 2008

    OK, I understand. And I must apologizes about previous
    my patch.

      fileout = _architecture_split(output)[1:]

    should not be deleted. It was should be changed to

      fileout = _architecture_split(output)

    Thank you.

    @jfdp
    Copy link
    Mannequin Author

    jfdp mannequin commented Sep 2, 2008

    To respond to a couple questions:

    Adding the double quotes fixed the issue I had -- but I did very
    limited testing.

    The "-b" option is not support by 'file' on Solaris.

    @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
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant