This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Python does not check for POSIX compliant open() modes
Type: Stage:
Components: Interpreter Core Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: georg.brandl Nosy List: georg.brandl, splitscreen, tim.peters
Priority: normal Keywords:

Created on 2006-03-31 15:02 by splitscreen, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fileobject2.diff splitscreen, 2006-04-01 21:59 Latest patch
file-modes.diff georg.brandl, 2006-04-06 08:15 Patch by gbrandl
Messages (13)
msg27975 - (view) Author: Matt Fleming (splitscreen) Date: 2006-03-31 15:02
Python does not check for POSIX-compliant modes when
passing them to open() and fopen().

According to the POSIX standard all modes should start
with either an 'a', 'r' or 'w'.

This patch implements this check in the
check_the_mode() function of fileobject.c and a test
has been modified in the test_file.py test.
msg27976 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2006-03-31 18:03
Logged In: YES 
user_id=31435

There's a small danger of backward-incompatibility here,
since platforms are free to support any goofy mode
characters they like, and Python just passes on whatever the
user typed.  I understand that some MS compilers in debug
mode raise internal exceptions, but that's an MS bug and the
workaround is dead easy ("don't do that").

All that said, I'm in favor of this patch ;-).  However, if
it goes in two things are needed:

1. The error message should be more informative, like

   PyErr_Format(PyExc_ValueError, "mode argument must "
     "begin with 'w', 'r', or 'a', not '%.200s'", mode);

2. The Python docs should change to match (i.e., the
   manual should document this new restriction on mode
   strings).

WRT the test case, it's more natural to write, e.g.,

    TESTFN in s

than

    s.find(TESTFN) != -1
msg27977 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-04-01 07:08
Logged In: YES 
user_id=849994

Instead of this patch, I'd rather like check_the_mode to

* remove any 'U' from the mode string
* if 'U' was found:
  * error out if the new string begins with 'w' or 'a'
  * add a 'r' and 'b' if it isn't already given
* if not:
  * error out if the string doesn't begin with 'w', 'r', 'a'

What do you think of it?
msg27978 - (view) Author: Matt Fleming (splitscreen) Date: 2006-04-01 14:14
Logged In: YES 
user_id=1126061

That seems sensible. What does a mode containing 'U' mean
anyway?
msg27979 - (view) Author: Matt Fleming (splitscreen) Date: 2006-04-01 19:32
Logged In: YES 
user_id=1126061

Ignore my question, it's for "universal newlines", right? 

Have I understood your proposal correctly in that you want
to remove the 'U' from the mode? 
msg27980 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-04-01 20:43
Logged In: YES 
user_id=849994

Yes, I want to remove 'U' from the mode since it's at this
point already recognized by Python, and it's not meaningful
to the underlying C library.
msg27981 - (view) Author: Matt Fleming (splitscreen) Date: 2006-04-01 21:59
Logged In: YES 
user_id=1126061

Ok, uploading latest patch, not quite sure I've hit the mark
here.

Please review.
msg27982 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-04-06 08:15
Logged In: YES 
user_id=849994

splitscreen: your patch was incomplete and could have
overwritten memory.

tim_one: Attaching new patch implementing what I proposed in
my comment below.
msg27983 - (view) Author: Matt Fleming (splitscreen) Date: 2006-04-12 10:14
Logged In: YES 
user_id=1126061

Yeah, your patch looks much better.
msg27984 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2006-04-24 01:19
Logged In: YES 
user_id=31435

This is still ;-) fine with me.  There doesn't seem to be a
test in fileobject2.diff.  WRT the docs, "must either be"
should say "must be one of" instead ("either" is proper for
two choices, but there are three).  It should state that
this requirement is new in Python 2.5; in LaTeX, something like

\versionchanged[Restriction on first letter of mode string
introduced]{2.5}
msg27985 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-04-28 09:55
Logged In: YES 
user_id=849994

Tim: Did you review file-modes.diff which is really the
latest patch? (There's no test in it either, but I'll add
one when I check it in)
msg27986 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2006-05-10 02:16
Logged In: YES 
user_id=31435

You're right, I reviewed the older patch.  One of the same
comments, though, plus a new one:

1. The docs should state that this requirement is new in
Python 2.5; in LaTeX, something like

\versionchanged[Restriction on first letter of mode string
introduced]{2.5}

2. We can never assume that allocating memory will work:   after

    newmode = PyMem_MALLOC(strlen(mode) + 3);

you must guard against `newmode` being NULL (and raise an
exception it if is; e.g., call PyErr_NoMemory()).
msg27987 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-05-18 07:01
Logged In: YES 
user_id=849994

Corrected and committed in rev. 46037.
History
Date User Action Args
2022-04-11 14:56:16adminsetgithub: 43120
2006-03-31 15:02:22splitscreencreate