Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Lib/json/decoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def py_scanstring(s, end, strict=True,
if strict:
#msg = "Invalid control character %r at" % (terminator,)
msg = "Invalid control character {0!r} at".format(terminator)
raise JSONDecodeError(msg, s, end)
raise JSONDecodeError(msg, s, end - 1)
else:
_append(terminator)
continue
Expand Down
42 changes: 42 additions & 0 deletions Lib/test/pickletester.py
Original file line number Diff line number Diff line change
Expand Up @@ -1246,6 +1246,48 @@ def test_frame_readline(self):
# 15: . STOP
self.assertEqual(self.loads(pickled), 42)

def test_frame_ends_at_opcode_boundary(self):
# A frame may end exactly between two opcodes; the following opcodes
# are then read from outside the frame.
for pickled in [
b'\x80\x04\x95\x01\x00\x00\x00\x00\x00\x00\x00N.', # FRAME 1, NONE, STOP
b'\x80\x04\x95\x00\x00\x00\x00\x00\x00\x00\x00N.', # empty FRAME, NONE, STOP
]:
with self.subTest(pickled=pickled):
self.assertIsNone(self.loads(pickled))

def test_frame_does_not_straddle_boundary(self):
# An opcode or its argument must not cross a frame boundary
# (PEP 3154). Such a pickle must be rejected rather than silently
# reading past the declared frame length, which would make the
# meaning of the pickle diverge from its pickletools disassembly.
# See gh-154848.
for pickled in [
# FRAME 6; UNICODE argument read by readline() straddles the frame.
b'\x80\x04\x95\x06\x00\x00\x00\x00\x00\x00\x00Vhelloworld\n.',
# FRAME 6; BINUNICODE argument straddles the frame.
b'\x80\x04\x95\x06\x00\x00\x00\x00\x00\x00\x00'
b'X\x0a\x00\x00\x00helloworld.',
# FRAME 3; SHORT_BINBYTES argument straddles the frame.
b'\x80\x04\x95\x03\x00\x00\x00\x00\x00\x00\x00C\x0ahelloworld.',
# FRAME 9; GLOBAL argument (second line) straddles the frame.
b'\x80\x04\x95\x09\x00\x00\x00\x00\x00\x00\x00cbuiltins\nprint\n.',
]:
self.check_unpickling_error(self.truncated_errors, pickled)

def test_nested_frame(self):
# A new frame must not begin before the current one has ended: here
# the outer frame still has data left after the inner frame header.
pickled = (b'\x80\x04\x95\x0c\x00\x00\x00\x00\x00\x00\x00'
b'N\x95\x00\x00\x00\x00\x00\x00\x00\x00NN.')
self.check_unpickling_error(self.truncated_errors, pickled)

# But the inner frame header may lie inside the outer frame as long as
# it exactly consumes it.
pickled = (b'\x80\x04\x95\x0a\x00\x00\x00\x00\x00\x00\x00'
b'N\x95\x00\x00\x00\x00\x00\x00\x00\x00N.')
self.assertIsNone(self.loads(pickled))

def test_compat_unpickle(self):
# xrange(1, 7)
pickled = b'\x80\x02c__builtin__\nxrange\nK\x01K\x07K\x01\x87R.'
Expand Down
31 changes: 31 additions & 0 deletions Lib/test/test_codecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -3679,6 +3679,10 @@ def iconv_encoding_available(name):
# Encodings iconv may provide but for which CPython has no built-in codec
# (cp1047 is EBCDIC, i.e. not ASCII-compatible).
_ICONV_ONLY = ['cp1047', 'cp1133', 'GEORGIAN-PS', 'ARMSCII-8']
# Encodings that leave a shift state pending, so encoding ends with a flush.
_ICONV_SHIFT_STATE = [('ISO-2022-CN-EXT', 'ABC\u4e2dDEF'),
('ISO-2022-CN', 'ABC\u4e2dDEF'),
('ISO-2022-JP', 'ABC\u65e5DEF')]


@unittest.skipUnless(hasattr(codecs, 'iconv_encode'),
Expand Down Expand Up @@ -3812,6 +3816,33 @@ def test_encode_kinds(self):
with self.subTest(text=text):
self.assertEqual(text.encode('iconv:' + enc), text.encode(enc))

def test_encode_shift_state_flush(self):
# Encoding ends with a flush that emits the pending shift sequence. Its
# return value counts nonreversible conversions, and some iconv
# implementations make it positive for the flush itself (glibc does for
# ISO-2022-CN-EXT). That must not be read as a substituted character:
# doing so discarded the whole output, ASCII included.
#
# Only the ASCII around the character is checked, not a full round-trip.
# An iconv that cannot represent the character either rejects it or
# substitutes for it silently, as macOS does for ISO-2022-CN.
tested = False
for enc, text in _ICONV_SHIFT_STATE:
if not iconv_encoding_available(enc):
continue
with self.subTest(encoding=enc):
try:
data = codecs.iconv_encode(enc, text)[0]
except UnicodeEncodeError:
continue
tested = True
self.assertNotEqual(data, b'')
decoded = codecs.iconv_decode(enc, data, 'strict', True)[0]
self.assertStartsWith(decoded, 'ABC')
self.assertEndsWith(decoded, 'DEF')
if not tested:
self.skipTest('no shift-state iconv encoding is available')

def test_encode_surrogateescape(self):
# A lone surrogate lives in the 2-byte kind and round-trips.
enc = self.require('ASCII')
Expand Down
8 changes: 7 additions & 1 deletion Lib/test/test_json/test_unicode.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ def test_ascii_non_printable_decode(self):
'\b\t\n\f\r')
s = ''.join(map(chr, range(32)))
for c in s:
self.assertRaises(self.JSONDecodeError, self.loads, f'"{c}"')
with self.subTest(control_character=ord(c)):
with self.assertRaises(self.JSONDecodeError) as cm:
self.loads(f'"a{c}b"')
error = cm.exception
self.assertEqual(error.pos, 2)
self.assertEqual(error.lineno, 1)
self.assertEqual(error.colno, 3)
self.assertEqual(self.loads(f'"{s}"', strict=False), s)
self.assertEqual(self.loads('"\x7f"'), '\x7f')

Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_pickle.py
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ def test_pickler(self):
0) # Write buffer is cleared after every dump().

