-
-
Notifications
You must be signed in to change notification settings - Fork 184
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
Move to lxml #286
Move to lxml #286
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,19 @@ | ||
import os | ||
from collections import defaultdict | ||
from typing import Optional, Iterable, Union, Any, List, Dict | ||
from typing import Optional, Iterable, Union, Tuple, List, Dict | ||
|
||
import junitparser | ||
from junitparser import Element, JUnitXml, TestCase, TestSuite, Skipped | ||
from junitparser.junitparser import etree | ||
|
||
from publish.unittestresults import ParsedUnitTestResults, UnitTestCase, ParseError | ||
|
||
try: | ||
import lxml | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. opt.semgrep.python.lang.security.use-defused-xml.use-defused-xml: Found use of the native Python XML libraries, which is vulnerable to XML external entity (XXE) (at-me in a reply with Was this a good recommendation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. defusedxml.lxml is deprecated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sonatype-lift ignore There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've recorded this as ignored for this pull request. If you change your mind, just comment |
||
lxml_available = True | ||
except ImportError: | ||
lxml_available = False | ||
|
||
|
||
def get_results(results: Union[Element, List[Element]], status: Optional[str] = None) -> List[Element]: | ||
""" | ||
|
@@ -97,10 +103,24 @@ def end(self, tag: Union[str, bytes]) -> Element: | |
if self._stack: | ||
self._stack.pop() | ||
|
||
def close(self) -> Element: | ||
# when lxml is around, we have to return an ElementTree here, otherwise | ||
# XMLParser(target=...).parse(..., parser=...) | ||
# returns an Element, not a ElementTree, but junitparser expects an ElementTree | ||
# | ||
# https://lxml.de/parsing.html: | ||
# Note that the parser does not build a tree when using a parser target. The result of the parser run is | ||
# whatever the target object returns from its .close() method. If you want to return an XML tree here, you | ||
# have to create it programmatically in the target object. | ||
if lxml_available: | ||
return lxml.etree.ElementTree(super().close()) | ||
else: | ||
return super().close() | ||
|
||
|
||
def parse_junit_xml_files(files: Iterable[str], time_factor: float = 1.0, drop_testcases: bool = False) -> ParsedUnitTestResults: | ||
"""Parses junit xml files and returns aggregated statistics as a ParsedUnitTestResults.""" | ||
def parse(path: str) -> Union[str, Any]: | ||
def parse(path: str) -> Union[JUnitXml, BaseException]: | ||
if not os.path.exists(path): | ||
return FileNotFoundError(f'File does not exist.') | ||
if os.stat(path).st_size == 0: | ||
|
@@ -114,8 +134,7 @@ def parse(path: str) -> Union[str, Any]: | |
except BaseException as e: | ||
return e | ||
|
||
parsed_files = [(result_file, parse(result_file)) | ||
for result_file in files] | ||
parsed_files = [(result_file, parse(result_file)) for result_file in files] | ||
junits = [(result_file, junit) | ||
for result_file, junit in parsed_files | ||
if not isinstance(junit, BaseException)] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blacklist: Using lxml to parse untrusted XML data is known to be vulnerable to XML attacks. Replace lxml with the equivalent defusedxml package.
(at-me in a reply with
help
orignore
)Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defusedxml.lxml is deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sonatype-lift ignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've recorded this as ignored for this pull request. If you change your mind, just comment
@sonatype-lift unignore
.