======================================================================
ERROR: test_module_funcs (test.test_curses.TestCurses)
Test module-level functions
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/sdcard/org.bitbucket.pyona/lib/python3.7/test/test_curses.py", line 219, in test_module_fun
cs
    self.stdscr.putwin(f)
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/py.curses.putwin.nsIZYY'

----------------------------------------------------------------------

I guess replace mkstemp (C function) with tempfile.mkstemp (Python function) can solve the problem.

The test suite runner in the test.libregrtest.main module calls tempfile.gettempdir() and mkstemp_curses.patch uses the same logic to fix the problem on Android where the tests that I run use TMPDIR=/data/local/tmp on Android API 24.

As a side note, when the curses library is ncurses-6.0, the library must be patched in ncurses/base/lib_screen.c with the attached ncurses.patch, see:
    http://lists.gnu.org/archive/html/bug-ncurses/2016-05/msg00000.html
    https://bugs.python.org/issue27323

The putwin() function uses mkstemp() with the path template /tmp/py.curses.getwin.XXXXXX.

I would prefer to use the Python function tempfile.mkstemp(). This function has a more portable behaviour. For example, Python is able to atomatically make the file descriptor non-inherirable if the OS supports this feature.

I suggest to expect a file descriptor in the C putwin() and write a Python putwin() which expects a file and pass file.fileno() if available, or use tempfile.TemporaryFile().

tempfile.TemporaryFile is even more secure because the file is not accessible from the regular file system on most platforms. This function is able to use the secure Linux O_TMPFILE flag and the O_TEMPORARY flag on Windows.

I change the issue type to security.

Oops, I missed Chi Hsuan Yen's comment:
> I guess replace mkstemp (C function) with tempfile.mkstemp (Python function) can solve the problem.

So yes, I agree :-)

curses_temporaryfile.patch: Call tempfile.TemporaryFile in getwin() and putwin() to create the temporary file in a more portable and safer manner.

The curses library requires a FILE* object, but TemporaryFile.close() must be called to remove the temporary file. So I chose to duplicate the file descriptor, so we can close fclose() and tmpfile.close().

I chose to modify the C rather rather than trying to implement a window class in Python, because the C window class is part of a "PyCurses_API", and many C functions instanciate the C class.

curses_fix_window_class_name.patch: Fix the name of the C window class: "_curses.window", not "_curses.curses window" (with a space in the class name) !?

The following example current displays <class '_curses.curses window'>:
---
import curses
w = curses.initscr()
curses.endwin()
print(type(w))
---

Check that pickling/unpickling the windows object doesn't cause crash or undefined behaviour. Currently the pickling fails because the name of the class is not true name.

Serhiy Storchaka: "Check that pickling/unpickling the windows object doesn't cause crash or undefined behaviour. Currently the pickling fails because the name of the class is not true name."

Ah, good idea.

I tested with curses_fix_window_class_name.patch: window objects cannot be pickled. Hopefully! IMHO it doesn't make sense to serialize such "live" object.

haypo@selma$ cat > x.py
import curses
w = curses.initscr()
curses.endwin()
print(type(w))
^D

haypo@selma$ ./python -i x.py
<class '_curses.window'>
>>> import pickle
>>> pickle.dumps(w)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't pickle _curses.window objects

I created https://github.com/python/cpython/pull/52 for  curses_fix_window_class_name.patch.

I created the PR https://github.com/python/cpython/pull/53 for  curses_temporaryfile.patch.

Here are test results for PR 53:

shell@ASUS_Z00E_2:/data/local/tmp $ python3.7m -m test -u curses test_curses
Run tests sequentially
0:00:00 [1/1] test_curses
1 test OK.

Total duration: 1 sec
Tests result: SUCCESS
abcshell@ASUS_Z00E_2:/data/local/tmp $ 

I know little about the C API. Just a question: isn't PyImport_ImportModuleNoBlock deprecated?

Thank you for your check! Good to know that it works on Android.

Chi Hsuan Yen added the comment:
> I know little about the C API. Just a question: isn't PyImport_ImportModuleNoBlock deprecated?

I am not aware of any deprecation:
https://docs.python.org/dev/c-api/import.html#c.PyImport_ImportModuleNoBlock

Oh, it seems like the function is now more an less an alias to
PyImport_ImportModule().

I wasn't aware that the "NoBlock" is no more needed: I copied/pasted
from a different .c file, I don't recall which one. I should updated
other files ;-)

How about https://linux.die.net/man/3/tmpfile instead?

The tmpfile() function opens a unique temporary file in binary read/write (w+b) mode. The file will be automatically deleted when it is closed or the program terminates.

Is tmpfile() available on Android?

> Is tmpfile() available on Android?

Yes. Just tried it with Android NDK r13 with clang and API 21 (Android 5.0). Here's the source code test.c:

#include <stdio.h>

int main()
{
    FILE *fp = NULL;
    char c = '\0';

    fp = tmpfile();
    fprintf(fp, "123\n");
    scanf("%c", &c);
    fclose(fp);
    return 0;
}

Compilation command:

/opt/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -target aarch64-none-android-linux -gcc-toolchain /opt/android-ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64 --sysroot /opt/android-ndk/platforms/android-21/arch-arm64/usr -pie test.c

(didn't test gcc as it's declared as 'deprecated' in NDK)

Before scanf() returns, the temporary file is already deleted:

shell@ASUS_Z00E_2:/ $ ls -l /proc/19300/fd                                     
lrwx------ shell    shell             2017-02-22 15:21 0 -> /dev/pts/0
lrwx------ shell    shell             2017-02-22 15:21 1 -> /dev/pts/0
lrwx------ shell    shell             2017-02-22 15:21 2 -> /dev/pts/0
lrwx------ shell    shell             2017-02-22 15:20 3 -> /data/local/tmp/tmp.VguNf9sqcW (deleted)
lr-x------ shell    shell             2017-02-22 15:21 9 -> /dev/__properties__

For interested, here's its implementation: https://android.googlesource.com/platform/bionic/+/android-5.0.0_r1/libc/bionic/tmpfile.cpp

Thanks Chi Hsuan Yen,

I have created a new PR that replaces mkstemp() with tmpfile().

I created https://github.com/python/cpython/pull/237 using tmpfile(). The patch is simpler than my previous PR using Python tempfile.

Oh... I didn't see that Christian wrote almost the same change 20 min before me :-D

The manpage of tmpfile contains the note:

       POSIX.1-2001 specifies: an error message may be written to stdout if the stream cannot be opened.

Did you tested tmpfile() with readonly temporary files directory or overfull file system?

I think we can live with the fact some tmpfile() might write an error message if the system is broken. If user's tmp is not writeable, the user has more serious issues than a write to stdout inside curses module.

I hope that this never happen in real world, but in theory this change can introduce regression. That is why I think it should be documented in Misc/NEWS.

> POSIX.1-2001 specifies: an error message may be written to stdout if the stream cannot be opened.

At least, I don't see such message in the Android implementation:
https://android.googlesource.com/platform/bionic/+/android-5.0.0_r1/libc/bionic/tmpfile.cpp

Xavier de Gaye, Chi Hsuan Yen: I just merged Christian Heimes's PR. So the function should now work on Android. Can you double check that it works? Is it worth it to backport the change to Python 3.6? Do you plan to try to have a full Android support in Python 3.6? Or do you target Python 3.7?

I don't think that the backport is worth it, but the patch is simple enough, so it's not a big deal if it makes your life simpler :-)

me: "I change the issue type to security"

I'm not sure anymore that the change is really related to security. I don't think that replacing mkstemp("/tmp/py.curses.putwin.XXXXXX") with tmpfile() has an impact on security. So I don't think that it's worth it to backport to change to Python 2.7 and 3.3-3.5.

I've just tried with the latest git-master. With NDK r13b and my build scripts, [1] test_curses passed.

shell@ASUS_Z00E_2:/data/local/tmp $ python3.7m -m test test_curses -u curses   
Run tests sequentially
0:00:00 [1/1] test_curses
1 test OK.

Total duration: 1 sec
Tests result: SUCCESS
abcshell@ASUS_Z00E_2:/data/local/tmp $ 

By the way, with NDK r14 and unified headers, [2] the test also passed. Note that this configuration is not officially supported by CPython yet. (issue29040)

[1] https://github.com/yan12125/python3-android/tree/ndk-r13
[2] https://github.com/yan12125/python3-android/tree/master

New changeset 7253aded71df8d4bf8684fc78d11c596079211b3 by Victor Stinner (Mariatta) in branch '3.6':
bpo-29176: Fix name of the _curses.window class (#52) (#532)
https://github.com/python/cpython/commit/7253aded71df8d4bf8684fc78d11c596079211b3

New changeset 2b221b78d602f1684a81b85af748fa55b1449dac by Victor Stinner (Christian Heimes) in branch 'master':
bpo-29176 Use tmpfile() in curses module (#235)
https://github.com/python/cpython/commit/2b221b78d602f1684a81b85af748fa55b1449dac

New changeset 61e2bc74dfab1ceee332d3f480dcf86c478c87c5 by Victor Stinner in branch 'master':
bpo-29176: Fix name of the _curses.window class (#52)
https://github.com/python/cpython/commit/61e2bc74dfab1ceee332d3f480dcf86c478c87c5

Chi Hsuan Yen confirmed that "python3.7m -m test test_curses -u curses" now pass on Android, so I close the issue. Thanks.