def test_unpickler(self):
basesize = support.calcobjsize('2P2n3P 2P2n2i5P 2P3n8P2n3i')
basesize = support.calcobjsize('2P2n3P 2P2n2i5P 2P5n8P2n3i')
unpickler = _pickle.Unpickler
P = struct.calcsize('P') # Size of memo table entry.
n = struct.calcsize('n') # Size of mark table entry.
Expand Down
7 changes: 7 additions & 0 deletions Lib/test/test_pyexpat.py
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,13 @@ def test_parent_parser_outlives_its_subparsers__chain(self):
del parser
del subparser

def test_subparser_inherits_reparse_deferral(self):
for enabled in (True, False):
parser = expat.ParserCreate()
parser.SetReparseDeferralEnabled(enabled)
subparser = parser.ExternalEntityParserCreate(None)
self.assertEqual(subparser.GetReparseDeferralEnabled(), enabled)


class ExternalEntityParserCreateErrorTest(unittest.TestCase):
"""ExternalEntityParserCreate error paths should not crash or leak
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix :meth:`!ExternalEntityParserCreate` not propagating the reparse-deferral
setting to the subparser, which left :meth:`!GetReparseDeferralEnabled`
returning an uninitialized value. Patch by tonghuaroot.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
The :mod:`pickle` C accelerator now enforces frame boundaries when
unpickling, as the pure Python implementation already did. An argument that
straddles a frame boundary, or a frame that begins before the previous one has
ended, now raises :exc:`pickle.UnpicklingError` instead of
being silently read across the boundary. This prevents the loaded data from
diverging from the :mod:`pickletools` disassembly of the same pickle.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix the pure Python :mod:`json` decoder to report the correct position for
invalid literal control characters in JSON strings.
89 changes: 89 additions & 0 deletions Modules/_pickle.c
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,10 @@ typedef struct UnpicklerObject {
Py_ssize_t input_len;
Py_ssize_t next_read_idx;
Py_ssize_t prefetched_idx; /* index of first prefetched byte */
Py_ssize_t frame_end; /* End of the current frame, or -1 if not in a
frame. While in a frame, input_len is capped
here so reads can't cross it (PEP 3154). */
Py_ssize_t saved_input_len; /* input_len to restore when the frame ends. */

PyObject *read; /* read() method of the input stream. */
PyObject *readinto; /* readinto() method of the input stream. */
Expand Down Expand Up @@ -1250,6 +1254,7 @@ _Unpickler_SetStringInput(UnpicklerObject *self, PyObject *input)
self->input_len = self->buffer.len;
self->next_read_idx = 0;
self->prefetched_idx = self->input_len;
self->frame_end = -1;
return self->input_len;
}

Expand All @@ -1260,6 +1265,30 @@ bad_readline(PickleState *st)
return -1;
}

/* End the current frame, uncapping input_len. The frame must be fully read. */
static void
_Unpickler_EndFrame(UnpicklerObject *self)
{
assert(self->frame_end >= 0);
assert(self->next_read_idx == self->frame_end);
self->input_len = self->saved_input_len;
self->frame_end = -1;
}

/* Reached the end of the current frame. If straddle, the read crosses the
frame boundary (PEP 3154) and fails; otherwise end the frame. */
static int
_Unpickler_LeaveFrame(PickleState *st, UnpicklerObject *self, int straddle)
{
if (straddle) {
PyErr_SetString(st->UnpicklingError,
"pickle exhausted before end of frame");
return -1;
}
_Unpickler_EndFrame(self);
return 0;
}

/* Skip any consumed data that was only prefetched using peek() */
static int
_Unpickler_SkipConsumed(UnpicklerObject *self)
Expand Down Expand Up @@ -1444,6 +1473,19 @@ _Unpickler_ReadImpl(UnpicklerObject *self, PickleState *st, char **s, Py_ssize_t
return -1;
}

if (self->frame_end >= 0) {
if (_Unpickler_LeaveFrame(st, self,
self->next_read_idx < self->frame_end) < 0) {
return -1;
}
/* Frame ended; the read may now be satisfied from the buffer. */
if (n <= self->input_len - self->next_read_idx) {
*s = self->input_buffer + self->next_read_idx;
self->next_read_idx += n;
return n;
}
}

/* This case is handled by the _Unpickler_Read() macro for efficiency */
assert(self->next_read_idx + n > self->input_len);

Expand Down Expand Up @@ -1489,6 +1531,25 @@ _Unpickler_ReadInto(PickleState *state, UnpicklerObject *self, char *buf,
}
}

if (self->frame_end >= 0) {
/* in_buffer > 0 is next_read_idx < frame_end on entry: frame data was
consumed above, so the read crosses the frame boundary. */
if (_Unpickler_LeaveFrame(state, self, in_buffer > 0) < 0) {
return -1;
}
in_buffer = self->input_len - self->next_read_idx;
if (in_buffer > 0) {
Py_ssize_t to_read = Py_MIN(in_buffer, n);
memcpy(buf, self->input_buffer + self->next_read_idx, to_read);
self->next_read_idx += to_read;
buf += to_read;
n -= to_read;
if (n == 0) {
return n;
}
}
}

/* Read from file */
if (!self->read) {
/* We're unpickling memory, this means the input is truncated */
Expand Down Expand Up @@ -1547,6 +1608,7 @@ _Unpickler_Readline(PickleState *state, UnpicklerObject *self, char **result)
{
Py_ssize_t i, num_read;

rescan:
for (i = self->next_read_idx; i < self->input_len; i++) {
if (self->input_buffer[i] == '\n') {
char *line_start = self->input_buffer + self->next_read_idx;
Expand All @@ -1555,6 +1617,14 @@ _Unpickler_Readline(PickleState *state, UnpicklerObject *self, char **result)
return _Unpickler_CopyLine(self, line_start, num_read, result);
}
}
if (self->frame_end >= 0) {
if (_Unpickler_LeaveFrame(state, self,
self->next_read_idx < self->frame_end) < 0) {
return -1;
}
/* Frame ended; continue the line past its end. */
goto rescan;
}
if (!self->read)
return bad_readline(state);

Expand Down Expand Up @@ -1729,6 +1799,8 @@ _Unpickler_New(PyObject *module)
self->input_len = 0;
self->next_read_idx = 0;
self->prefetched_idx = 0;
self->frame_end = -1;
self->saved_input_len = 0;
self->read = NULL;
self->readinto = NULL;
self->readline = NULL;
Expand Down Expand Up @@ -7078,6 +7150,17 @@ load_frame(PickleState *state, UnpicklerObject *self)
if (_Unpickler_Read(self, state, &s, 8) < 0)
return -1;

/* A new frame must not begin before the current one ends (PEP 3154). Its
header may lie at the tail of the current frame if it drains it. */
if (self->frame_end >= 0) {
if (self->next_read_idx < self->frame_end) {
PyErr_SetString(state->UnpicklingError,
"beginning of a new frame before end of current frame");
return -1;
}
_Unpickler_EndFrame(self);
}

frame_len = calc_binsize(s, 8);
if (frame_len < 0) {
PyErr_Format(PyExc_OverflowError,
Expand All @@ -7091,6 +7174,12 @@ load_frame(PickleState *state, UnpicklerObject *self)

/* Rewind to start of frame */
self->next_read_idx -= frame_len;

/* Cap input_len at the frame end so reads can't cross it (PEP 3154);
_Unpickler_EndFrame() restores it when the frame is fully read. */
self->frame_end = self->next_read_idx + frame_len;
self->saved_input_len = self->input_len;
self->input_len = self->frame_end;
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions Modules/pyexpat.c
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,7 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
new_parser->specified_attributes = self->specified_attributes;
new_parser->in_callback = 0;
new_parser->ns_prefixes = self->ns_prefixes;
new_parser->reparse_deferral_enabled = self->reparse_deferral_enabled;
new_parser->itself = XML_ExternalEntityParserCreate(self->itself, context,
encoding);
// The new subparser will make use of the parent XML_Parser inside of Expat.
Expand Down
6 changes: 3 additions & 3 deletions Objects/unicodeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -8462,6 +8462,9 @@ _PyUnicode_EncodeIconv(const char *encoding, PyObject *unicode,
}

if (ret != (size_t)-1) {
if (flushing) {
break;
}
/* A positive result counts nonreversible conversions: iconv()
substituted an unencodable character instead of failing with
EILSEQ (musl and *BSD citrus do this). Treat it as unencodable
Expand All @@ -8479,9 +8482,6 @@ _PyUnicode_EncodeIconv(const char *encoding, PyObject *unicode,
out = out_before;
up -= unit;
}
else if (flushing) {
break;
}
else if (careful && up < uend) {
continue;
}
Expand Down
Loading