classification
Title: IDLE - sys.stdin is writeable
Type: behavior Stage:
Components: IDLE Versions: Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: loewis, python-dev, roger.serwy, serhiy.storchaka, terry.reedy
Priority: low Keywords: patch

Created on 2012-07-10 14:13 by roger.serwy, last changed 2012-07-25 08:57 by loewis. This issue is now closed.

Files
File name Uploaded Description Edit
blockwrite.diff loewis, 2012-07-11 07:24 review
blockwrite.diff loewis, 2012-07-11 07:38 review
idle_stdstreams-2.patch serhiy.storchaka, 2012-07-11 09:13 review
idle_stdstreams-3.patch serhiy.storchaka, 2012-07-11 09:35 review
blockfile-3.diff loewis, 2012-07-11 21:32 review
blockfile-4.diff roger.serwy, 2012-07-12 00:33 review
Messages (25)
msg165190 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-07-10 14:13
This is a follow-up to issue13532 which fixed problems with IDLE's sys.stdout and sys.stderr implementation.

Presently, sys.stdin has a write method, but does not raise "io.UnsupportedOperation: not writable" when passed a string. Instead, it writes to the IDLE shell.
msg165215 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-07-11 01:25
To me, this is part of #15319. sys.stdin is broken in being writable but not readable. Unwrapping is just the first, trivial step to fixing it. So I think we should close this in favor of the other.
msg165216 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-07-11 01:42
I agree. I'll close it.
msg165230 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-11 06:35
It's *clearly* not a duplicate. While #15319 is fixed, this issue remains - so it can't possibly be a duplicate.
msg165231 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-11 07:24
I propose the attached patch to block write() calls.
msg165232 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-11 07:38
Revised
msg165240 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-11 09:13
sys.std* streams have many other issues.

>>> sys.stdin.readable()
Traceback (most recent call last):
  File "<pyshell#4>", line 1, in <module>
    sys.stdin.readable()
AttributeError: readable
>>> sys.stdout.writable()
Traceback (most recent call last):
  File "<pyshell#5>", line 1, in <module>
    sys.stdout.writable()
AttributeError: writable
>>> sys.stdout.newlines
Traceback (most recent call last):
  File "<pyshell#8>", line 1, in <module>
    sys.stdout.newlines
AttributeError: newlines
>>> sys.stdin.read(1)
Traceback (most recent call last):
  File "<pyshell#3>", line 1, in <module>
    sys.stdin.read(1)
AttributeError: read

And some other.

Here is a patch that fixes almost all IDLE sys.std* issues.
msg165241 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-11 09:35
Added getvar method to PyShell.console.
msg165242 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-11 09:51
Serhiy: can you please explicitly list what issues your patch fixes?

I can think of many *more* issues that your patch doesn't fix, e.g. sys.stdin.read(sys) and sys.stdout.read().

I'm quite opposed to monkey-patching, so I won't commit any patch that involves monkey-patching (i.e. _checked_text_writer). I'm also unsure what problem this change to monkey-patching solves: I believe the current code is fine in this respect as it stands.

Before you grow the patch to add many more lines of code, please try to stick to the "one issue at a time" principle. As a guideline: if you need to write additional lines of code, it's a separate issue.
msg165245 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-11 10:10
As an additional note: the change to monkey-patching breaks the test 

isinstance(sys.stdout, io.TextIOBase)

which currently succeeds.
msg165249 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-11 11:16
> Serhiy: can you please explicitly list what issues your patch fixes?

issue12967. And sys.std* in IDLE lacks other common sys.std* methods and
attributes.

> I can think of many *more* issues that your patch doesn't fix, e.g. sys.stdin.read(sys) and sys.stdout.read().

sys.stdin.read(sys) and sys.stdout.read() should not work.

> I'm quite opposed to monkey-patching, so I won't commit any patch that involves monkey-patching (i.e. _checked_text_writer). I'm also unsure what problem this change to monkey-patching solves: I believe the current code is fine in this respect as it stands.

I introduced _checked_text_writer, because the implementation of
_RPCFile was broken (I wrote this patch last evening and debug it this
morning). In addition, _RPCFile is a wrapper around RPCFile, which
itself is a wrapper. There are too many layers for me. If you are sure
that the current implementation of _RPCFile is correct, it is not
necessary.

> Before you grow the patch to add many more lines of code, please try to stick to the "one issue at a time" principle.

