classification
Title: file accepts 'rU+' as a mode
Type: behavior Stage: resolved
Components: Interpreter Core, IO Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, benjamin.peterson, brett.cannon, francismb, haypo, jcon, jeff.balogh, larry, pitrou, python-dev, r.david.murray, rbcollins, serhiy.storchaka
Priority: high Keywords: needs review, patch

Created on 2008-02-12 22:55 by brett.cannon, last changed 2015-07-25 18:59 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
issue2091.diff jeff.balogh, 2008-03-17 19:52 review
issue2091-2.patch jcon, 2011-06-01 00:59 Updated patch for new io review
issue2091-3.patch rbcollins, 2015-07-22 16:48 review
Messages (17)
msg62343 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-02-12 22:55
The docs on file's constructor says that the 'U' mode should not work
with '+', and yet 'rU+' does not throw an error.
msg63741 - (view) Author: Jeff Balogh (jeff.balogh) * Date: 2008-03-17 19:52
Attaching a patch that checks for '+' in the mode string, updates the 
docs, and tests bad mode strings.
msg104582 - (view) Author: Tres Seaver (tseaver) * Date: 2010-04-29 20:27
I can confirm that:

- the patch applies cleanly to the release26-maint branch, with the
  exception of the Misc/NEWS portion::

  $ hg branch
  release26-maint
  $ ./python -E -tt Lib/test/regrtest.py test_file
  test_file
  1 test OK.
  $ patch -p0 < /tmp/issue2091.diff 
  patching file Objects/fileobject.c
  Hunk #1 succeeded at 209 (offset 54 lines).
  Hunk #2 succeeded at 2269 (offset 194 lines).
  patching file Misc/NEWS
  Hunk #1 FAILED at 12.
  1 out of 1 hunk FAILED -- saving rejects to file Misc/NEWS.rej
  patching file Lib/test/test_file.py
  Hunk #1 succeeded at 176 (offset 55 lines).

- the added test fails before rebuilding Objects/fileobject.c::

  $ ./python -E -tt Lib/test/regrtest.py test_file
  test_file
  test test_file failed -- Traceback (most recent call last):
    File "/home/tseaver/projects/hg-repo/py-2.6/Lib/test/test_file.py", line 181, in testModeStrings
      f = open(TESTFN, mode)
  IOError: [Errno 2] No such file or directory: '@test'
  
  1 test failed:
      test_file

- the added test passes after rebuilding Objects/fielobject.c::

  $ make
  gcc -pthread -c -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes  -I. -IInclude -I./Include   -DPy_BUILD_CORE -o Objects/fileobject.o Objects/fileobject.c
  ...
  $ ./python -E -tt Lib/test/regrtest.py test_file
  test_file
  1 test OK.
msg116954 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-09-20 16:37
The patch is now way out of date to the extent that I can't find the code in fileobject.c, perhaps I'm just blind  Can someone please provide a new patch, thanks.
msg116955 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-09-20 17:33
Of course, the implementation is now in the io module:
Modules/_io/_iomodule.c *and* Lib/_pyio.py
msg136425 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2011-05-21 09:29
In Python 3, the documentation no longer mentions that 'U' should not work with '+' (or 'w' or 'a', for that matter), and the code throws ValueError if 'U' is used with 'w' or 'a', but not '+'.

On the other hand, the documentation *does* mention that 'U' is for backwards compatibility and shouldn't be used with new code.

In light of this, how do you suggest to proceed with this issue?
msg137409 - (view) Author: John O'Connor (jcon) Date: 2011-06-01 00:59
It seems to me that adding the proper check is a good idea. It also may not be obvious to some that 'U' is now more or less an alias for 'r'. I have updated the patch so that it at least applies.

I also removed a redundant `reading = 1`
msg223368 - (view) Author: Francis MB (francismb) * Date: 2014-07-17 20:32
> On the other hand, the documentation *does* mention that
> 'U' is for backwards compatibility and shouldn't be used
> with new code.

Shouldn't be some deprecation warning somewhere?
msg223810 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-24 05:26
In Python 3.4+ 'U' already emits deprecation warning.
msg247133 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-07-22 16:48
Updated patch. I'm not going to apply right now - giving it a little time to let folk chime on whether this should be applied all the way back to 3.4, or not.

