Skip to content

xml.dom.minidom does not escape CR, LF and TAB characters within attribute values #50002

Closed

Description

@Tomalak
mannequin
BPO 5752
Nosy @amauryfa, @devdanzin, @dvarrazzo, @ezio-melotti, @wking, @mitar, @moriyoshi
Files
  • minidom_test.py: A small test script that outlines the problem
  • test_toxml.py: New test script to demonstrate that the patch proposed works
  • test_multiline_roundtrip.diff: Tomalak's minidom_test.py as unittest, patch for test_minidom
  • minidom.patch: Proposed patch to minidom.py
  • minidom.diff: Old file before new file in patch.
  • minidom.py: monkey patch module applying minidom.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2009-04-14.11:13:06.464>
    labels = ['expert-XML', 'easy', 'type-bug']
    title = 'xml.dom.minidom does not escape CR, LF and TAB characters within attribute values'
    updated_at = <Date 2019-06-22.20:28:40.662>
    user = 'https://bugs.python.org/Tomalak'

    bugs.python.org fields:

    activity = <Date 2019-06-22.20:28:40.662>
    actor = 'mitar'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['XML']
    creation = <Date 2009-04-14.11:13:06.464>
    creator = 'Tomalak'
    dependencies = []
    files = ['13921', '13960', '13966', '13977', '18184', '18325']
    hgrepos = []
    issue_num = 5752
    keywords = ['patch', 'easy']
    message_count = 33.0
    messages = ['85964', '87178', '87332', '87333', '87351', '87429', '87431', '87432', '87526', '87527', '87528', '87564', '87586', '87590', '87604', '87609', '87676', '87679', '87681', '90548', '90676', '110999', '111493', '111642', '112497', '112570', '119728', '142964', '185398', '185408', '185419', '185423', '185576']
    nosy_count = 13.0
    nosy_names = ['effbot', 'amaury.forgeotdarc', 'ajaksu2', 'piro', 'ezio.melotti', 'sayap', 'Tomalak', 'devon', 'labrat', 'mitar', 'moriyoshi', 'gauges', 'Jens.Grivolla']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue5752'
    versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']

    Linked PRs

    Activity

    1. Tomalak commented on Apr 14, 2009

      Tomalakmannequin
      MannequinAuthor

      Current behavior upon toxml() is:

      <foo attribute="multiline
      value" />

      Upon reading the document again, the new line is normalized and
      collapsed into a space (according to the XML spec, section 3.3.3), which
      means that it is lost.

      Better behavior would be something like this (within attribute values only):

      <foo attribute="multiline value" />

    2. sechifrancesco commented on May 4, 2009

      sechifrancescomannequin
      Mannequin

      Ok, I've tried to solve this problem, but I think that the keyword
      'easy' is not suitable for this kind of task, because it is necessary to
      modify the expat module that is very complex.

    3. Tomalak commented on May 6, 2009

      Tomalakmannequin
      MannequinAuthor

      @francesco Sechi: Would it not just require a minimal change to the
      _write_data() method? Something along the lines of (sorry, no Python
      expert, maybe I am way off):

      def _write_data(writer, data, is_attrib=False):
          "Writes datachars to writer."
          if is_attrib: 
              data = data.replace("\r", "&#13;").replace("\n", "&#10;")
          data = data.replace("&", "&amp;").replace("<", "&lt;")
          data = data.replace("\"", "&quot;").replace(">", "&gt;")
          writer.write(data)

      and in Element.writexml():

          #[...]
          for a_name in a_names:
              writer.write(" %s=\"" % a_name)
              _write_data(writer, attrs[a_name].value, True)
          #[...]
    4. Tomalak commented on May 6, 2009

      Tomalakmannequin
      MannequinAuthor

      Of course it should be:

      def _write_data(writer, data, is_attrib=False):
          "Writes datachars to writer."
          data = data.replace("&", "&amp;").replace("<", "&lt;")
          data = data.replace("\"", "&quot;").replace(">", "&gt;")
          if is_attrib: 
              data = data.replace("\r", "&#13;").replace("\n", "&#10;")
          writer.write(data)
    5. sechifrancesco commented on May 6, 2009

      sechifrancescomannequin
      Mannequin

      Don't worry, I'm a newer too.
      No, your solution does not work, because the method you refer
      (_write_data) is called by the toxml() function, but the newline is
      replaced with a whitespace by the parsestring() function. The
      parsestring function, as I already said, refers to the 'expat' module,
      that is very complex (for me).

    6. Tomalak commented on May 8, 2009

      Tomalakmannequin
      MannequinAuthor

      Hmm... I thought toxml() is the part that needs to be fixed, not the
      parsing/reading. I mentioned the reading only to outline the data loss
      that occurs eventually.

      My point is: The toxml() (i.e. _write_data) *actually writes* the
      newline to the output. And within parameters, it just shouldn't.

    7. Tomalak commented on May 8, 2009

      Tomalakmannequin
      MannequinAuthor

      Attaching a patch that fixes the problem.

    8. Tomalak commented on May 8, 2009

      Tomalakmannequin
      MannequinAuthor

      Attaching a test file that outlines the problem. Output on my system
      (Windows / Python 3.0) is:

      Without the patch:
      C:\Python30>python.exe c:\minidom_test.py
      False
      1 -->"multiline
      value"
      2 -->"multiline value"

      With the patch:
      C:\Python30>python.exe c:\minidom_test.py
      True
      1 -->"multiline
      value"
      2 -->"multiline
      value"

    9. sechifrancesco commented on May 10, 2009

      sechifrancescomannequin
      Mannequin

      I think that the problem is: the xmldoc1 has the newline or not? If it
      hasn't your patch works only in the particular case you pass a toxml
      return value to 'parsestring'. If I pass an XML string with newlines it
      doesn't work. So your solution is not generic and cannot be considered a
      patch for the issue you proposed.

    10. sechifrancesco commented on May 10, 2009

      sechifrancescomannequin
      Mannequin

      I try to explain better what is my opinion:

      • If you add the attribute by using setAttribute the newlines are kept
        and the toxml works well

      • If you add the attribute by using the parsestring, passing it an XML
        string the newlines are replaced

      • Your patch works only if you act on a well-constructed (i.e.with
        newlines kept in internal data structures) xml.dom.minidom.Document
        object only, so...

      • If you try to execute your patched toxml method of a
        xml.dom.minidom.Document constructed using parsestring passing it a
        string with newline it does not work.

      So your patch works only in a specific case: you are trying to fix a
      problem in parsestring, acting on its actual parameter.

    11. Tomalak commented on May 10, 2009

      Tomalakmannequin
      MannequinAuthor

      Francesco, I think you are missing the point. :-) The problem has two sides.

      If I create an XML document using the DOM (not by parsing it from a
      string!), then I can put newline characters into attribute value. This
      is allowed and conforms to the XML spec.

      However, *literal* newlines in an attribute value (i.e. when the
      document is parsed from a string) have no meaning. The parser treats
      them as if they were insignificant whitespace -- they are converted to a
      single space. This is also valid and conforms to the XML spec.

      The catch: This leads to an actual data loss if I *wanted* to store
      newline characters in an attribute -- unless the newline characters are
      properly encoded. Encoding the newline characters is also valid and
      conforms to the spec, so the DOM implementation should do it.

      In other words - the parsing process you refer to is actually working
      fine. If an attribute contains a literal newline, it is indeed okay to
      collapse it into a space. It's only the document serializing that is broken.

      Minidom is clearly missing functionality here, and it does not conform
      to the XML spec. If I store a string of data in an XML document, it must
      be ensured that upon reading the document again, I get the *same* data
      back. This is what I check with my test script.

    12. changed the title [-]xml.dom.minidom does not handle newline characters in attribute values[/-] [+]xml.dom.minidom does not escape newline characters within attribute values[/+] on May 10, 2009
    13. 18 remaining items

    14. dvarrazzo commented on Mar 28, 2013

      dvarrazzomannequin
      Mannequin

      Just got bitten by this bug, which affects xml.etree.ElementTree and cElementTree too. Any chance to have it fixed?

      Note that lxml.etree is not affected by the bug and can be used as a replacement for the stdlib module if you happen to have some work to do.

    15. amauryfa commented on Mar 28, 2013

      @amauryfa
      Contributor

      minidom may be broken, but what's the issue with ElementTree?

      >>> import xml.etree.cElementTree as etree
      >>> doc = etree.fromstring('<xml />')
      >>> doc.set('attr', "multiline\nvalue")
      >>> etree.tostring(doc)
      '<xml attr="multiline&#10;value" />'
    16. dvarrazzo commented on Mar 28, 2013

      dvarrazzomannequin
      Mannequin

      ElementTree issue is with tabs:

      In [1]: import xml.etree.cElementTree as etree
      
      In [2]: doc = etree.fromstring('<xml />')
      
      In [4]: doc.set('attr', "here\tthere")
      
      In [5]: etree.tostring(doc)
      Out[5]: '<xml attr="here\tthere" />'
      
      In [6]: etree.fromstring(_5).attrib['attr']
      Out[6]: 'here there'
      

      bye bye tab.

    17. dvarrazzo commented on Mar 28, 2013

      dvarrazzomannequin
      Mannequin

      I was going to open an issue on itself about etree and tabs, but I've found this and thought it would have been marked as duplicate. If you don't think it's the case I will open it anyway.

      I've added my comment because ElementTree is not reported in this page at all: I've been googling forever about "ElementTree tabs attributes" without result. I've found this (and the network of duplicates) only searching for less generic "xml tabs" into the python bug tracker, when I was already filing the bug.

    18. dvarrazzo commented on Mar 30, 2013

      dvarrazzomannequin
      Mannequin

      Added separate issue bpo-17582 as ElementTree implementation doesn't depend on whatever causes the bug in xml.dom.minidom.

    19. transferred this issue fromon Apr 10, 2022
    20. jbaker6953 commented on Jan 2, 2023

      @jbaker6953

      I had to make an updated version of this monkey patch for a project I was working on. I leave it here for anyone's reference. The old instance method overwrite was bombing out with TypeError: function() argument 'code' must be code, not function so I just changed it for my needs. It works well with my data set.

      https://gist.github.com/jbaker6953/deb9a8e4eae1e622f467fc9b4edf11db

    21. added 3 commits that reference this issue on Aug 14, 2023
    22. added a commit that references this issue on Aug 23, 2023
    23. hugovk commented on Nov 10, 2023

      @hugovk
      Member

      Looks like this has been fixed by #107947.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions