From 389a844a65db673a13cf0c9a8c0f50a73bc999d3 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Tue, 17 Dec 2024 18:37:53 -0800 Subject: [PATCH] Fix file seek errors (#1842) --- Src/IronPython/Modules/_fileio.cs | 42 ++++++++++++------- Src/IronPython/Runtime/PythonFileManager.cs | 1 + Src/Scripts/generate_os_codes.py | 2 +- Tests/test_file.py | 45 +++++++++++++++++++++ 4 files changed, 75 insertions(+), 15 deletions(-) diff --git a/Src/IronPython/Modules/_fileio.cs b/Src/IronPython/Modules/_fileio.cs index 5e093c6fb..51b1b58e8 100644 --- a/Src/IronPython/Modules/_fileio.cs +++ b/Src/IronPython/Modules/_fileio.cs @@ -133,7 +133,7 @@ public FileIO(CodeContext/*!*/ context, string name, string mode = "r", bool clo // according to [documentation](https://learn.microsoft.com/en-us/dotnet/api/system.io.filestream.safefilehandle?view=net-9.0#remarks) // accessing SafeFileHandle sets the current stream position to 0 // in practice it doesn't seem to be the case, but better to be sure - if (this.mode == "ab+") { + if (this.mode.StartsWith("ab", StringComparison.InvariantCulture)) { _streams.WriteStream.Seek(0L, SeekOrigin.End); } if (!_streams.IsSingleStream) { @@ -372,26 +372,40 @@ public override BigInteger readinto(CodeContext/*!*/ context, object buf) { return readinto(bufferProtocol); } - [Documentation("seek(offset: int[, whence: int]) -> None. Move to new file position.\n\n" - + "Argument offset is a byte count. Optional argument whence defaults to\n" - + "0 (offset from start of file, offset should be >= 0); other values are 1\n" - + "(move relative to current position, positive or negative), and 2 (move\n" - + "relative to end of file, usually negative, although many platforms allow\n" - + "seeking beyond the end of a file).\n" - + "Note that not all file objects are seekable." - )] + + [Documentation(""" + seek(offset: int[, whence: int]) -> int. Change stream position. + + Argument offset is a byte count. Optional argument whence defaults to + 0 or `os.SEEK_SET` (offset from start of file, offset should be >= 0); + other values are 1 or `os.SEEK_CUR` (move relative to current position, + positive or negative), and 2 or `os.SEEK_END` (move relative to end of + file, usually negative, although many platforms allow seeking beyond + the end of a file, by adding zeros to enlarge the file). + + Return the new absolute position. + + Note that not all file objects are seekable. + """)] public override BigInteger seek(CodeContext/*!*/ context, BigInteger offset, [Optional] object whence) { _checkClosed(); - return _streams.ReadStream.Seek((long)offset, (SeekOrigin)GetInt(whence)); - } + var origin = (SeekOrigin)GetInt(whence); + if (origin < SeekOrigin.Begin || origin > SeekOrigin.End) + throw PythonOps.OSError(PythonFileManager.EINVAL, "Invalid argument"); - public BigInteger seek(double offset, [Optional] object whence) { - _checkClosed(); + long ofs = checked((long)offset); + if (ofs < 0 && ClrModule.IsMono && origin == SeekOrigin.Current) { + // Mono does not support negative offsets with SeekOrigin.Current + // so we need to calculate the absolute offset + ofs += _streams.ReadStream.Position; + origin = SeekOrigin.Begin; + } - throw PythonOps.TypeError("an integer is required"); + return _streams.ReadStream.Seek(ofs, origin); } + [Documentation("seekable() -> bool. True if file supports random-access.")] public override bool seekable(CodeContext/*!*/ context) { _checkClosed(); diff --git a/Src/IronPython/Runtime/PythonFileManager.cs b/Src/IronPython/Runtime/PythonFileManager.cs index 03ee1d9b3..ca4dbef44 100644 --- a/Src/IronPython/Runtime/PythonFileManager.cs +++ b/Src/IronPython/Runtime/PythonFileManager.cs @@ -222,6 +222,7 @@ internal class PythonFileManager { internal const int ENOENT = 2; internal const int EBADF = 9; internal const int EACCES = 13; + internal const int EINVAL = 22; internal const int EMFILE = 24; // *** END GENERATED CODE *** diff --git a/Src/Scripts/generate_os_codes.py b/Src/Scripts/generate_os_codes.py index 912923bd6..b5b226581 100644 --- a/Src/Scripts/generate_os_codes.py +++ b/Src/Scripts/generate_os_codes.py @@ -102,7 +102,7 @@ def darwin_code_expr(codes, fmt): def linux_code_expr(codes, fmt): return fmt(codes[linux_idx]) -common_errno_codes = ['ENOENT', 'EBADF', 'EACCES', 'EMFILE'] +common_errno_codes = ['ENOENT', 'EBADF', 'EACCES', 'EINVAL', 'EMFILE'] def generate_common_errno_codes(cw): for name in common_errno_codes: diff --git a/Tests/test_file.py b/Tests/test_file.py index 97cc97e36..c40936dfc 100644 --- a/Tests/test_file.py +++ b/Tests/test_file.py @@ -783,6 +783,51 @@ def test_opener_uncallable(self): self.assertRaises(TypeError, open, "", "r", opener=uncallable_opener) + + def test_seek(self): + with open(self.temp_file, "w") as f: + f.write("abc") + self.assertRaises(TypeError, f.buffer.seek, 1.5) + if is_cli: + self.assertRaises(TypeError, f.seek, 1.5) # surprisingly, this doesn't raise an error in CPython + + with open(self.temp_file, "rb") as f: + self.assertEqual(f.tell(), 0) + self.assertEqual(f.read(1), b"a") + self.assertEqual(f.tell(), 1) + + f.seek(2) + self.assertEqual(f.tell(), 2) + self.assertEqual(f.read(1), b"c") + self.assertEqual(f.tell(), 3) + + f.seek(0, os.SEEK_SET) + self.assertEqual(f.tell(), 0) + self.assertEqual(f.read(1), b"a") + self.assertEqual(f.tell(), 1) + + f.seek(0, os.SEEK_CUR) + self.assertEqual(f.tell(), 1) + self.assertEqual(f.read(1), b"b") + self.assertEqual(f.tell(), 2) + + f.raw.seek(2, os.SEEK_SET) + f.raw.seek(-2, os.SEEK_CUR) + self.assertEqual(f.raw.tell(), 0) + self.assertEqual(f.raw.read(1), b"a") + self.assertEqual(f.raw.tell(), 1) + + f.raw.seek(-1, os.SEEK_END) + self.assertEqual(f.raw.tell(), 2) + self.assertEqual(f.raw.read(1), b"c") + self.assertEqual(f.raw.tell(), 3) + + self.assertRaises(TypeError, f.seek, 1.5) + self.assertRaises(TypeError, f.raw.seek, 1.5) + self.assertRaises(OSError, f.raw.seek, -1) + self.assertRaises(OSError, f.raw.seek, 0, -1) + + def test_open_wbplus(self): with open(self.temp_file, "wb+") as f: f.write(b"abc")