Skip to content

Commit ce8cb48

Browse files
committed
address final comments
1 parent e898811 commit ce8cb48

File tree

4 files changed

+47
-24
lines changed

4 files changed

+47
-24
lines changed

Doc/library/pyexpat.rst

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@ The :mod:`xml.parsers.expat` module contains two functions:
7272
*encoding* [1]_ is given it will override the implicit or explicit encoding of the
7373
document.
7474

75+
.. _xmlparser-non-root:
76+
77+
Parsers created through :func:`!ParserCreate` are called "root" parsers,
78+
in the sense that they do not have any parent parser attached. Non-root
79+
parsers are created by :meth:`parser.ExternalEntityParserCreate
80+
<xmlparser.ExternalEntityParserCreate>`.
81+
7582
Expat can optionally do XML namespace processing for you, enabled by providing a
7683
value for *namespace_separator*. The value must be a one-character string; a
7784
:exc:`ValueError` will be raised if the string has an illegal length (``None``
@@ -120,11 +127,6 @@ The :mod:`xml.parsers.expat` module contains two functions:
120127
XMLParser Objects
121128
-----------------
122129

123-
.. class:: xmlparser
124-
125-
The type of an Expat XML parser created by :func:`ParserCreate`.
126-
127-
128130
:class:`xmlparser` objects have the following methods:
129131

130132

@@ -236,7 +238,7 @@ XMLParser Objects
236238
.. versionadded:: 3.13
237239

238240

239-
:class:`xmlparser` objects have the following methods to mitigate some
241+
:class:`!xmlparser` objects have the following methods to mitigate some
240242
common XML vulnerabilities.
241243

242244
.. method:: xmlparser.SetAllocTrackerActivationThreshold(threshold, /)
@@ -247,7 +249,8 @@ common XML vulnerabilities.
247249
By default, parser objects have an allocation activation threshold of 64 MiB,
248250
or equivalently 67,108,864 bytes.
249251

250-
An :exc:`ExpatError` is raised if this method is called on a non-root parser.
252+
An :exc:`ExpatError` is raised if this method is called on a
253+
|xml-non-root-parser| parser.
251254
The corresponding :attr:`~ExpatError.lineno` and :attr:`~ExpatError.offset`
252255
should not be used as they may have no special meaning.
253256

@@ -270,8 +273,8 @@ common XML vulnerabilities.
270273

271274
By default, parser objects have a maximum amplification factor of 100.0.
272275

273-
An :exc:`ExpatError` is raised if this method is called on a non-root
274-
parser or if *max_factor* is outside the valid range.
276+
An :exc:`ExpatError` is raised if this method is called on a
277+
|xml-non-root-parser| parser or if *max_factor* is outside the valid range.
275278
The corresponding :attr:`~ExpatError.lineno` and :attr:`~ExpatError.offset`
276279
should not be used as they may have no special meaning.
277280

@@ -1007,3 +1010,4 @@ The ``errors`` module has the following attributes:
10071010
not. See https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-EncodingDecl
10081011
and https://www.iana.org/assignments/character-sets/character-sets.xhtml.
10091012
1013+
.. |xml-non-root-parser| replace:: :ref:`non-root <xmlparser-non-root>`

Doc/whatsnew/3.15.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ xml.parsers.expat
545545

546546
* Add :func:`~xml.parsers.expat.xmlparser.SetAllocTrackerActivationThreshold`
547547
and :func:`~xml.parsers.expat.xmlparser.SetAllocTrackerMaximumAmplification`
548-
to :class:`~xml.parsers.expat.xmlparser` objects to prevent use of
548+
to :ref:`xmlparser <xmlparser-objects>` objects to prevent use of
549549
disproportional amounts of dynamic memory from within an Expat parser.
550550
(Contributed by Bénédikt Tran in :gh:`90949`.)
551551

Lib/test/test_pyexpat.py

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import re
88
import sys
99
import sysconfig
10+
import textwrap
1011
import unittest
1112
import traceback
1213
from io import BytesIO
@@ -834,24 +835,42 @@ class AttackProtectionTestBase(abc.ABC):
834835
"""
835836

836837
@staticmethod
837-
def exponential_expansion_payload(ncols, nrows, text='.'):
838+
def exponential_expansion_payload(*, nrows, ncols, text='.'):
838839
"""Create a billion laughs attack payload.
839840
840841
Be careful: the number of total items is pow(n, k), thereby
841842
requiring at least pow(ncols, nrows) * sizeof(text) memory!
842843
"""
844+
template = textwrap.dedent(f"""\
845+
<?xml version="1.0"?>
846+
<!DOCTYPE doc [
847+
<!ENTITY row0 "{text}">
848+
<!ELEMENT doc (#PCDATA)>
849+
{{body}}
850+
]>
851+
<doc>&row{nrows};</doc>
852+
""").rstrip()
853+
843854
body = '\n'.join(
844855
f'<!ENTITY row{i + 1} "{f"&row{i};" * ncols}">'
845856
for i in range(nrows)
846857
)
847-
return f"""\
848-
<?xml version="1.0"?>
849-
<!DOCTYPE doc [
850-
<!ENTITY row0 "{text}">
851-
<!ELEMENT doc (#PCDATA)>
852-
{body}
853-
]>
854-
<doc>&row{nrows};</doc>"""
858+
body = textwrap.indent(body, ' ' * 4)
859+
return template.format(body=body)
860+
861+
def test_payload_generation(self):
862+
# self-test for exponential_expansion_payload()
863+
payload = self.exponential_expansion_payload(nrows=2, ncols=3)
864+
self.assertEqual(payload, textwrap.dedent("""\
865+
<?xml version="1.0"?>
866+
<!DOCTYPE doc [
867+
<!ENTITY row0 ".">
868+
<!ELEMENT doc (#PCDATA)>
869+
<!ENTITY row1 "&row0;&row0;&row0;">
870+
<!ENTITY row2 "&row1;&row1;&row1;">
871+
]>
872+
<doc>&row2;</doc>
873+
""").rstrip())
855874

856875
def assert_root_parser_failure(self, func, /, *args, **kwargs):
857876
"""Check that func(*args, **kwargs) is invalid for a sub-parser."""
@@ -966,7 +985,7 @@ def test_set_activation_threshold__threshold_reached(self):
966985
# Check that the threshold is reached by choosing a small factor
967986
# and a payload whose peak amplification factor exceeds it.
968987
self.assertIsNone(self.set_maximum_amplification(parser, 1.0))
969-
payload = self.exponential_expansion_payload(10, 4)
988+
payload = self.exponential_expansion_payload(ncols=10, nrows=4)
970989
self.assert_rejected(parser.Parse, payload, True)
971990

972991
def test_set_activation_threshold__threshold_not_reached(self):
@@ -976,7 +995,7 @@ def test_set_activation_threshold__threshold_not_reached(self):
976995
# Check that the threshold is reached by choosing a small factor
977996
# and a payload whose peak amplification factor exceeds it.
978997
self.assertIsNone(self.set_maximum_amplification(parser, 1.0))
979-
payload = self.exponential_expansion_payload(10, 4)
998+
payload = self.exponential_expansion_payload(ncols=10, nrows=4)
980999
self.assertIsNotNone(parser.Parse(payload, True))
9811000

9821001
def test_set_maximum_amplification__amplification_exceeded(self):
@@ -986,7 +1005,7 @@ def test_set_maximum_amplification__amplification_exceeded(self):
9861005
# Choose a max amplification factor expected to always be exceeded.
9871006
self.assertIsNone(self.set_maximum_amplification(parser, 1.0))
9881007
# Craft a payload for which the peak amplification factor is > 1.0.
989-
payload = self.exponential_expansion_payload(1, 2)
1008+
payload = self.exponential_expansion_payload(ncols=1, nrows=2)
9901009
self.assert_rejected(parser.Parse, payload, True)
9911010

9921011
def test_set_maximum_amplification__amplification_not_exceeded(self):
@@ -996,7 +1015,7 @@ def test_set_maximum_amplification__amplification_not_exceeded(self):
9961015
# Choose a max amplification factor expected to never be exceeded.
9971016
self.assertIsNone(self.set_maximum_amplification(parser, 1e4))
9981017
# Craft a payload for which the peak amplification factor is < 1e4.
999-
payload = self.exponential_expansion_payload(1, 2)
1018+
payload = self.exponential_expansion_payload(ncols=1, nrows=2)
10001019
self.assertIsNotNone(parser.Parse(payload, True))
10011020

10021021

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Add :func:`~xml.parsers.expat.xmlparser.SetAllocTrackerActivationThreshold`
22
and :func:`~xml.parsers.expat.xmlparser.SetAllocTrackerMaximumAmplification`
3-
to :class:`~xml.parsers.expat.xmlparser` objects to prevent use of
3+
to :ref:`xmlparser <xmlparser-objects>` objects to prevent use of
44
disproportional amounts of dynamic memory from within an Expat parser.
55
Patch by Bénédikt Tran.

0 commit comments

Comments
 (0)