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

man page says -I implies -S. code says -s. #68265

Closed
jbeck mannequin opened this issue Apr 29, 2015 · 6 comments
Closed

man page says -I implies -S. code says -s. #68265

jbeck mannequin opened this issue Apr 29, 2015 · 6 comments
Labels
docs Documentation in the Doc dir

Comments

@jbeck
Copy link
Mannequin

jbeck mannequin commented Apr 29, 2015

BPO 24077
Nosy @warsaw, @tiran, @ned-deily, @dhduvall
Files
  • 25-site-module.patch: patch to get -I to imply -S and fixe side-effects
  • 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 2015-04-29.21:55:24.192>
    created_at = <Date 2015-04-29.21:19:52.356>
    labels = ['docs']
    title = 'man page says -I implies -S. code says -s.'
    updated_at = <Date 2015-04-29.21:59:26.345>
    user = 'https://bugs.python.org/jbeck'

    bugs.python.org fields:

    activity = <Date 2015-04-29.21:59:26.345>
    actor = 'barry'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2015-04-29.21:55:24.192>
    closer = 'ned.deily'
    components = ['Documentation']
    creation = <Date 2015-04-29.21:19:52.356>
    creator = 'jbeck'
    dependencies = []
    files = ['39233']
    hgrepos = []
    issue_num = 24077
    keywords = ['patch']
    message_count = 6.0
    messages = ['242248', '242249', '242250', '242251', '242252', '242253']
    nosy_count = 7.0
    nosy_names = ['barry', 'christian.heimes', 'ned.deily', 'dhduvall', 'docs@python', 'python-dev', 'jbeck']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue24077'
    versions = ['Python 3.4', 'Python 3.5']

    @jbeck
    Copy link
    Mannequin Author

    jbeck mannequin commented Apr 29, 2015

    The man page for Python (3.4 and 3.5) says:

    -I Run Python in isolated mode. This also implies -E and -S. In
    isolated mode sys.path contains neither the scripts directory
    nor the users site-packages directory. All PYTHON* environment
    variables are ignored, too. Further restrictions may be imposed
    to prevent the user from injecting malicious code.

    But the code says:

    -I : isolate Python from the user's environment (implies -E and -s)

    and the code to handle -I does:

        case 'I':
            Py_IsolatedFlag++;
            Py_NoUserSiteDirectory++;
            Py_IgnoreEnvironmentFlag++;
            break;
    

    where Py_NoUserSiteDirectory is the variable corresponding to the -s flag
    rather than the -S flag. But it seems like -I should really imply both
    -s and -S. So I am filing this bug primarily to find out whether or not
    it really should be both. If so, great: a patch is attached; details
    below. But if not, then the man page should be corrected.

    The rest of this is written under the assumption that -I should imply -S
    as well as -s.

    Background: depending on which packages are installed on different Solaris
    systems, test_site passes or not. Certain packages (e.g., dogpile.core,
    dogpile.cache, repoze.lru) that have a .pth file with "import types"
    which results in test_site.StartupImportTests failing because types has
    been imported which is in the list of collections modules, none of which
    are expected to be imported. So we thought "well, -S should fix that"
    then noticed the man page saying -I implied -S which is how we got here.

    Tweaking the code and man page so -I does imply -S was trivial. But three
    other changes were needed:

    1. In test_site.py, test_startup_imports() asserted that 'site' was in the
      list of modules that had been imported. This is no longer true, so I
      deleted the assert.

    2. test_inspect failed because of a name error, that turned out to be
      inspect.py calling exit instead of sys.exit. So the attached patch
      corrects both of those. This fix is probably generally applicable
      even if the "-I should imply both -S and -s" assumption turns out to
      be false.

    3. test_venv failed because it and the venv module were using -I to imply
      -s and -E but not -S. Changing three instances of "-Im" to "-Esm"
      (one in Lib/venv/init.py, the other two in Lib/test/test_venv.py)
      fixed this. However, even if the "-I should imply both -S and -s"
      assumption is true, this change may not be desirable in the general
      case, but needed because of Solaris' hacky work-around for issue
      1298835 not yet being fixed.'

      I.e., we ship /usr/lib/python3.4/site-packages/vendor-packages.pth
      with the one line:
      import site; site.addsitedir('/usr/lib/python3.4/vendor-packages')
      (likewise for other versions).

      So this may not be desirable in general, but I mention it for the
      sake of completeness.

    @jbeck jbeck mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Apr 29, 2015
    @jbeck
    Copy link
    Mannequin Author

    jbeck mannequin commented Apr 29, 2015

    Adding Christian Heimes to the nosy list; as the author of the fix for
    bpo-16499, he seems an excellent person to answer the question and
    offer advice on the approaches discussed herein.

    @tiran
    Copy link
    Member

    tiran commented Apr 29, 2015

    The isolated mode implies -E (ignore env vars) and -s (don't add user site directory). The code and tests are correct, just the man page is wrong. The site module is still loaded in -I mode as it doesn't impose any security implications.

    I'd looks like I made a typo in dd0d751cc7f1 and used upper case instead of lower case for python.man.

    @ned-deily ned-deily added docs Documentation in the Doc dir and removed interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Apr 29, 2015
    @jbeck
    Copy link
    Mannequin Author

    jbeck mannequin commented Apr 29, 2015

    Thank you very much for clarifying that. I have updated the bug Title
    accordingly.

    @jbeck jbeck mannequin changed the title man page says -I implies -S. code says -s. Should it be both? man page says -I implies -S. code says -s. Apr 29, 2015
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 29, 2015

    New changeset d774401879d8 by Ned Deily in branch '3.4':
    Issue bpo-24077: Fix typo in man page for -I command option: -s, not -S.
    https://hg.python.org/cpython/rev/d774401879d8

    New changeset 493b3310d5d0 by Ned Deily in branch 'default':
    Issue bpo-24077: merge from 3.4
    https://hg.python.org/cpython/rev/493b3310d5d0

    @ned-deily
    Copy link
    Member

    Thanks for the report, John!

    @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
    docs Documentation in the Doc dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants