bpo-37367: octal escapes applied inconsistently throughout the interpreter and lib #14654
Conversation
values greater than 255 (0o377)
Misc/NEWS.d/next/Core and Builtins/2019-07-08-19-08-38.bpo-37367.SVtBhh.rst
Outdated
Show resolved
Hide resolved
1. use "octal escape sequences" instead of "octal values" in comments 2. remove underscore from _literal_eval Also, use "0o1000-0o400" instead of "256" to make it clear to the reader where the value came from.
| @@ -0,0 +1,2 @@ | |||
| Byte strings containing octal escape sequences with values greater than 255 | |||
| will now raise ValueError. Patch by Jeffrey Kintscher. | |||
There was a problem hiding this comment.
It would be helpful to mention the previous behavior here.
| @@ -14,6 +14,7 @@ | |||
| import tempfile | |||
| import textwrap | |||
| import unittest | |||
| import ast | |||
There was a problem hiding this comment.
Please keep the imports sorted alphabetically.
|
@websurfer5, could you merge the master branch into this PR's branch, resolving the merge conflicts? |
| @@ -962,6 +963,45 @@ def test_translate(self): | |||
| c = b.translate(None, delete=b'e') | |||
| self.assertEqual(c, b'hllo') | |||
|
|
|||
| def test_octal_values(self): | |||
There was a problem hiding this comment.
It is not correct place for such test. This file is for tests of bytes and bytearray methods, not parsing Python code.
test_string_literals is for parsing string and bytes literals. Add some tests there.
| @@ -1174,7 +1174,7 @@ def test_escape(self): | |||
| check(br"[\418]", b"[!8]") | |||
| check(br"[\101]", b"[A]") | |||
| check(br"[\1010]", b"[A0]") | |||
| check(br"[\501]", b"[A]") | |||
| self.assertRaises(ValueError, decode, br"[\501]") | |||
There was a problem hiding this comment.
Move it to test_errors below.
| @@ -1185,6 +1185,11 @@ PyObject *_PyBytes_DecodeEscape(const char *s, | |||
| if (s < end && '0' <= *s && *s <= '7') | |||
| c = (c<<3) + *s++ - '0'; | |||
| } | |||
| if (c > 255) { | |||
| PyErr_Format(PyExc_ValueError, | |||
| "octal value must be in range(0, 256)"); | |||
There was a problem hiding this comment.
The error message should contain the position of the invalid octal escape sequence. It would be nice if look similar to error message for invalid hexadecimal escape sequence. For example:
"invalid octal escape sequence at position %zd"
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
#91668 was merged instead. |
Add a range check for octal values in byte strings that raises ValueError if the value is more than 255 (0o377).
Tests for all 1-, 2-, and 3-digit octal values are provided (both valid and error cases).
https://bugs.python.org/issue37367