[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Eryk Sun]

Eryk Sun  added the comment:

> I suspect that we've not used the flags in those ones because we 
> think we *can* properly format the message, so fixing the use of 
> inserts is a better option than simply going back to the 
> unformatted message.

We're advised that it's "unsafe to take an arbitrary system error code returned 
from an API and use FORMAT_MESSAGE_FROM_SYSTEM without 
FORMAT_MESSAGE_IGNORE_INSERTS". We don't control the content of system error 
messages. If we get the number and type of arguments wrong for the message 
inserts, then FormatMessageW may fail with an invalid parameter error, as it 
does in the examples I showed, or return nonsense, or even crash the process 
due to an unhandled access violation. 

Also, nothing was implemented in _overlapped.FormatMessage or 
_ctypes.FormatError to special case an `Arguments` array for particular error 
codes, or even to expose this capability to Python code. I think omitting the 
flag was just a mistake.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Steve Dower]

Steve Dower  added the comment:

> I suggested creating a new issue to fix the calls that omit this flag.

Right, that's a separate issue that is easily fixed (and backported, IMHO). 
Though I suspect that we've not used the flags in those ones because we think 
we *can* properly format the message, so fixing the use of inserts is a better 
option than simply going back to the unformatted message.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Eryk Sun]

Eryk Sun  added the comment:

>> A new issue should be raised to fix the FormatMessage calls in the 
>> standard library that mistakenly leave out 
>> FORMAT_MESSAGE_IGNORE_INSERTS.
>
> Do you suggest to modify OSError constructor to modify the call to
> FormatMessageW(): don't pass the FORMAT_MESSAGE_IGNORE_INSERTS flag?
> I prefer "%1 is not a valid Win32 application" message than 
> "".

I suggested creating a new issue to fix the calls that omit this flag. I think 
it's just two modules: Modules/overlapped.c and Modules/_ctypes/callproc.c. If 
there are more inserts than arguments (i.e. any inserts in our case since we 
pass no arguments), then FormatMessageW fails and the above modules use a 
default message. For example:

>>> _overlapped.FormatMessage(193)
'unknown error code 193'
>>> _ctypes.FormatError(193)
''

> There is no need to re-raise the exception: the "strerror" attribute 
> contains the error message and it can be modified.

I meant that Popen._execute_child would handle the exception by modifying and 
reraising it. In general for OSError exceptions, we could set `filename` to 
either `executable`, if it's defined, or else parse it out of the commandline. 
For ERROR_BAD_EXE_FORMAT (193), we could also change `strerror` to something 
like "Invalid executable format" instead of "%1 is not a valid Win32 
application". This is more consistent with how we append ": {filename}" to the 
message.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Raúl Núñez de Arenas]

Raúl Núñez de Arenas  added the comment:

That's the kind of patch I assumed was created for the Windows part, Berker, 
that's why I reopened the issue.

Thanks for the information.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Berker Peksag]

Berker Peksag  added the comment:

FYI, in msg261315, Eryk has mentioned about possible improvement of the 
exception message on POSIX. The filename has been added to the exception 
message in 8621bb5d93239316f97281826461b85072ff6db7:

>>> import subprocess, os
>>> os.access('log.txt', os.X_OK)
True
>>> subprocess.call(['./log.txt'])
Traceback (most recent call last):
  File "", line 1, in 
  File "/Users/berkerpeksag/projects/cpython/Lib/subprocess.py", line 325, in 
call
with Popen(*popenargs, **kwargs) as p:
  File "/Users/berkerpeksag/projects/cpython/Lib/subprocess.py", line 830, in 
__init__
self._execute_child(args, executable, preexec_fn, close_fds,
  File "/Users/berkerpeksag/projects/cpython/Lib/subprocess.py", line 1648, in 
_execute_child
raise child_exception_type(errno_num, err_msg, err_filename)
OSError: [Errno 8] Exec format error: './log.txt'

--
nosy: +berker.peksag
stage:  -> needs patch
versions: +Python 3.8 -Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Raúl Núñez de Arenas]

Raúl Núñez de Arenas  added the comment:

Hi Steve :) I assumed that the issue was solved, that's why I warned, in case a 
patch was applied and it didn't work.

When I did read "I suggest to only call reformat_strerror() when an user 
complains", I supposed a patch was ready O:)

No big deal, I can deal with that particular error in my code.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Steve Dower]

Steve Dower  added the comment:

It still happens because nobody has proposed a patch to specially handle this 
one particular error code. The earlier messages explained why we can't do 
formatting on error messages.

If this one is particularly common, then it's best for it to be handled in 
Python code at the point where it shows up and converted into something more 
readable.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Raúl Núñez de Arenas]

Raúl Núñez de Arenas  added the comment:

I'm sorry to say that this still happens in Python 3.7.0...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[STINNER Victor]

STINNER Victor added the comment:

See also issue #25585: "DLL load failed: %1 is not a valid Win32 application" 
message of ImportError exception. Hum, so maybe a function is better, since 
ImportError doesn't inherit from OSError.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[STINNER Victor]

STINNER Victor added the comment:

> A new issue should be raised to fix the FormatMessage calls in the standard 
> library that mistakenly leave out FORMAT_MESSAGE_IGNORE_INSERTS.

Do you suggest to modify OSError constructor to modify the call to 
FormatMessageW(): don't pass the FORMAT_MESSAGE_IGNORE_INSERTS flag?

I prefer "%1 is not a valid Win32 application" message than "".

