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

os.path.normcase(None) does not raise an error on linux and should #53264

Closed
bitdancer opened this issue Jun 17, 2010 · 13 comments
Closed

os.path.normcase(None) does not raise an error on linux and should #53264

bitdancer opened this issue Jun 17, 2010 · 13 comments
Assignees
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@bitdancer
Copy link
Member

BPO 9018
Nosy @orsenthil, @pitrou, @giampaolo, @ezio-melotti, @bitdancer
Files
  • issue9018.diff: Simple testcase that fails on all the test_*path
  • issue9018-2.diff: Patch against py3k + unittest + doc.
  • 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/ezio-melotti'
    closed_at = <Date 2010-06-25.10:59:52.691>
    created_at = <Date 2010-06-17.13:40:15.252>
    labels = ['easy', 'type-bug', 'library']
    title = 'os.path.normcase(None) does not raise an error on linux and should'
    updated_at = <Date 2010-06-25.10:59:52.690>
    user = 'https://github.com/bitdancer'

    bugs.python.org fields:

    activity = <Date 2010-06-25.10:59:52.690>
    actor = 'ezio.melotti'
    assignee = 'ezio.melotti'
    closed = True
    closed_date = <Date 2010-06-25.10:59:52.691>
    closer = 'ezio.melotti'
    components = ['Library (Lib)']
    creation = <Date 2010-06-17.13:40:15.252>
    creator = 'r.david.murray'
    dependencies = []
    files = ['17700', '17735']
    hgrepos = []
    issue_num = 9018
    keywords = ['patch', 'easy']
    message_count = 13.0
    messages = ['108016', '108049', '108050', '108208', '108297', '108319', '108320', '108329', '108335', '108416', '108424', '108428', '108584']
    nosy_count = 6.0
    nosy_names = ['orsenthil', 'pitrou', 'giampaolo.rodola', 'ezio.melotti', 'r.david.murray', 'l0nwlf']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue9018'
    versions = ['Python 3.2']

    @bitdancer
    Copy link
    Member Author

    os.path.normcase(None) raises an error on Windows but returns None on linux. os.path.abspath(None) raises an error in both cases. os.path.normcase(None) should raise an error on linux.

    I've only marked this for 3.2 because I suspect there may be linux code out there that this will break, so the fix should probably only be applied to 3.2. (I discovered this because a unit test someone else wrote passed on linux but failed on windows.)

    @bitdancer bitdancer added easy type-bug An unexpected behavior, bug, or error labels Jun 17, 2010
    @ezio-melotti
    Copy link
    Member

    Right now posixpath returns the argument unchanged, ntpath performs a .replace(), and macpath a .lower(), so when non-string (or non-bytes) are passed to normcase the results are:
    posixpath: arg returned as-is;
    ntpath: AttributeError (object has no attribute 'replace');
    macpath: AttributeError (object has no attribute 'lower');

    In posixpath we could reject all the non-string (and non-bytes) args, raising a TypeError. For consistency, the other functions should raise a TypeError too, but I'm not sure it's worth changing it.

    Attached a simple testcase that checks that normcase raises a TypeError for invalid values with all the three implementations.

    @pitrou
    Copy link
    Member

    pitrou commented Jun 17, 2010

    I've only marked this for 3.2 because I suspect there may be linux code
    out there that this will break,

    It should be noticed that Linux-only code has absolutely no point in using normcase(), since it's a no-op there.

    @l0nwlf
    Copy link
    Mannequin

    l0nwlf mannequin commented Jun 19, 2010

    Following the documentation,
    os.path.normcase(path)
    Normalize the case of a pathname. On Unix and Mac OS X, this returns the path unchanged; on case-insensitive filesystems, it converts the path to lowercase. On Windows, it also converts forward slashes to backward slashes.

    on Mac OS X,
    >>> import os
    >>> os.name
    'posix'

    Checking through, Lib/posixpath.py,

    # Normalize the case of a pathname. Trivial in Posix, string.lower on Mac.
    # On MS-DOS this may also turn slashes into backslashes; however, other
    # normalizations (such as optimizing '../' away) are not allowed
    # (another function should be defined to do that).

    def normcase(s):
        """Normalize case of pathname.  Has no effect under Posix"""
        # TODO: on Mac OS X, this should really return s.lower().
        return s

    Why on Mac OS X, it should return s.lower() ?
    Also to raise Error for None case we can probably change normcase() in Lib/posixpath.py as,

    def normcase(s):
        """Normalize case of pathname.  Has no effect under Posix"""
        if s is None:  
            raise AttributeError
        return s

    @orsenthil
    Copy link
    Member

    On Sat, Jun 19, 2010 at 08:00:55PM +0000, Shashwat Anand wrote:

    Why on Mac OS X, it should return s.lower() ?

    That is is because some file systems on Mac OS X are case-sensitive.

    def normcase(s):
    """Normalize case of pathname. Has no effect under Posix"""
    if s is None:
    raise AttributeError
    return s

    It should be a TypeError, not AttributeError. (Attached Test cases
    checks it properly)

    @ezio-melotti
    Copy link
    Member

    ntpath and macpath raise an AttributeError, so we could:

    1. change them all to accept only bytes/str and raise a TypeError for other wrong types (correct, consistent, non-backward-compatible);
    2. change only posixpath to raise a TypeError for wrong types (partially correct, inconsistent, backward-compatible);
    3. change only posixpath to raise an AttributeError for wrong types (wrong, consistent, backward-compatible);

    The option 2 is still an improvement over the current situation, but it would be better to find a backward-compatible way to also obtain option 1 (assuming that backward compatibility is a concern here -- and I think it is (even though people could just change the code to catch (AttributeError, TypeError) and eventually get rid of the AttributeError)).

    @pitrou
    Copy link
    Member

    pitrou commented Jun 21, 2010

    ntpath and macpath raise an AttributeError, so we could:

    1. change them all to accept only bytes/str and raise a TypeError for
      other wrong types (correct, consistent, non-backward-compatible);

    Sounds like the best thing to do.

    The option 2 is still an improvement over the current situation, but
    it would be better to find a backward-compatible way to also obtain
    option 1 (assuming that backward compatibility is a concern here --
    and I think it is (even though people could just change the code to
    catch (AttributeError, TypeError) and eventually get rid of the
    AttributeError)).

    This isn't an exception you catch at runtime. It's an exception you get
    when your code is wrong, and then you fix your code. Therefore I don't
    think backwards compatibility is important.

    @ezio-melotti
    Copy link
    Member

    Here is the patch.

    @ezio-melotti ezio-melotti added the stdlib Python modules in the Lib dir label Jun 21, 2010
    @ezio-melotti ezio-melotti self-assigned this Jun 21, 2010
    @orsenthil
    Copy link
    Member

    Nice patch. I like the use of new string formating. It can be applied.

    BTW, do you think that TODO of s.lower() for MAC OS X should be addressed along with this. It might require some research as how much of a desirable behavior it would be.

    @bitdancer
    Copy link
    Member Author

    My understanding is that there is a deep problem here, in that file system case (and in some cases encoding) is not OS/platform dependent, but is rather *file system* dependent.

    Adding lower to normcase on OS X is probably not going to break much code (he says with no evidence to back it up), but someday it would be nice to address the various file system type dependencies in the os module :(

    In any case, it is probably better to address the 'lower' issue in a separate issue/patch.

    @bitdancer
    Copy link
    Member Author

    Ah, I see my lack of knowledge of OS X has betrayed me. Apparently you can set the case sensitivity of OS X file systems....so the deep problem is even deeper than I thought :(

    @ezio-melotti
    Copy link
    Member

    This problem should be addressed in another issue though.

    @ezio-melotti
    Copy link
    Member

    Fixed in r82214. Now os.path.normcase() raises a TypeError if the arg is not str or bytes.

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

    No branches or pull requests

    4 participants