Skip to content

[2.7] bpo-30264: ExpatParser now closes the source #1476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 5, 2017
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
18 changes: 17 additions & 1 deletion Lib/test/test_sax.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
# $Id$

from xml.sax import make_parser, ContentHandler, \
SAXException, SAXReaderNotAvailable, SAXParseException
SAXException, SAXReaderNotAvailable, SAXParseException, \
saxutils
try:
make_parser()
except SAXReaderNotAvailable:
Expand Down Expand Up @@ -173,6 +174,21 @@ def test_parse_InputSource(self):
input.setEncoding('iso-8859-1')
self.check_parse(input)

def test_parse_close_source(self):
builtin_open = open
non_local = {'fileobj': None}

def mock_open(*args):
fileobj = builtin_open(*args)
non_local['fileobj'] = fileobj
return fileobj

with support.swap_attr(saxutils, 'open', mock_open):
make_xml_file(self.data, 'iso-8859-1', None)
with self.assertRaises(SAXException):
self.check_parse(TESTFN)
self.assertTrue(non_local['fileobj'].closed)

def check_parseString(self, s):
from xml.sax import parseString
result = StringIO()
Expand Down
25 changes: 22 additions & 3 deletions Lib/xml/sax/expatreader.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,16 @@ def parse(self, source):
source = saxutils.prepare_input_source(source)

self._source = source
self.reset()
self._cont_handler.setDocumentLocator(ExpatLocator(self))
xmlreader.IncrementalParser.parse(self, source)
try:
self.reset()
self._cont_handler.setDocumentLocator(ExpatLocator(self))
xmlreader.IncrementalParser.parse(self, source)
except:
# bpo-30264: Close the source on error to not leak resources:
# xml.sax.parse() doesn't give access to the underlying parser
# to the caller
self._close_source()
raise

def prepareParser(self, source):
if source.getSystemId() is not None:
Expand Down Expand Up @@ -216,6 +223,17 @@ def feed(self, data, isFinal = 0):
# FIXME: when to invoke error()?
self._err_handler.fatalError(exc)

def _close_source(self):
source = self._source
try:
file = source.getCharacterStream()
if file is not None:
file.close()
finally:
file = source.getByteStream()
if file is not None:
file.close()

def close(self):
if (self._entity_stack or self._parser is None or
isinstance(self._parser, _ClosedParser)):
Expand All @@ -235,6 +253,7 @@ def close(self):
parser.ErrorColumnNumber = self._parser.ErrorColumnNumber
parser.ErrorLineNumber = self._parser.ErrorLineNumber
self._parser = parser
self._close_source()

def _reset_cont_handler(self):
self._parser.ProcessingInstructionHandler = \
Expand Down