bpo-30264: ExpatParser now closes the source (#1476)

bpo-30264: ExpatParser now closes the source (#1476) · python/cpython@d81f9e2

2 files changed

lines changed

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,8 @@`
`2`	`2`	`# $Id$`
`3`	`3`
`4`	`4`	`from xml.sax import make_parser, ContentHandler, \`
`5`		`-SAXException, SAXReaderNotAvailable, SAXParseException`
	`5`	`+SAXException, SAXReaderNotAvailable, SAXParseException, \`
	`6`	`+saxutils`
`6`	`7`	`try:`
`7`	`8`	`make_parser()`
`8`	`9`	`except SAXReaderNotAvailable:`
`@@ -173,6 +174,21 @@ def test_parse_InputSource(self):`
`173`	`174`	`input.setEncoding('iso-8859-1')`
`174`	`175`	`self.check_parse(input)`
`175`	`176`
	`177`	`+def test_parse_close_source(self):`
	`178`	`+builtin_open = open`
	`179`	`+non_local = {'fileobj': None}`
	`180`	`+`
	`181`	`+def mock_open(*args):`
	`182`	`+fileobj = builtin_open(*args)`
	`183`	`+non_local['fileobj'] = fileobj`
	`184`	`+return fileobj`
	`185`	`+`
	`186`	`+with support.swap_attr(saxutils, 'open', mock_open):`
	`187`	`+make_xml_file(self.data, 'iso-8859-1', None)`
	`188`	`+with self.assertRaises(SAXException):`
	`189`	`+self.check_parse(TESTFN)`
	`190`	`+self.assertTrue(non_local['fileobj'].closed)`
	`191`	`+`
`176`	`192`	`def check_parseString(self, s):`
`177`	`193`	`from xml.sax import parseString`
`178`	`194`	`result = StringIO()`

Original file line number	Diff line number	Diff line change
`@@ -105,9 +105,16 @@ def parse(self, source):`
`105`	`105`	`source = saxutils.prepare_input_source(source)`
`106`	`106`
`107`	`107`	`self._source = source`
`108`		`-self.reset()`
`109`		`-self._cont_handler.setDocumentLocator(ExpatLocator(self))`
`110`		`-xmlreader.IncrementalParser.parse(self, source)`
	`108`	`+try:`
	`109`	`+self.reset()`
	`110`	`+self._cont_handler.setDocumentLocator(ExpatLocator(self))`
	`111`	`+xmlreader.IncrementalParser.parse(self, source)`
	`112`	`+except:`
	`113`	`+# bpo-30264: Close the source on error to not leak resources:`
	`114`	`+# xml.sax.parse() doesn't give access to the underlying parser`
	`115`	`+# to the caller`
	`116`	`+self._close_source()`
	`117`	`+raise`
`111`	`118`
`112`	`119`	`def prepareParser(self, source):`
`113`	`120`	`if source.getSystemId() is not None:`
`@@ -216,6 +223,17 @@ def feed(self, data, isFinal = 0):`
`216`	`223`	`# FIXME: when to invoke error()?`
`217`	`224`	`self._err_handler.fatalError(exc)`
`218`	`225`
	`226`	`+def _close_source(self):`
	`227`	`+source = self._source`
	`228`	`+try:`
	`229`	`+file = source.getCharacterStream()`
	`230`	`+if file is not None:`
	`231`	`+file.close()`
	`232`	`+finally:`
	`233`	`+file = source.getByteStream()`
	`234`	`+if file is not None:`
	`235`	`+file.close()`
	`236`	`+`
`219`	`237`	`def close(self):`
`220`	`238`	`if (self._entity_stack or self._parser is None or`
`221`	`239`	`isinstance(self._parser, _ClosedParser)):`
`@@ -235,6 +253,7 @@ def close(self):`
`235`	`253`	`parser.ErrorColumnNumber = self._parser.ErrorColumnNumber`
`236`	`254`	`parser.ErrorLineNumber = self._parser.ErrorLineNumber`
`237`	`255`	`self._parser = parser`
	`256`	`+self._close_source()`
`238`	`257`
`239`	`258`	`def _reset_cont_handler(self):`
`240`	`259`	`self._parser.ProcessingInstructionHandler = \`