Complete O_flags used for file open (#1853)

* Complete O_flags used for file open

* Update after review

Author: BCSharp

Src/IronPython/Modules/_fileio.cs

@@ -152,6 +152,11 @@ public FileIO(CodeContext/*!*/ context, string name, string mode = "r", bool clo
                         }
 
                         if (!_context.FileManager.TryGetStreams(fd, out _streams)) {
+                            // TODO: This is not necessarily an error on Posix.
+                            // The descriptor could have been opened by a different means than os.open.
+                            // In such case: 
+                            // _streams = new(new UnixStream(fd, ownsHandle: true))
+                            // _context.FileManager.Add(fd, _streams);
                             throw PythonOps.OSError(PythonFileManager.EBADF, "Bad file descriptor");
                         }
                     } else {
@@ -163,34 +168,40 @@ public FileIO(CodeContext/*!*/ context, string name, string mode = "r", bool clo
             }
 
             private static string NormalizeMode(string mode, out int flags) {
+                flags = 0;
+                if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {
+                    flags |= O_NOINHERIT | O_BINARY;
+                } else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
+                    flags |= O_CLOEXEC;
+                }
                 switch (StandardizeMode(mode)) {
                     case "r":
-                        flags = O_RDONLY;
+                        flags |= O_RDONLY;
                         return "rb";
                     case "w":
-                        flags = O_CREAT | O_TRUNC | O_WRONLY;
+                        flags |= O_CREAT | O_TRUNC | O_WRONLY;
                         return "wb";
                     case "a":
-                        flags = O_APPEND | O_CREAT;
+                        flags |= O_APPEND | O_CREAT | O_WRONLY;
                         return "ab";
                     case "x":
-                        flags = O_CREAT | O_EXCL;
+                        flags |= O_CREAT | O_EXCL | O_WRONLY;
                         return "xb";
                     case "r+":
                     case "+r":
-                        flags = O_RDWR;
+                        flags |= O_RDWR;
                         return "rb+";
                     case "w+":
                     case "+w":
-                        flags = O_CREAT | O_TRUNC | O_RDWR;
+                        flags |= O_CREAT | O_TRUNC | O_RDWR;
                         return "wb+";
                     case "a+":
                     case "+a":
-                        flags = O_APPEND | O_CREAT | O_RDWR;
+                        flags |= O_APPEND | O_CREAT | O_RDWR;
                         return "ab+";
                     case "x+":
                     case "+x":
-                        flags = O_CREAT | O_RDWR | O_EXCL;
+                        flags |= O_CREAT | O_EXCL | O_RDWR;
                         return "xb+";
                     default:
                         throw BadMode(mode);
@@ -225,14 +236,14 @@ static Exception BadMode(string mode) {
                             case 'a':
                             case 'x':
                                 if (foundMode) {
-                                    return PythonOps.ValueError("Must have exactly one of create/read/write/append mode and at most one plus");
+                                    return BadModeException();
                                 } else {
                                     foundMode = true;
                                     continue;
                                 }
                             case '+':
                                 if (foundPlus) {
-                                    return PythonOps.ValueError("Must have exactly one of create/read/write/append mode and at most one plus");
+                                    return BadModeException();
                                 } else {
                                     foundPlus = true;
                                     continue;
@@ -245,7 +256,9 @@ static Exception BadMode(string mode) {
                         }
                     }
 
-                    return PythonOps.ValueError("Must have exactly one of create/read/write/append mode and at most one plus");
+                    return BadModeException();
+
+                    static Exception BadModeException() => PythonOps.ValueError("Must have exactly one of create/read/write/append mode and at most one plus");
                 }
             }
 

Src/IronPython/Modules/_io.cs

@@ -1,4 +1,4 @@
-// Licensed to the .NET Foundation under one or more agreements.
+// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the Apache 2.0 License.
 // See the LICENSE file in the project root for more information.
 
@@ -12,6 +12,7 @@
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using System.Runtime.Versioning;
 using System.Text;
 
 using Microsoft.Scripting.Runtime;
@@ -54,6 +55,16 @@ public static partial class PythonIOModule {
 
         private static int O_EXCL => RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 0x400 : RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? 0x800 : 0x80;
 
+        [SupportedOSPlatform("linux")]
+        [SupportedOSPlatform("macos")]
+        private static int O_CLOEXEC => RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? 0x1000000 : 0x80000;
+
+        [SupportedOSPlatform("windows")]
+        private static int O_BINARY => 0x8000;
+
+        [SupportedOSPlatform("windows")]
+        private static int O_NOINHERIT => 0x80;
+
         // *** END GENERATED CODE ***
 
         #endregion

Src/Scripts/generate_os_codes.py

@@ -199,7 +199,7 @@ def generate_O_flags(cw, flagvalues, access):
             if codes[darwin_idx] is None:
                 hidden_on += ["PlatformID.MacOSX"]
                 supported_on.discard(systems[darwin_idx])
-        if hidden_on:
+        if hidden_on and (access == 'public' or access == 'protected' or access == 'protected internal'):
             cw.write(f"[PythonHidden({', '.join(hidden_on)})]")
         if len(supported_on) != len(systems):
             for s in sorted(supported_on):
@@ -211,7 +211,7 @@ def generate_O_flags(cw, flagvalues, access):
 def generate_all_O_flags(cw):
     generate_O_flags(cw, O_flagvalues, 'public')
 
-common_O_flags = ['O_RDONLY', 'O_WRONLY', 'O_RDWR', 'O_APPEND', 'O_CREAT', 'O_TRUNC', 'O_EXCL']
+common_O_flags = ['O_RDONLY', 'O_WRONLY', 'O_RDWR', 'O_APPEND', 'O_CREAT', 'O_TRUNC', 'O_EXCL', 'O_CLOEXEC', 'O_BINARY', 'O_NOINHERIT']
 
 def generate_common_O_flags(cw):
     generate_O_flags(cw, OrderedDict((f, O_flagvalues[f]) for f in common_O_flags), 'private')

Tests/test_file.py

@@ -2,14 +2,15 @@
 # The .NET Foundation licenses this file to you under the Apache 2.0 License.
 # See the LICENSE file in the project root for more information.
 
+import errno
 import os
 import sys
 import unittest
 import _thread
 
 CP16623_LOCK = _thread.allocate_lock()
 
-from iptest import IronPythonTestCase, is_cli, is_cpython, is_netcoreapp, is_posix, run_test, skipUnlessIronPython
+from iptest import IronPythonTestCase, is_cli, is_cpython, is_netcoreapp, is_posix, is_windows, run_test, skipUnlessIronPython
 
 class FileTest(IronPythonTestCase):
 
@@ -505,12 +506,12 @@ def test_file_manager_leak(self):
         # the number of iterations should be larger than Microsoft.Scripting.Utils.HybridMapping.SIZE (currently 4K)
         N = 5000
         for i in range(N):
-            fd = os.open(self.temp_file, os.O_WRONLY | os.O_CREAT)
+            fd = os.open(self.temp_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC)
             f = os.fdopen(fd, 'w', closefd=True)
             f.close()
 
         for i in range(N):
-            fd = os.open(self.temp_file, os.O_WRONLY | os.O_CREAT)
+            fd = os.open(self.temp_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC)
             f = os.fdopen(fd, 'w', closefd=False)
             g = os.fdopen(f.fileno(), 'w', closefd=True)
             g.close()
@@ -624,7 +625,7 @@ def test_modes(self):
         self.assertRaisesMessage(ValueError, "invalid mode: 'p'", open, 'abc', 'p')
 
         # allow anything w/ U but r and w
-        err_msg = "mode U cannot be combined with 'x', 'w', 'a', or '+'" if is_cli or sys.version_info >= (3,7) else "mode U cannot be combined with x', 'w', 'a', or '+'" if sys.version_info >= (3,6) else "can't use U and writing mode at once"
+        err_msg = "mode U cannot be combined with 'x', 'w', 'a', or '+'" if is_cli or sys.version_info >= (3,7,4) else "mode U cannot be combined with x', 'w', 'a', or '+'" if sys.version_info >= (3,6) else "can't use U and writing mode at once"
         self.assertRaisesMessage(ValueError, err_msg, open, 'abc', 'Uw')
         self.assertRaisesMessage(ValueError, err_msg, open, 'abc', 'Ua')
         self.assertRaisesMessage(ValueError, err_msg, open, 'abc', 'Uw+')
@@ -701,7 +702,7 @@ def test_errors(self):
 
         with self.assertRaises(OSError) as cm:
             open('path_too_long' * 100)
-        self.assertEqual(cm.exception.errno, (36 if is_posix else 22) if is_netcoreapp and not is_posix or sys.version_info >= (3,6) else 2)
+        self.assertEqual(cm.exception.errno, (errno.ENAMETOOLONG if is_posix else errno.EINVAL) if is_netcoreapp and not is_posix or sys.version_info >= (3,6) else errno.ENOENT)
 
     def test_write_bytes(self):
         fname = self.temp_file
@@ -756,6 +757,39 @@ def test_open_with_BOM(self):
         with open(fileName, "rb") as f:
             self.assertEqual(f.read(), b"\xef\xbb\xbf\x42\xc3\x93\x4d\x0d\x0a")
 
+
+    def test_open_flags(self):
+        test_data = {
+            'rb': os.O_RDONLY,
+            'wb': os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
+            'ab': os.O_WRONLY | os.O_CREAT | os.O_APPEND,
+            'xb': os.O_WRONLY | os.O_CREAT | os.O_EXCL,
+            'rb+': os.O_RDWR,
+            'wb+': os.O_RDWR | os.O_CREAT | os.O_TRUNC,
+            'ab+': os.O_RDWR | os.O_CREAT | os.O_APPEND,
+            'xb+': os.O_RDWR | os.O_CREAT | os.O_EXCL,
+        }
+        extra_flags = 0
+        if is_posix:
+            extra_flags |= os.O_CLOEXEC
+        elif is_windows:
+            extra_flags |= os.O_NOINHERIT | os.O_BINARY
+        test_data = {k: v | extra_flags for k, v in test_data.items()}
+
+        flags_received = None
+        def test_open(name, flags):
+            nonlocal flags_received
+            flags_received = flags
+            if mode[0] == 'x':
+                os.unlink(name)
+            return os.open(name, flags)
+
+        for mode in sorted(test_data):
+            with self.subTest(mode=mode):
+                with open(self.temp_file, mode, opener=test_open): pass
+                self.assertEqual(flags_received, test_data[mode])
+
+
     def test_opener(self):
         data = "test message\n"
         with open(self.temp_file, "w", opener=os.open) as f: