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

Current os.tmpfile() implementation requires admin privs on Vista/2k8. #46485

Closed
tpn opened this issue Mar 4, 2008 · 9 comments
Closed

Current os.tmpfile() implementation requires admin privs on Vista/2k8. #46485

tpn opened this issue Mar 4, 2008 · 9 comments
Labels
OS-windows type-bug An unexpected behavior, bug, or error

Comments

@tpn
Copy link
Member

tpn commented Mar 4, 2008

BPO 2232
Nosy @loewis, @tiran, @benjaminp, @tpn
Files
  • test_os.py.patch: Patch to trunk/Lib/test/test_os.py
  • test_os.py.2.patch: Updated patch to trunk/Lib/test/test_os.py
  • 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 2008-03-06.06:57:46.163>
    created_at = <Date 2008-03-04.17:01:17.820>
    labels = ['type-bug', 'OS-windows']
    title = 'Current os.tmpfile() implementation requires admin privs on Vista/2k8.'
    updated_at = <Date 2008-03-06.06:57:46.162>
    user = 'https://github.com/tpn'

    bugs.python.org fields:

    activity = <Date 2008-03-06.06:57:46.162>
    actor = 'loewis'
    assignee = 'none'
    closed = True
    closed_date = <Date 2008-03-06.06:57:46.163>
    closer = 'loewis'
    components = ['Windows']
    creation = <Date 2008-03-04.17:01:17.820>
    creator = 'trent'
    dependencies = []
    files = ['9616', '9617']
    hgrepos = []
    issue_num = 2232
    keywords = ['patch']
    message_count = 9.0
    messages = ['63254', '63260', '63262', '63279', '63294', '63299', '63300', '63301', '63305']
    nosy_count = 5.0
    nosy_names = ['loewis', 'christian.heimes', 'JosephArmbruster', 'benjamin.peterson', 'trent']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue2232'
    versions = ['Python 2.6']

    @tpn
    Copy link
    Member Author

    tpn commented Mar 4, 2008

    posix_tmpfile() needs to be reworked such that tmpfile() isn't used if
    MS_WINDOWS is defined, as it requires administrative privileges to run
    (it creates the file in the root directory) on Vista/2k8 (although
    there are reports of this not working on XP w/ 2.5:
    http://www.thescripts.com/forum/thread600423.html).

    The recommended approach in MSDN is to use, quote, "tmpname or tempnam
    in conjunction with fopen": http://msdn2.microsoft.com/en-
    us/library/x8x7sakw(VS.80).aspx

    Assuming no one beats me to it, I'll look at creating a patch in the
    next day or two when I get some spare time.

    @tpn tpn added OS-windows type-bug An unexpected behavior, bug, or error labels Mar 4, 2008
    @tiran
    Copy link
    Member

    tiran commented Mar 4, 2008

    Side note:
    I've removed the methods from Python 3.0 about half a year ago. Code
    should use the tempfile module anyway. Does any of the Python 2.6 stdlib
    code use an os.tmp* method?

    @benjaminp
    Copy link
    Contributor

    According to grep, the only place where os.tmp* is referenced is in test_os.

    @tpn
    Copy link
    Member Author

    tpn commented Mar 5, 2008

    With Chris and Ben's comments taken into account, what's the best way to
    handle this?

    1. Change the test: if the user is not admin, assert os.tmpfile()
      returns a permission denied OSError, otherwise, assert return value is
      a current file.
    2. Alter posix_tmpfile() to warn the user of the situation if the call
      fails and we're on Windows:
      Index: posixmodule.c
      ===================================================================
      --- posixmodule.c (revision 61233)
      +++ posixmodule.c (working copy)
      @@ -7029,8 +7029,15 @@
      FILE *fp;
         fp = tmpfile();
    -    if (fp == NULL)
    +    if (fp == NULL) {
    +#ifdef MS_WINDOWS
    +        PyErr_Warn(PyExc_RuntimeWarning,
    +                   "tmpfile creates a file in the root directory and "   \
    +                   "requires administrative privileges, consider using " \
    +                   "tempfile.mkstemp() instead");
    +#endif
             return posix_error();
    +    }
         return PyFile_FromFile(fp, "<tmpfile>", "w+b", fclose);
     }
     #endif
    1. Alter posix_tmpfile() to use _tempnam() on Windows instead, such
      admin privileges are no longer required. (Possibly adding a similar
      warning that tempfile.mkstemp should be used instead though, as above?)

    @benjaminp
    Copy link
    Contributor

    With Chris and Ben's comments taken into account, what's the best way to
    handle this?
    I think, given that is being removed, we can safely go with option 1.

    @tpn
    Copy link
    Member Author

    tpn commented Mar 5, 2008

    I agree. Following patch fixes the issue for me:

    Index: test_os.py
    ===================================================================

    --- test_os.py  (revision 61260)
    +++ test_os.py  (working copy)
    @@ -65,6 +65,44 @@
         def test_tmpfile(self):
             if not hasattr(os, "tmpfile"):
                 return
    +        # As with test_tmpnam() below, the Windows implementation of 
    tmpfile()
    +        # attempts to create a file in the root directory of the 
    current drive.
    +        # On Vista and Server 2008, this test will always fail for 
    normal users
    +        # as writing to the root directory requires elevated 
    privileges.  With
    +        # XP and below, the semantics of tmpfile() are the same, but 
    the user
    +        # running the test is more likely to have administrative 
    privileges on
    +        # their account already.  If that's the case, then os.tmpfile
    () should
    +        # work.  In order to make this test as useful as possible, 
    rather than
    +        # trying to detect Windows versions or whether or not the user 
    has the
    +        # right permissions, just try and create a file in the root 
    directory
    +        # and see if it raises a 'Permission denied' OSError.  If it 
    does, then
    +        # test that a subsequent call to os.tmpfile() raises the same 
    error. If
    +        # it doesn't, assume we're on XP or below and the user running 
    the test
    +        # has administrative privileges, and proceed with the test as 
    normal.
    +        if sys.platform == 'win32':
    +            name = '\\python_test_os_test_tmpfile.txt'
    +            if os.path.exists(name):
    +                os.remove(name)
    +            try:
    +                fp = open(name, 'w')
    +            except IOError, first:
    +                # open() failed, assert tmpfile() fails in the same 
    way.
    +                # Although open() raises an IOError and os.tmpfile() 
    raises an
    +                # OSError(), 'args' will be (12, 'Permission denied') 
    in both
    +                # cases.
    +                try:
    +                    fp = os.tmpfile()
    +                except OSError, second:
    +                    self.assertEqual(first.args, second.args)
    +                else:
    +                    self.fail("expected os.tmpfile() to raise OSError")
    +                return
    +            else:
    +                # open() worked, therefore, tmpfile() should work.  
    Close our
    +                # dummy file and proceed with the test as normal.
    +                fp.close()
    +                os.remove(name)
    +
             fp = os.tmpfile()
             fp.write("foobar")
             fp.seek(0,0)

    @tpn
    Copy link
    Member Author

    tpn commented Mar 5, 2008

    Er, errno being referred to in a comment in that last patch should be
    13, not 12.

    @JosephArmbruster
    Copy link
    Mannequin

    JosephArmbruster mannequin commented Mar 5, 2008

    Tested patch against: http://svn.python.org/projects/python/trunk @ 61260

    OS Name: Microsoft Windows XP Professional
    OS Version: 5.1.2600 Service Pack 2 Build 260

    rt test_os
    Deleting .pyc/.pyo files ...
    (57, '.pyc deleted,', 0, '.pyo deleted')

    python -E -tt ../lib/test/regrtest.py test_os
    test_os
    1 test OK.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 6, 2008

    Thanks for the patch. Committed as r61264 and r61266.

    @loewis loewis mannequin closed this as completed Mar 6, 2008
    @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
    OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants