From b261515afa18a5d2a38d729b174bbd99ddee14ac Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 22 Feb 2013 12:36:52 +0000 Subject: [PATCH 1/3] XML cleanup --- rest_framework/renderers.py | 38 +++++++++++- rest_framework/utils/__init__.py | 102 ------------------------------- 2 files changed, 36 insertions(+), 104 deletions(-) diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 637904c4e..41f4e0fc5 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -14,14 +14,17 @@ import json from django import forms from django.http.multipartparser import parse_header from django.template import RequestContext, loader, Template +from django.utils.xmlutils import SimplerXMLGenerator from rest_framework.compat import yaml from rest_framework.exceptions import ConfigurationError from rest_framework.settings import api_settings from rest_framework.request import clone_request -from rest_framework.utils import dict2xml from rest_framework.utils import encoders from rest_framework.utils.breadcrumbs import get_breadcrumbs from rest_framework import exceptions, parsers, status, VERSION +from rest_framework.compat import StringIO +from rest_framework.compat import six +from rest_framework.compat import smart_text class BaseRenderer(object): @@ -118,7 +121,38 @@ class XMLRenderer(BaseRenderer): """ if data is None: return '' - return dict2xml(data) + + stream = StringIO() + + xml = SimplerXMLGenerator(stream, "utf-8") + xml.startDocument() + xml.startElement("root", {}) + + self._to_xml(xml, data) + + xml.endElement("root") + xml.endDocument() + return stream.getvalue() + + def _to_xml(self, xml, data): + if isinstance(data, (list, tuple)): + for item in data: + xml.startElement("list-item", {}) + self._to_xml(xml, item) + xml.endElement("list-item") + + elif isinstance(data, dict): + for key, value in six.iteritems(data): + xml.startElement(key, {}) + self._to_xml(xml, value) + xml.endElement(key) + + elif data is None: + # Don't output any value + pass + + else: + xml.characters(smart_text(data)) class YAMLRenderer(BaseRenderer): diff --git a/rest_framework/utils/__init__.py b/rest_framework/utils/__init__.py index 3bab3b5fa..e69de29bb 100644 --- a/rest_framework/utils/__init__.py +++ b/rest_framework/utils/__init__.py @@ -1,102 +0,0 @@ -from __future__ import unicode_literals -from django.utils.xmlutils import SimplerXMLGenerator -from rest_framework.compat import StringIO -from rest_framework.compat import six -from rest_framework.compat import smart_text -import re -import xml.etree.ElementTree as ET - - -# From xml2dict -class XML2Dict(object): - - def __init__(self): - pass - - def _parse_node(self, node): - node_tree = {} - # Save attrs and text, hope there will not be a child with same name - if node.text: - node_tree = node.text - for (k, v) in node.attrib.items(): - k, v = self._namespace_split(k, v) - node_tree[k] = v - #Save childrens - for child in node.getchildren(): - tag, tree = self._namespace_split(child.tag, self._parse_node(child)) - if tag not in node_tree: # the first time, so store it in dict - node_tree[tag] = tree - continue - old = node_tree[tag] - if not isinstance(old, list): - node_tree.pop(tag) - node_tree[tag] = [old] # multi times, so change old dict to a list - node_tree[tag].append(tree) # add the new one - - return node_tree - - def _namespace_split(self, tag, value): - """ - Split the tag '{http://cs.sfsu.edu/csc867/myscheduler}patients' - ns = http://cs.sfsu.edu/csc867/myscheduler - name = patients - """ - result = re.compile("\{(.*)\}(.*)").search(tag) - if result: - value.namespace, tag = result.groups() - return (tag, value) - - def parse(self, file): - """parse a xml file to a dict""" - f = open(file, 'r') - return self.fromstring(f.read()) - - def fromstring(self, s): - """parse a string""" - t = ET.fromstring(s) - unused_root_tag, root_tree = self._namespace_split(t.tag, self._parse_node(t)) - return root_tree - - -def xml2dict(input): - return XML2Dict().fromstring(input) - - -# Piston: -class XMLRenderer(): - def _to_xml(self, xml, data): - if isinstance(data, (list, tuple)): - for item in data: - xml.startElement("list-item", {}) - self._to_xml(xml, item) - xml.endElement("list-item") - - elif isinstance(data, dict): - for key, value in six.iteritems(data): - xml.startElement(key, {}) - self._to_xml(xml, value) - xml.endElement(key) - - elif data is None: - # Don't output any value - pass - - else: - xml.characters(smart_text(data)) - - def dict2xml(self, data): - stream = StringIO() - - xml = SimplerXMLGenerator(stream, "utf-8") - xml.startDocument() - xml.startElement("root", {}) - - self._to_xml(xml, data) - - xml.endElement("root") - xml.endDocument() - return stream.getvalue() - - -def dict2xml(input): - return XMLRenderer().dict2xml(input) From dcee027fa97f015ff3b87f0fd72b7995cdd6e155 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 22 Feb 2013 13:17:22 +0000 Subject: [PATCH 2/3] defusedxml for security fix. As per: http://blog.python.org/2013/02/announcing-defusedxml-fixes-for-xml.html --- .travis.yml | 1 + README.md | 8 +++++--- docs/index.md | 2 ++ optionals.txt | 1 + rest_framework/compat.py | 17 ++++------------- rest_framework/parsers.py | 14 ++++++++------ rest_framework/tests/parsers.py | 4 ++++ rest_framework/tests/renderers.py | 13 ++++++------- tox.ini | 7 +++++++ 9 files changed, 38 insertions(+), 29 deletions(-) diff --git a/.travis.yml b/.travis.yml index 662ea5c8b..3787e5177 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ env: install: - pip install $DJANGO + - pip install defusedxml==0.3 - "if [[ ${TRAVIS_PYTHON_VERSION::1} != '3' ]]; then pip install django-filter==0.5.4 --use-mirrors; fi" - "if [[ ${TRAVIS_PYTHON_VERSION::1} == '3' ]]; then pip install https://github.com/alex/django-filter/tarball/master; fi" - export PYTHONPATH=. diff --git a/README.md b/README.md index 7b7d1d47c..4ab44e0bd 100644 --- a/README.md +++ b/README.md @@ -31,9 +31,10 @@ There is also a sandbox API you can use for testing purposes, [available here][s **Optional:** -* [Markdown] - Markdown support for the self describing API. -* [PyYAML] - YAML content type support. -* [django-filter] - Filtering support. +* [Markdown][markdown] - Markdown support for the self describing API. +* [PyYAML][pyyaml] - YAML content type support. +* [defusedxml][defusedxml] - XML content-type support. +* [django-filter][django-filter] - Filtering support. # Installation @@ -115,4 +116,5 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. [urlobject]: https://github.com/zacharyvoase/urlobject [markdown]: http://pypi.python.org/pypi/Markdown/ [pyyaml]: http://pypi.python.org/pypi/PyYAML +[defusedxml]: https://pypi.python.org/pypi/defusedxml [django-filter]: http://pypi.python.org/pypi/django-filter diff --git a/docs/index.md b/docs/index.md index 0188accf0..b2c04735a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -34,6 +34,7 @@ The following packages are optional: * [Markdown][markdown] (2.1.0+) - Markdown support for the browseable API. * [PyYAML][yaml] (3.10+) - YAML content-type support. +* [defusedxml][defusedxml] (0.3+) - XML content-type support. * [django-filter][django-filter] (0.5.4+) - Filtering support. ## Installation @@ -173,6 +174,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. [urlobject]: https://github.com/zacharyvoase/urlobject [markdown]: http://pypi.python.org/pypi/Markdown/ [yaml]: http://pypi.python.org/pypi/PyYAML +[defusedxml]: https://pypi.python.org/pypi/defusedxml [django-filter]: http://pypi.python.org/pypi/django-filter [0.4]: https://github.com/tomchristie/django-rest-framework/tree/0.4.X [image]: img/quickstart.png diff --git a/optionals.txt b/optionals.txt index 1d2358c6e..3d98cc0e7 100644 --- a/optionals.txt +++ b/optionals.txt @@ -1,3 +1,4 @@ markdown>=2.1.0 PyYAML>=3.10 +defusedxml>=0.3 django-filter>=0.5.4 diff --git a/rest_framework/compat.py b/rest_framework/compat.py index 3fd865f85..07fdddce4 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -421,17 +421,8 @@ except ImportError: yaml = None -# xml.etree.parse only throws ParseError for python >= 2.7 +# XML is optional try: - from xml.etree import ParseError as ETParseError -except ImportError: # python < 2.7 - ETParseError = None - - -# XMLParser only takes an encoding arg from >= 2.7 -def ET_XMLParser(encoding=None): - from xml.etree import ElementTree as ET - try: - return ET.XMLParser(encoding=encoding) - except TypeError: - return ET.XMLParser() + import defusedxml.ElementTree as etree +except ImportError: + etree = None diff --git a/rest_framework/parsers.py b/rest_framework/parsers.py index 06b022264..7bbb5f940 100644 --- a/rest_framework/parsers.py +++ b/rest_framework/parsers.py @@ -9,11 +9,9 @@ from django.conf import settings from django.http import QueryDict from django.http.multipartparser import MultiPartParser as DjangoMultiPartParser from django.http.multipartparser import MultiPartParserError -from rest_framework.compat import yaml, ETParseError, ET_XMLParser +from rest_framework.compat import yaml, etree from rest_framework.exceptions import ParseError from rest_framework.compat import six -from xml.etree import ElementTree as ET -from xml.parsers.expat import ExpatError import json import datetime import decimal @@ -80,6 +78,8 @@ class YAMLParser(BaseParser): `data` will be an object which is the parsed content of the response. `files` will always be `None`. """ + assert yaml, 'YAMLParser requires pyyaml to be installed' + parser_context = parser_context or {} encoding = parser_context.get('encoding', settings.DEFAULT_CHARSET) @@ -146,12 +146,14 @@ class XMLParser(BaseParser): media_type = 'application/xml' def parse(self, stream, media_type=None, parser_context=None): + assert etree, 'XMLParser requires defusedxml to be installed' + parser_context = parser_context or {} encoding = parser_context.get('encoding', settings.DEFAULT_CHARSET) - parser = ET_XMLParser(encoding=encoding) + parser = etree.DefusedXMLParser(encoding=encoding) try: - tree = ET.parse(stream, parser=parser) - except (ExpatError, ETParseError, ValueError) as exc: + tree = etree.parse(stream, parser=parser) + except (etree.ParseError, ValueError) as exc: raise ParseError('XML parse error - %s' % six.u(exc)) data = self._xml_convert(tree.getroot()) diff --git a/rest_framework/tests/parsers.py b/rest_framework/tests/parsers.py index c03df08f9..539c5b44f 100644 --- a/rest_framework/tests/parsers.py +++ b/rest_framework/tests/parsers.py @@ -2,6 +2,8 @@ from __future__ import unicode_literals from rest_framework.compat import StringIO from django import forms from django.test import TestCase +from django.utils import unittest +from rest_framework.compat import etree from rest_framework.parsers import FormParser from rest_framework.parsers import XMLParser import datetime @@ -69,11 +71,13 @@ class TestXMLParser(TestCase): ] } + @unittest.skipUnless(etree, 'defusedxml not installed') def test_parse(self): parser = XMLParser() data = parser.parse(self._input) self.assertEqual(data, self._data) + @unittest.skipUnless(etree, 'defusedxml not installed') def test_complex_data_parse(self): parser = XMLParser() data = parser.parse(self._complex_data_input) diff --git a/rest_framework/tests/renderers.py b/rest_framework/tests/renderers.py index 90ef12212..0f3fe3f10 100644 --- a/rest_framework/tests/renderers.py +++ b/rest_framework/tests/renderers.py @@ -1,23 +1,21 @@ -import pickle -import re - +from decimal import Decimal from django.core.cache import cache from django.test import TestCase from django.test.client import RequestFactory - +from django.utils import unittest from rest_framework import status, permissions -from rest_framework.compat import yaml, patterns, url, include +from rest_framework.compat import yaml, etree, patterns, url, include from rest_framework.response import Response from rest_framework.views import APIView from rest_framework.renderers import BaseRenderer, JSONRenderer, YAMLRenderer, \ XMLRenderer, JSONPRenderer, BrowsableAPIRenderer from rest_framework.parsers import YAMLParser, XMLParser from rest_framework.settings import api_settings - from rest_framework.compat import StringIO from rest_framework.compat import six import datetime -from decimal import Decimal +import pickle +import re DUMMYSTATUS = status.HTTP_200_OK @@ -410,6 +408,7 @@ class XMLRendererTestCase(TestCase): self.assertXMLContains(content, 'first') self.assertXMLContains(content, 'second') + @unittest.skipUnless(etree, 'defusedxml not installed') def test_render_and_parse_complex_data(self): """ Test XML rendering. diff --git a/tox.ini b/tox.ini index 05733efe1..ee4db1465 100644 --- a/tox.ini +++ b/tox.ini @@ -9,11 +9,13 @@ commands = {envpython} rest_framework/runtests/runtests.py basepython = python3.3 deps = https://www.djangoproject.com/download/1.5c1/tarball/ https://github.com/alex/django-filter/archive/master.tar.gz + defusedxml==0.3 [testenv:py3.2-django1.5] basepython = python3.2 deps = https://www.djangoproject.com/download/1.5c1/tarball/ https://github.com/alex/django-filter/archive/master.tar.gz + defusedxml==0.3 [testenv:py2.7-django1.5] basepython = python2.7 @@ -24,23 +26,28 @@ deps = https://www.djangoproject.com/download/1.5c1/tarball/ basepython = python2.6 deps = https://www.djangoproject.com/download/1.5c1/tarball/ django-filter==0.5.4 + defusedxml==0.3 [testenv:py2.7-django1.4] basepython = python2.7 deps = django==1.4.3 django-filter==0.5.4 + defusedxml==0.3 [testenv:py2.6-django1.4] basepython = python2.6 deps = django==1.4.3 django-filter==0.5.4 + defusedxml==0.3 [testenv:py2.7-django1.3] basepython = python2.7 deps = django==1.3.5 django-filter==0.5.4 + defusedxml==0.3 [testenv:py2.6-django1.3] basepython = python2.6 deps = django==1.3.5 django-filter==0.5.4 + defusedxml==0.3 From 569c3a28e662ccef251acc6494047ec9c83556c2 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 22 Feb 2013 19:41:09 +0000 Subject: [PATCH 3/3] Add forbid_dtd flag, since we don't need any DTDs. --- rest_framework/parsers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/parsers.py b/rest_framework/parsers.py index 7bbb5f940..491acd68c 100644 --- a/rest_framework/parsers.py +++ b/rest_framework/parsers.py @@ -152,7 +152,7 @@ class XMLParser(BaseParser): encoding = parser_context.get('encoding', settings.DEFAULT_CHARSET) parser = etree.DefusedXMLParser(encoding=encoding) try: - tree = etree.parse(stream, parser=parser) + tree = etree.parse(stream, parser=parser, forbid_dtd=True) except (etree.ParseError, ValueError) as exc: raise ParseError('XML parse error - %s' % six.u(exc)) data = self._xml_convert(tree.getroot())