In IDLE sys.std* bad mimic text streams -- this is one issue.
msg165251 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-11 11:23
> As an additional note: the change to monkey-patching breaks the test 
> 
> isinstance(sys.stdout, io.TextIOBase)
> 
> which currently succeeds.

Agree, this is a regression. However isinstance(sys.stdout,
io.TextIOBase) is False now in no-subprocess mode. This should be fixed
also.
msg165259 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-11 14:58
issue9290 also has relation to this.
msg165260 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-11 15:00
issue7163 is another PseudoFile-related issue.
msg165269 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-11 19:51
"implements textio badly" is *not* a well-defined issue.

First it is a personal judgement: some think it's really bad, some think it's quite ok.

More importantly, such issue has no clear success criterion: how do we know we are done - can we hope to EVER close such an issue? No, because somebody might still find bad behavior. An issue absolutely needs anobjective test to determine whether it is resolved, and to reproduce it, this is not negotiable. "it behaves badly" provides no test.

In any case,*this* issue is about sys.stdin being writable.

Any objection to checking in my patch?
msg165271 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-11 20:32
> In any case,*this* issue is about sys.stdin being writable.

I already got confused with all these closing/reopening/renaming of
issues and floating code. Should I open my own issue even if it
duplicates and supersedes some other?

> Any objection to checking in my patch?

It does not fixes the issue. sys.stdin.writelines does not raise
exception.

Inheritance input file from output file looks very strange.
_RPCInputFile unnecessary, if redirect stdin not on PyShell self, but on
PyShell.stdin.
msg165274 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-11 21:34
> I already got confused with all these closing/reopening/renaming of
> issues and floating code. Should I open my own issue even if it
> duplicates and supersedes some other?

I personally think it would be best if these issues where closed
*first*, and only then you submit a patch to fix any remaining issues.

>> Any objection to checking in my patch?
> 
> It does not fixes the issue. sys.stdin.writelines does not raise
> exception.

Ah. See blockfiles-3.diff then.

> Inheritance input file from output file looks very strange.

Where do you see that? In my patch, _RPCInputFile inherits from
_RPCFile, which is neither output nor input. Instead, _RPCOutputFile
also inherits from _RPCFile.

> _RPCInputFile unnecessary, if redirect stdin not on PyShell self, but on
> PyShell.stdin.

It is necessary, as rpc will not communicate the exceptions properly.
msg165277 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-07-12 00:33
I tested blockfile-3.diff against the latest code in the repository.

sys.stdin.write returns the wrong error message when passed a non-string. Presently it returns io.UnsupportedOperation instead of TypeError: must be str, not ...

The attached blockfile-4.diff incorporates blockfile-3 and creates a decorator in _RPCFile to ensure the argument is a string and applies it to the _RPCInputFile and _RPCOutputFile write methods.

A simpler alternative would be duplicating the string check code in _RPCInputFile before the raise statement.
msg165291 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-12 09:13
> Ah. See blockfiles-3.diff then.

Well, I have no objections. Patch fixes this issue.

> Where do you see that? In my patch, _RPCInputFile inherits from
> _RPCFile, which is neither output nor input. Instead, _RPCOutputFile
> also inherits from _RPCFile.

I made a mistake. I mean PseudoInputFile, but it just monkey-patched.

> > _RPCInputFile unnecessary, if redirect stdin not on PyShell self, but on
> > PyShell.stdin.
> It is necessary, as rpc will not communicate the exceptions properly.

Yes, this is yet another issue. Rpc not communicates properly only
OSError. Remove "except socket.error: raise" in SocketIO.localcall in
Lib/idlelib/rpc.py and io.UnsupportedOperation will transparently
transfered.
msg165292 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-12 09:13
> sys.stdin.write returns the wrong error message when passed a non-string. Presently it returns io.UnsupportedOperation instead of TypeError: must be str, not ...

It's not a bug. sys.stdin.write raises io.UnsupportedOperation in
standard interpreter.
msg165307 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-07-12 15:05
On 07/12/2012 04:13 AM, Serhiy Storchaka wrote:
> Serhiy Storchaka<storchaka@gmail.com>  added the comment:
>
>> sys.stdin.write returns the wrong error message when passed a non-string. Presently it returns io.UnsupportedOperation instead of TypeError: must be str, not ...
> It's not a bug. sys.stdin.write raises io.UnsupportedOperation in
> standard interpreter.

