From 19ed7f12d789d7cd677ed33fd027b9764de2dfe8 Mon Sep 17 00:00:00 2001 From: AngstyDuck Date: Sun, 12 Sep 2021 22:54:35 +0800 Subject: [PATCH 1/7] bpo-44687: Fixed unintended exception raised when file is closed when peeking --- Modules/_io/bufferedio.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 5984d34cc08290..5167caee0df582 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -346,6 +346,14 @@ _enter_buffered_busy(buffered *self) return NULL; \ } +#define CHECK_CLOSED_PEEK(self, error_msg) \ + Py_ssize_t have; \ + have = Py_SAFE_DOWNCAST(READAHEAD(self), Py_off_t, Py_ssize_t); \ + if (IS_CLOSED(self) & (have == 0)) { \ + PyErr_SetString(PyExc_ValueError, error_msg); \ + return NULL; \ + } + #define VALID_READ_BUFFER(self) \ (self->readable && self->read_end != -1) @@ -842,7 +850,7 @@ _io__Buffered_peek_impl(buffered *self, Py_ssize_t size) PyObject *res = NULL; CHECK_INITIALIZED(self) - CHECK_CLOSED(self, "peek of closed file") + CHECK_CLOSED_PEEK(self, "peek of closed file") if (!ENTER_BUFFERED(self)) return NULL; From 7cca3c595a2c6a71c7e1d5420fdd9755a79bd3fc Mon Sep 17 00:00:00 2001 From: AngstyDuck Date: Mon, 20 Sep 2021 00:49:03 +0800 Subject: [PATCH 2/7] bpo-44687: fixed failing test where the closed file check unintentionally passes when the file was previously read --- Modules/_io/bufferedio.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 5167caee0df582..8c89420f6649b0 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -346,14 +346,11 @@ _enter_buffered_busy(buffered *self) return NULL; \ } -#define CHECK_CLOSED_PEEK(self, error_msg) \ - Py_ssize_t have; \ - have = Py_SAFE_DOWNCAST(READAHEAD(self), Py_off_t, Py_ssize_t); \ - if (IS_CLOSED(self) & (have == 0)) { \ +#define PEEK_CHECK_CLOSED(self, error_msg) \ + if (IS_CLOSED(self) & (Py_SAFE_DOWNCAST(READAHEAD(self), Py_off_t, Py_ssize_t) == 0)) { \ PyErr_SetString(PyExc_ValueError, error_msg); \ return NULL; \ - } - + } \ #define VALID_READ_BUFFER(self) \ (self->readable && self->read_end != -1) @@ -850,7 +847,13 @@ _io__Buffered_peek_impl(buffered *self, Py_ssize_t size) PyObject *res = NULL; CHECK_INITIALIZED(self) - CHECK_CLOSED_PEEK(self, "peek of closed file") + + // use a check specific to peek if the previous operation was not a read + if (Py_SAFE_DOWNCAST(self->pos, Py_off_t, Py_ssize_t) == 0) { + PEEK_CHECK_CLOSED(self, "peek of closed file") + } else { + CHECK_CLOSED(self, "peek of closed file") + } if (!ENTER_BUFFERED(self)) return NULL; From 455b0d56b88c5672e5d02ff2db8b3b07cc4d7b7b Mon Sep 17 00:00:00 2001 From: AngstyDuck Date: Mon, 20 Sep 2021 00:54:59 +0800 Subject: [PATCH 3/7] bpo-44687: fixed typos --- Modules/_io/bufferedio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 8c89420f6649b0..ca4b491f045aa5 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -848,7 +848,7 @@ _io__Buffered_peek_impl(buffered *self, Py_ssize_t size) CHECK_INITIALIZED(self) - // use a check specific to peek if the previous operation was not a read + // use a check specific to peek if the previous file operation was not a read if (Py_SAFE_DOWNCAST(self->pos, Py_off_t, Py_ssize_t) == 0) { PEEK_CHECK_CLOSED(self, "peek of closed file") } else { From ef597e4b58b3bfaa2066a1edb6938aa7f1b17932 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 19 Sep 2021 17:18:26 +0000 Subject: [PATCH 4/7] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst diff --git a/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst b/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst new file mode 100644 index 00000000000000..4ee7fce37c8bda --- /dev/null +++ b/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst @@ -0,0 +1 @@ +Add a new :c:macro:`PEEK_CHECK_CLOSED` macro to check if a file is closed using a method specific to BufferedReader.peek() \ No newline at end of file From 6b2cce445b9864376f0378b5f1dd0fba709e5046 Mon Sep 17 00:00:00 2001 From: AngstyDuck Date: Fri, 24 Sep 2021 18:37:10 +0800 Subject: [PATCH 5/7] bpo-44687: replaced old closed file check with new check completely, new check now works regardless of which method was called before peek() --- Modules/_io/bufferedio.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index ca4b491f045aa5..ba966f568b399a 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -341,12 +341,6 @@ _enter_buffered_busy(buffered *self) : buffered_closed(self))) #define CHECK_CLOSED(self, error_msg) \ - if (IS_CLOSED(self)) { \ - PyErr_SetString(PyExc_ValueError, error_msg); \ - return NULL; \ - } - -#define PEEK_CHECK_CLOSED(self, error_msg) \ if (IS_CLOSED(self) & (Py_SAFE_DOWNCAST(READAHEAD(self), Py_off_t, Py_ssize_t) == 0)) { \ PyErr_SetString(PyExc_ValueError, error_msg); \ return NULL; \ @@ -535,6 +529,9 @@ buffered_close(buffered *self, PyObject *args) Py_CLEAR(res); } + self->read_end = 0; + self->pos = 0; + end: LEAVE_BUFFERED(self) return res; @@ -847,13 +844,7 @@ _io__Buffered_peek_impl(buffered *self, Py_ssize_t size) PyObject *res = NULL; CHECK_INITIALIZED(self) - - // use a check specific to peek if the previous file operation was not a read - if (Py_SAFE_DOWNCAST(self->pos, Py_off_t, Py_ssize_t) == 0) { - PEEK_CHECK_CLOSED(self, "peek of closed file") - } else { - CHECK_CLOSED(self, "peek of closed file") - } + CHECK_CLOSED(self, "peek of closed file") if (!ENTER_BUFFERED(self)) return NULL; From 7121431cac1fd553238f02c2c66a1b74493bf2c9 Mon Sep 17 00:00:00 2001 From: AngstyDuck Date: Fri, 24 Sep 2021 18:42:48 +0800 Subject: [PATCH 6/7] bpo-44687: updated NEWS --- Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst b/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst index 4ee7fce37c8bda..a35a5f84a2e6f6 100644 --- a/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst +++ b/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst @@ -1 +1 @@ -Add a new :c:macro:`PEEK_CHECK_CLOSED` macro to check if a file is closed using a method specific to BufferedReader.peek() \ No newline at end of file +Added additional checks to :c:macro:`CHECK_CLOSED` macro to fix unintended closing file error when peeking a BufferedReader object From ae74730b2590c422ae5f70bd26a58411d25c5621 Mon Sep 17 00:00:00 2001 From: AngstyDuck Date: Sat, 25 Sep 2021 13:05:21 +0800 Subject: [PATCH 7/7] bpo-44687: Update Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst Co-authored-by: Steve Dower --- Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst b/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst index a35a5f84a2e6f6..d38fa6057f6f99 100644 --- a/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst +++ b/Misc/NEWS.d/next/C API/2021-09-19-17-18-25.bpo-44687.3fqDRC.rst @@ -1 +1 @@ -Added additional checks to :c:macro:`CHECK_CLOSED` macro to fix unintended closing file error when peeking a BufferedReader object +:meth:`BufferedReader.peek` no longer raises :exc:`ValueError` when the entire file has already been buffered.