My inclination is to only apply it to 3.6.
msg247193 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-07-23 13:23
I agree, a change that emits an error where none was emitted before should only go in the next release.  With a what's new entry.

It is possible there should be a deprecation or -3 warning in 2.7, but I'm not sure it is worth the effort.
msg247212 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-07-23 14:57
@Larry - should this go in 3.5, or would you rather it not?
msg247215 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-07-23 15:21
I would add a warning in 3.4- (or 3.5-). The 'U' mode should not work
with '+', and it can't work with '+' correctly. This is a bug that we can decide not fix by raising an exception.
msg247334 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-07-25 10:14
Yeah, considering how long this bug has been sitting around, I think we can wait for one more release for the fix.  3.6 please.
msg247365 - (view) Author: Roundup Robot (python-dev) Date: 2015-07-25 18:43
New changeset 1a5bbb31f740 by Robert Collins in branch 'default':
- Issue #2091: error correctly on open() with mode 'U' and '+'
https://hg.python.org/cpython/rev/1a5bbb31f740
msg247366 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-07-25 18:45
@larry thanks - that was my inclination too. Applied to 3.6

@Serhiy I've not done a warning in 3.4/3.5 because - the behaviour is already broken, this patch just catches it a lot earlier (on open rather than subsequent operations).
msg247370 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-07-25 18:59
Perhaps the 'U' mode should just raise an exception in 3.6.
History
Date User Action Args
2015-07-25 18:59:21serhiy.storchakasetmessages: + msg247370
2015-07-25 18:45:53rbcollinssetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2015-07-25 18:45:39rbcollinssetmessages: + msg247366
2015-07-25 18:43:33python-devsetnosy: + python-dev
messages: + msg247365
2015-07-25 10:14:20larrysetmessages: + msg247334
2015-07-23 15:21:05serhiy.storchakasetmessages: + msg247215
2015-07-23 14:57:19rbcollinssetnosy: + larry
messages: + msg247212
2015-07-23 13:23:01r.david.murraysetnosy: + r.david.murray
messages: + msg247193
2015-07-22 16:48:46rbcollinssetfiles: + issue2091-3.patch
nosy: + rbcollins
messages: + msg247133

2014-07-24 05:26:59serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg223810
2014-07-23 22:42:51BreamoreBoysetversions: + Python 3.5, - Python 3.2, Python 3.3
2014-07-17 20:32:10francismbsetnosy: + francismb
messages: + msg223368
2014-02-03 19:07:49BreamoreBoysetnosy: - BreamoreBoy
2012-10-06 03:43:42ezio.melottisetversions: + Python 3.3, Python 3.4, - Python 3.1
2011-07-24 03:21:26eli.benderskysetnosy: - eli.bendersky
2011-06-01 01:00:00jconsetfiles: + issue2091-2.patch
nosy: + jcon
messages: + msg137409

2011-05-21 16:25:17tseaversetnosy: - tseaver
2011-05-21 09:29:42eli.benderskysetmessages: + msg136425
2011-05-21 08:09:23eli.benderskysetnosy: + eli.bendersky
2010-09-20 17:33:02amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg116955
2010-09-20 16:37:33BreamoreBoysetnosy: + BreamoreBoy

messages: + msg116954
versions: + Python 3.1, Python 3.2, - Python 2.6
2010-05-05 00:45:40hayposetnosy: + haypo
2010-05-02 17:17:41brett.cannonsetkeywords: + needs review
stage: patch review -> commit review
2010-04-29 20:32:51pitrousetversions: + Python 2.7, - Python 3.1
2010-04-29 20:27:41tseaversetnosy: + tseaver
messages: + msg104582
2009-04-28 00:06:45ajaksu2setnosy: + pitrou, benjamin.peterson
2009-04-28 00:06:10ajaksu2setnosy: brett.cannon, jeff.balogh
versions: + Python 3.1, - Python 2.5
components: + IO
stage: patch review
2008-03-17 19:52:40jeff.baloghsetfiles: + issue2091.diff
nosy: + jeff.balogh
messages: + msg63741
keywords: + patch
2008-02-14 20:20:49christian.heimessetpriority: high
2008-02-12 22:55:09brett.cannoncreate