Currently, PyErr_SetFromErrnoWithFilenameObjects() doesn't pass any argument to 
FormatMessageW(). The problem is that it looks like the expected argument 
depends a lot of the failing system call. I don't think that it's ok to always 
pass the filename (or two filenames when we get two filenames, ex: os.rename) 
to FormatMessageW().

For example, _winapi.CreateProcess() doesn't pass any filename to OSError 
constructor.


> It would be possible for subprocess to replace "%1" with the filename parsed 
> from the command line and then re-raise the exception.

I like the idea of formatting the error message in the subprocess module. IMHO 
it's much safer to reformat the error message from the function which raises 
the exception, since the function knows the system call, has all parameters to 
the system call, and may expect some specific system calls.

There is no need to re-raise the exception: the "strerror" attribute contains 
the error message and it can be modified.

We need an helper function or add a new method to the OSError class. Example of 
method:

def reformat_strerror(self, *args):
   sef.strerror = FormatMessageW(..., self.strerror, ..., args)

For this specific issue, I don't know if reformat_strerror() should be called 
from subprocess.py or CreateProcess function of Modules/_winapi.c.

I understand that we should call reformat_strerror() with application_name 
(first parameter of CreateProcess, "executable" in subprocess), but only if the 
error message is 193.

Since there are "a lot" of error messages and a lot of functions calling system 
calls, I don't think that it will be possible to handle all possible error 
messages in all Python functions. I suggest to only call reformat_strerror() 
when an user complains, only promise best effort ;-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[STINNER Victor]

Changes by STINNER Victor :


--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Eryk Sun]

Eryk Sun added the comment:

It would be possible for subprocess to replace "%1" with the filename parsed 
from the command line and then re-raise the exception. That said, it's not as 
if this is a deficiency in the Windows implementation relative to subprocess on 
POSIX. For example, in 3.4 on Linux it raises a generic ENOEXEC error:

>>> os.access('./test.txt', os.X_OK)
True
>>> subprocess.call(['./test.txt'])
Traceback (most recent call last):
  File "", line 1, in 
  File "/usr/lib/python3.4/subprocess.py", line 537, in call
with Popen(*popenargs, **kwargs) as p:
  File "/usr/lib/python3.4/subprocess.py", line 859, in __init__
restore_signals, start_new_session)
  File "/usr/lib/python3.4/subprocess.py", line 1457, in _execute_child
raise child_exception_type(errno_num, err_msg)
OSError: [Errno 8] Exec format error

It could provide the filename, for example:

>>> raise OSError(errno.ENOEXEC, os.strerror(errno.ENOEXEC), './test.txt')
Traceback (most recent call last):
  File "", line 1, in 
OSError: [Errno 8] Exec format error: './test.txt'

A new issue should be raised to fix the FormatMessage calls in the standard 
library that mistakenly leave out FORMAT_MESSAGE_IGNORE_INSERTS.

--
type: behavior -> enhancement

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Raúl Núñez de Arenas]

Raúl Núñez de Arenas added the comment:

Thanks for the information, Eryk, I've read the blog.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Eryk Sun]

Eryk Sun added the comment:

When Python creates an exception generically from a Windows error code, it 
calls WinAPI FormatMessage with the flag FORMAT_MESSAGE_IGNORE_INSERTS. The 
reason for this is explained in a blog post written by Raymond Chen[1]: "when 
you are not in control of the message, you had better pass the 
FORMAT_MESSAGE_IGNORE_INSERTS flag."

[1]: https://blogs.msdn.microsoft.com/oldnewthing/20071128-00/?p=24353

Some extension modules in the code base get this wrong. For example, 
ctypes.FormatError() doesn't use FORMAT_MESSAGE_IGNORE_INSERTS, so it fails to 
get the system error message for ERROR_BAD_EXE_FORMAT (193) and just returns 
its default string "":

>>> ctypes.FormatError(193)
''

--
components: +Windows
nosy: +eryksun, paul.moore, steve.dower, tim.golden, zach.ware
type:  -> behavior

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26493] Bad formatting in WinError 193 when using subprocess.check_call

[Raúl Núñez de Arenas]

New submission from Raúl Núñez de Arenas:

Python 3.5.1 x64 @ Windows 10 x64

The error message in the traceback for OSError/WinError 193 has bad formatting 
and the offending file name is not printed.

For example, this code:

import subprocess

testfile = open('testfile.notexecutable', 'wb')
testfile.close()
subprocess.check_call(['testfile.notexecutable'])


produces this output:

Traceback (most recent call last):
  File "test.py", line 6, in 
subprocess.check_call(['testfile.notexecutable'])
  File "C:\Program Files\Python35\lib\subprocess.py", line 579, in check_call
retcode = call(*popenargs, **kwargs)
  File "C:\Program Files\Python35\lib\subprocess.py", line 560, in call
with Popen(*popenargs, **kwargs) as p:
  File "C:\Program Files\Python35\lib\subprocess.py", line 950, in __init__
restore_signals, start_new_session)
  File "C:\Program Files\Python35\lib\subprocess.py", line 1220, in 
_execute_child
startupinfo)
OSError: [WinError 193] %1 is not a valid Win32 application


Please note the "%1 is not a valid..." on the last line. Instead of the "%1" 
placeholder, the filename ("testfile.notexecutable") should appear instead.

Thanks :)

--
messages: 261262
nosy: Raúl Núñez de Arenas
priority: normal
severity: normal
status: open
title: Bad formatting in WinError 193 when using subprocess.check_call
versions: Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com