Here's what I get from the standard interpreter:

     Python 3.3.0b1 (default:4752fafb579d, Jul 11 2012, 22:05:03)
     [GCC 4.5.2] on linux
     Type "help", "copyright", "credits" or "license" for more information.
 >>> import sys
 >>> sys.stdin.write(123)
     Traceback (most recent call last):
       File "<stdin>", line 1, in <module>
     TypeError: must be str, not int
 >>>

> ----------
>
> _______________________________________
> Python tracker<report@bugs.python.org>
> <http://bugs.python.org/issue15318>
> _______________________________________
>
msg165316 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-07-12 16:04
>>> sys.stdin.write('qwe')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
io.UnsupportedOperation: not writable
msg165319 - (view) Author: Roger Serwy (roger.serwy) * (Python committer) Date: 2012-07-12 16:14
>>>> sys.stdin.write('qwe')
> Traceback (most recent call last):
>    File "<stdin>", line 1, in<module>
> io.UnsupportedOperation: not writable
>
> ----------

I'm passing a number, *not* a string.
msg166368 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-07-25 08:49
New changeset a82fd35e28be by Martin v. Löwis in branch '3.2':
Issue #15318: Prevent writing to sys.stdin.
http://hg.python.org/cpython/rev/a82fd35e28be
msg166369 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-07-25 08:56
New changeset fa7d4ecc6357 by Martin v. Löwis in branch '2.7':
Issue #15318: Prevent writing to sys.stdin.
http://hg.python.org/cpython/rev/fa7d4ecc6357
History
Date User Action Args
2012-07-25 08:57:02loewissetstatus: open -> closed
resolution: fixed
2012-07-25 08:56:34python-devsetmessages: + msg166369
2012-07-25 08:49:51python-devsetnosy: + python-dev
messages: + msg166368
2012-07-12 16:14:12roger.serwysetmessages: + msg165319
2012-07-12 16:04:37serhiy.storchakasetmessages: + msg165316
2012-07-12 15:05:18roger.serwysetmessages: + msg165307
2012-07-12 09:13:45serhiy.storchakasetmessages: + msg165292
2012-07-12 09:13:18serhiy.storchakasetmessages: + msg165291
2012-07-12 00:33:26roger.serwysetfiles: + blockfile-4.diff

messages: + msg165277
2012-07-11 21:34:39loewissetmessages: + msg165274
2012-07-11 21:32:10loewissetfiles: + blockfile-3.diff
2012-07-11 21:30:10loewissetfiles: - blockfile-3.diff
2012-07-11 21:29:27loewissetfiles: + blockfile-3.diff
2012-07-11 20:32:42serhiy.storchakasetmessages: + msg165271
2012-07-11 19:51:34loewissetmessages: + msg165269
2012-07-11 15:00:58serhiy.storchakasetmessages: + msg165260
2012-07-11 14:58:33serhiy.storchakasetmessages: + msg165259
2012-07-11 11:23:40serhiy.storchakasetmessages: + msg165251
2012-07-11 11:16:08serhiy.storchakasetmessages: + msg165249
2012-07-11 10:10:19loewissetmessages: + msg165245
2012-07-11 09:51:12loewissetmessages: + msg165242
2012-07-11 09:35:16serhiy.storchakasetfiles: + idle_stdstreams-3.patch

messages: + msg165241
2012-07-11 09:13:10serhiy.storchakasetfiles: + idle_stdstreams-2.patch

messages: + msg165240
2012-07-11 07:38:15loewissetfiles: + blockwrite.diff

messages: + msg165232
2012-07-11 07:24:15loewissetfiles: + blockwrite.diff
keywords: + patch
messages: + msg165231
2012-07-11 06:50:36loewissetstatus: closed -> open
2012-07-11 06:35:03loewissetnosy: + loewis
messages: + msg165230
resolution: duplicate -> (no value)

superseder: IDLE - readline, isatty, and input broken ->
2012-07-11 01:43:46roger.serwysetsuperseder: IDLE - readline, isatty, and input broken
2012-07-11 01:42:30roger.serwysetstatus: open -> closed
resolution: duplicate
messages: + msg165216
2012-07-11 01:25:43terry.reedysetmessages: + msg165215
2012-07-10 14:13:24roger.serwycreate