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: subprocess.communicate() breaks on no input with universal newlines true
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, chris.jerdonek, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2012-08-08 16:37 by chris.jerdonek, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
failing-test-case-1.patch chris.jerdonek, 2012-08-13 20:09 review
issue-15592-1.patch chris.jerdonek, 2012-08-13 21:22 review
Messages (11)
msg167694 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-08 16:37
Popen.communicate() raises an exception if passed no input when stdin=PIPE and universal_newlines=True.  With universal_newlines=False, no exception is raised.  For example, the following yields--

args = [sys.executable, '-c', 'pass']

popen = Popen(args, universal_newlines=False, stdin=PIPE, stdout=PIPE, stderr=PIPE)
print(popen.communicate())

popen = Popen(args, universal_newlines=True, stdin=PIPE, stdout=PIPE, stderr=PIPE)
print(popen.communicate())

(b'', b'[41449 refs]\n')
Traceback (most recent call last):
  ...
  File ".../Lib/subprocess.py", line 1581, in _communicate_with_poll
    self._input = self._input.encode(self.stdin.encoding)
AttributeError: 'NoneType' object has no attribute 'encode'

It seems like communicate() should not be trying to encode input if there is none.

I can provide a patch with tests if it is agreed this is an issue (along with a fix assuming it is reasonable).
msg167697 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-08 16:40
See issue 12623 for another issue related to communicate() and universal newline support.
msg168097 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-13 12:50
_communicate_with_select has the same problem as _communicate_with_poll.

I don't understand why input has encoded if  universal_newlines and passed unchanged otherwise.

From my perspective input should be encoded (converted to bytes) if it is str regardless of universal_newlines value.
msg168112 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-13 14:31
> From my perspective input should be encoded (converted to bytes) if it is str regardless of universal_newlines value.

I don't know the reason, but the limitation is documented:

"The optional input argument should be data to be sent to the child process, or None, if no data should be sent to the child. The type of input must be bytes or, if universal_newlines was True, a string."

http://docs.python.org/dev/library/subprocess.html#subprocess.Popen.communicate

Maybe "explicit better than implicit"?
msg168127 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-13 17:03
This issue may be the bug referenced here:

# BUG: can't give a non-empty stdin because it breaks both the
# select- and poll-based communicate() implementations.
(stdout, stderr) = p.communicate()

http://hg.python.org/cpython/file/843e0da7e91f/Lib/test/test_subprocess.py#l618

The reason is that the above test would fail if stdin were not None since the input to communicate is None.

I am preparing a test and patch for this issue.
msg168139 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-13 19:55
I like to accept both str and bytes if universal_newlines is False and raise explicit exception for bytes if universal_newlines is True.

Looks like universal_newlines for bytes doesn't make sense, also we prefer to use str everywhere. Bytes should be used if it is really byte-stream or if unicode cannot be passed by some limitations of underlying OS etc.

There are couple dark corners:
1. What to do if stdin doesn't have `encoding` attribute? Convert to bytes using filesystemencoding? Raise explicit exception? Adding `encode` parameter for .communicate doesn't looks elegant.
2. How .communicate works on Windows? I see completely different implementation of this method for win32.

I need some time to investigate before publish proposal to python-dev list.
msg168141 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-13 20:09
Attaching a failing test case.

Also, to confirm, this issue does not seem to affect 3.2.
msg168145 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-13 21:22
Attaching patch.
msg168204 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-08-14 15:43
New changeset 839bd8f98539 by Andrew Svetlov in branch '3.2':
Add test to explicit check the absence regression in subprocess (issue #15592).
http://hg.python.org/cpython/rev/839bd8f98539

New changeset 9d86480cc177 by Andrew Svetlov in branch 'default':
Issue #15592. Fix regression: subprocess.communicate() breaks on no input with universal newlines true.
http://hg.python.org/cpython/rev/9d86480cc177
msg168208 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-14 15:49
Close the issue. Thanks, Chris.
It was the real regression, 3.2 works fine.

See also #15649 for accepting str if not universal_newlines.
msg168209 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-14 16:29
Thanks, Andrew!
History
Date User Action Args
2022-04-11 14:57:34adminsetgithub: 59797
2012-08-14 16:29:16chris.jerdoneksetstatus: open -> closed
resolution: fixed
messages: + msg168209

stage: patch review -> resolved
2012-08-14 15:49:47asvetlovsetmessages: + msg168208
2012-08-14 15:43:10python-devsetnosy: + python-dev
messages: + msg168204
2012-08-13 21:22:48chris.jerdoneksetfiles: + issue-15592-1.patch

messages: + msg168145
stage: needs patch -> patch review
2012-08-13 20:09:58chris.jerdoneksetstage: test needed -> needs patch
2012-08-13 20:09:43chris.jerdoneksetfiles: + failing-test-case-1.patch
keywords: + patch
messages: + msg168141
2012-08-13 19:55:45asvetlovsetmessages: + msg168139
2012-08-13 17:03:27chris.jerdoneksetmessages: + msg168127
2012-08-13 14:31:54chris.jerdoneksetmessages: + msg168112
2012-08-13 12:50:39asvetlovsetnosy: + asvetlov
messages: + msg168097
2012-08-08 16:40:31chris.jerdoneksetnosy: + pitrou
messages: + msg167697
2012-08-08 16:37:59chris.jerdonekcreate