Skip to content

Commit bcc8fc4

Browse files
authored
Prevent closing of standard descriptors through file objects (#1725)
* Prevent closing of standard descriptors through file objects * Add test * Enable closing of standard descriptors directly * Add more tests
1 parent a1f06f9 commit bcc8fc4

4 files changed

Lines changed: 43 additions & 21 deletions

File tree

Src/DLR

Src/IronPython.Modules/nt.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,8 @@ public static void chmod(CodeContext context, object? path, int mode, [ParamDict
334334
public static void close(CodeContext/*!*/ context, int fd) {
335335
PythonFileManager fileManager = context.LanguageContext.FileManager;
336336
if (fileManager.TryGetFileFromId(fd, out PythonIOModule.FileIO? file)) {
337-
file.closefd = true;
338-
file.close(context);
337+
file.CloseStreams(fileManager);
338+
fileManager.RemoveObjectOnId(fd);
339339
} else {
340340
Stream stream = fileManager.GetStreamFromId(fd);
341341
fileManager.RemoveObjectOnId(fd);
@@ -358,7 +358,7 @@ public static int dup(CodeContext/*!*/ context, int fd) {
358358

359359
object obj = fileManager.GetObjectFromId(fd); // OSError if fd not valid
360360
if (obj is PythonIOModule.FileIO file) {
361-
var file2 = new PythonIOModule.FileIO(context, file.fileno(context));
361+
var file2 = new PythonIOModule.FileIO(context, file.fileno(context)) { closefd = false };
362362
int fd2 = fileManager.AddFile(file2);
363363
fileManager.EnsureRef(file._readStream);
364364
fileManager.AddRef(file2._readStream);
@@ -391,7 +391,7 @@ public static int dup2(CodeContext/*!*/ context, int fd, int fd2) {
391391
// TODO: race condition: `open` or `dup` on another thread may occupy fd2
392392

393393
if (obj is PythonIOModule.FileIO file) {
394-
var file2 = new PythonIOModule.FileIO(context, file.fileno(context));
394+
var file2 = new PythonIOModule.FileIO(context, file.fileno(context)) { closefd = false };
395395
fileManager.AddFile(fd2, file2);
396396
fileManager.EnsureRef(file._readStream);
397397
fileManager.AddRef(file2._readStream);
@@ -909,8 +909,8 @@ static Tuple<Stream, Stream> CreatePipeStreamsUnix() {
909909
public static PythonTuple pipe(CodeContext context) {
910910
var pipeStreams = CreatePipeStreams();
911911

912-
var inFile = new PythonIOModule.FileIO(context, pipeStreams.Item1);
913-
var outFile = new PythonIOModule.FileIO(context, pipeStreams.Item2);
912+
var inFile = new PythonIOModule.FileIO(context, pipeStreams.Item1) { closefd = false };
913+
var outFile = new PythonIOModule.FileIO(context, pipeStreams.Item2) { closefd = false };
914914

915915
return PythonTuple.MakeTuple(
916916
context.LanguageContext.FileManager.AddFile(inFile),

Src/IronPython/Modules/_fileio.cs

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ internal FileIO(CodeContext/*!*/ context, Stream stream, ConsoleStreamType conso
6464
: this(context, stream) {
6565
IsConsole = true;
6666
_consoleStreamType = consoleStreamType;
67+
_closefd= false;
6768
}
6869

6970
public FileIO(CodeContext/*!*/ context, int fd, string mode = "r", bool closefd = true, object opener = null)
@@ -92,7 +93,7 @@ public FileIO(CodeContext/*!*/ context, int fd, string mode = "r", bool closefd
9293
}
9394

9495
_fd = fd;
95-
_closefd = closefd;
96+
_closefd = closefd && !SharedIO.IsConsoleStream(_readStream);
9697
}
9798

9899
public FileIO(CodeContext/*!*/ context, string name, string mode = "r", bool closefd = true, object opener = null)
@@ -283,21 +284,25 @@ public override void close(CodeContext/*!*/ context) {
283284
myManager?.RemoveObjectOnId(_fd);
284285
}
285286

286-
bool readStreamClosed = true;
287-
if (_readStream is not null) {
288-
if (myManager is not null) {
289-
readStreamClosed = myManager.DerefAndCloseIfLast(_readStream);
290-
if (readStreamClosed) {
291-
myManager.Remove(_readStream);
292-
}
293-
} else {
294-
_readStream.Close();
287+
CloseStreams(myManager);
288+
}
289+
}
290+
291+
internal void CloseStreams(PythonFileManager manager) {
292+
bool readStreamClosed = true;
293+
if (_readStream is not null) {
294+
if (manager is not null) {
295+
readStreamClosed = manager.DerefAndCloseIfLast(_readStream);
296+
if (readStreamClosed) {
297+
manager.Remove(_readStream);
295298
}
299+
} else {
300+
_readStream.Close();
296301
}
297-
if (_writeStream is not null && !ReferenceEquals(_readStream, _writeStream) && readStreamClosed) {
298-
_writeStream.Close();
299-
myManager?.Remove(_writeStream);
300-
}
302+
}
303+
if (_writeStream is not null && !ReferenceEquals(_readStream, _writeStream) && readStreamClosed) {
304+
_writeStream.Close();
305+
manager?.Remove(_writeStream);
301306
}
302307
}
303308

Tests/modules/io_related/test_fd.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,4 +251,21 @@ def test_stat_chdir(self):
251251
os.close(fd)
252252
os.unlink(test_filename)
253253

254+
def test_stdio_fd(self):
255+
for file, fd, mode in [(sys.stdin, 0, 'r'), (sys.stdout, 1, 'w'), (sys.stderr, 2, 'w')]:
256+
with self.subTest(fd=fd):
257+
self.assertEqual(file.fileno(), fd)
258+
self.assertFalse(file.buffer.raw.closefd)
259+
with open(fd, mode=mode, closefd=True) as file2:
260+
# this fails in CPython if standard I/O is redirected
261+
self.assertFalse(file2.buffer.raw.closefd)
262+
with open(fd, mode=mode, closefd=True) as file3:
263+
self.assertFalse(file3.buffer.raw.closefd)
264+
265+
fd2 = os.dup(fd)
266+
self.assertNotEqual(fd2, fd)
267+
with open(fd2, mode=mode, closefd=True) as file4:
268+
self.assertFalse(file4.buffer.raw.closefd)
269+
os.close(fd2)
270+
254271
run_test(__name__)

0 commit comments

Comments
 (0)