[FIX] core: avoid feeding client invalid XML-RPC documents

The XML-RPC interface has a compatibility shim for binaries as
historically Odoo has returned "binary" data as base64 strings. To
avoid breakages during the Python 3 transition, the shim was
introduced to decode the output binary data (under the assumption that
it'd be ASCII-compatible).

In the case where the data is *not* ascii-compatible, however, it can
generate invalid XML documents: "C0" control codes (with the exception
of tab, LF, and CR) are not valid in XML 1.0 (which XML-RPC is an
application of), however they're perfectly valid string characters and
the standard library's marshaller does not check for them, embedding
them directly in the output document and breaking the client's
decoding.

Work around the issue by replacing such binary data with an empty
string.

While at it, move the bytes shim to the customized marshaller, this
way everything's at the same place and it's not necessary to waste
time trying to understand why the marshaller is just not calling what
it's supposed to call.

Fixes #61919

closes odoo/odoo#74699

Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>

odoo/addons/base/controllers/rpc.py

+import re
+import xmlrpc.client
 from datetime import date, datetime
 from xmlrpc.client import dumps, loads
-import xmlrpc.client
 
 from werkzeug.wrappers import Response
 
 from odoo.http import Controller, dispatch_rpc, request, route
 from odoo.service import wsgi_server
 from odoo.fields import Date, Datetime
-from odoo.tools import lazy
+from odoo.tools import lazy, ustr
 from odoo.tools.misc import frozendict
 
-
+# ustr decodes as utf-8 or latin1 so we can search for the ASCII bytes
+# 	Char	   ::=   	#x9 | #xA | #xD | [#x20-#xD7FF]
+XML_INVALID = re.compile(b'[\x00-\x08\x0B\x0C\x0F-\x1F]')
 class OdooMarshaller(xmlrpc.client.Marshaller):
-
-    """
-    XMLRPC Marshaller that converts date(time) objects to strings in iso8061 format.
-    """
-
     dispatch = dict(xmlrpc.client.Marshaller.dispatch)
 
     def dump_frozen_dict(self, value, write):
@@ -24,6 +22,21 @@ class OdooMarshaller(xmlrpc.client.Marshaller):
         self.dump_struct(value, write)
     dispatch[frozendict] = dump_frozen_dict
     
+    # By default, in xmlrpc, bytes are converted to xmlrpclib.Binary object.
+    # Historically, odoo is sending binary as base64 string.
+    # In python 3, base64.b64{de,en}code() methods now works on bytes.
+    # Convert them to str to have a consistent behavior between python 2 and python 3.
+    def dump_bytes(self, value, write):
+        # XML 1.0 disallows control characters, check for them immediately to
+        # see if this is a "real" binary (rather than base64 or somesuch) and
+        # blank it out, otherwise they get embedded in the output and break
+        # client-side parsers
+        if XML_INVALID.search(value):
+            self.dump_unicode('', write)
+        else:
+            self.dump_unicode(ustr(value), write)
+    dispatch[bytes] = dump_bytes
+
     def dump_datetime(self, value, write):
         # override to marshall as a string for backwards compatibility
         value = Datetime.to_string(value)

odoo/addons/base/tests/test_xmlrpc.py

 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 import time
+from xmlrpc.client import Binary
 
 from odoo.exceptions import AccessDenied, AccessError
 from odoo.http import _request_stack
@@ -80,6 +81,13 @@ class TestXMLRPC(common.HttpCase):
             }
         })
 
+    def test_xmlrpc_attachment_raw(self):
+        ids = self.env['ir.attachment'].create({'name': 'n', 'raw': b'\x01\02\03'}).ids
+        [att] = self.xmlrpc_object.execute(
+            common.get_db_name(), self.admin_uid, 'admin',
+            'ir.attachment', 'read', ids, ['raw'])
+        self.assertEqual(att['raw'], '', "actual binary data should be blanked out on read")
+
 # really just for the test cursor
 @common.tagged('post_install', '-at_install')
 class TestAPIKeys(common.HttpCase):

odoo/service/server.py

@@ -1240,7 +1240,6 @@ def start(preload=None, stop=False):
     global server
 
     load_server_wide_modules()
-    odoo.service.wsgi_server._patch_xmlrpc_marshaller()
 
     if odoo.evented:
         server = GeventServer(odoo.service.wsgi_server.application)

odoo/service/wsgi_server.py

@@ -70,17 +70,6 @@ def xmlrpc_handle_exception_string(e):
 
     return xmlrpclib.dumps(fault, allow_none=None, encoding=None)
 
-def _patch_xmlrpc_marshaller():
-    # By default, in xmlrpc, bytes are converted to xmlrpclib.Binary object.
-    # Historically, odoo is sending binary as base64 string.
-    # In python 3, base64.b64{de,en}code() methods now works on bytes.
-    # Convert them to str to have a consistent behavior between python 2 and python 3.
-    # TODO? Create a `/xmlrpc/3` route prefix that respect the standard and uses xmlrpclib.Binary.
-    def dump_bytes(marshaller, value, write):
-        marshaller.dump_unicode(odoo.tools.ustr(value), write)
-
-    xmlrpclib.Marshaller.dispatch[bytes] = dump_bytes
-
 def application_unproxied(environ, start_response):
     """ WSGI entry point."""
     # cleanup db/uid trackers - they're set at HTTP dispatch in