Merge lp:~cjwatson/lazr.restful/text-field-normalize-crlf into lp:lazr.restful

[Colin Watson]

Normalize line breaks in text fields marshalled from a request.

multipart/form-data encoding (now used by lazr.restful.testing.webservice.WebServiceCaller for named POST requests) requires line breaks to be encoded as CRLF. This caused a discrepancy in Launchpad's lib/lp/code/stories/webservice/xx-branchmergeproposal.txt test, and it seems reasonable to normalize this in the specific case of text fields. Convert any line breaks to Unix-style LF there.

[Cristian Gonzalez (cristiangsp)]

Loos good!

review: Approve

=== modified file 'NEWS.rst'
--- NEWS.rst	2021-01-04 11:24:14 +0000
+++ NEWS.rst	2021-01-18 11:37:39 +0000
@@ -11,6 +11,9 @@
 allowing robust use of binary arguments on both Python 2 and 3
 (bug 1116954).
 
+Normalize line breaks in text fields marshalled from a request to Unix-style
+LF, since ``multipart/form-data`` encoding requires CRLF.
+
 0.23.0 (2020-09-28)
 ===================
 
 
=== modified file 'src/lazr/restful/docs/webservice-marshallers.rst'
--- src/lazr/restful/docs/webservice-marshallers.rst	2020-09-07 09:30:30 +0000
+++ src/lazr/restful/docs/webservice-marshallers.rst	2021-01-18 11:37:39 +0000
@@ -26,6 +26,7 @@
     >>> def pformat_value(value):
     ...     """Pretty-format a single value."""
     ...     if isinstance(value, six.text_type):
+    ...         value = value.encode('unicode_escape').decode('ASCII')
     ...         if "'" in value and '"' not in value:
     ...             return '"%s"' % value
     ...         else:
@@ -450,6 +451,15 @@
     >>> print(marshaller.marshall_from_request('null'))
     None
 
+Line breaks coming from the request are normalized to LF.
+
+    >>> pprint_value(marshaller.marshall_from_request('abc\r\n\r\ndef\r\n'))
+    'abc\n\ndef\n'
+    >>> pprint_value(marshaller.marshall_from_request('abc\n\ndef\n'))
+    'abc\n\ndef\n'
+    >>> pprint_value(marshaller.marshall_from_request('abc\r\rdef\r'))
+    'abc\n\ndef\n'
+
 Bytes
 -----
 
 
=== modified file 'src/lazr/restful/example/base/interfaces.py'
--- src/lazr/restful/example/base/interfaces.py	2020-09-02 22:35:45 +0000
+++ src/lazr/restful/example/base/interfaces.py	2021-01-18 11:37:39 +0000
@@ -238,6 +238,11 @@
     def getRecipes():
         """Return the list of recipes."""
 
+    @export_factory_operation(
+        IRecipe, ['id', 'cookbook', 'dish', 'instructions', 'private'])
+    def createRecipe(id, cookbook, dish, instructions, private=False):
+        """Create a new recipe."""
+
     def removeRecipe(recipe):
         """Remove a recipe from the list."""
 
 
=== modified file 'src/lazr/restful/example/base/root.py'
--- src/lazr/restful/example/base/root.py	2020-02-04 11:52:59 +0000
+++ src/lazr/restful/example/base/root.py	2021-01-18 11:37:39 +0000
@@ -19,6 +19,7 @@
 from zope.location.interfaces import ILocation
 from zope.component import getMultiAdapter, getUtility
 from zope.schema.interfaces import IBytes
+from zope.security.proxy import removeSecurityProxy
 
 from lazr.restful import directives, ServiceRootResource
 
@@ -180,9 +181,9 @@
     def __init__(self, id, cookbook, dish, instructions, private=False):
         self.id = id
         self.dish = dish
-        self.dish.recipes.append(self)
+        removeSecurityProxy(self.dish.recipes).append(self)
         self.cookbook = cookbook
-        self.cookbook.recipes.append(self)
+        removeSecurityProxy(self.cookbook.recipes).append(self)
         self.instructions = instructions
         self.private = private
         self.prepared_image = None
@@ -321,10 +322,15 @@
             return match[0]
         return None
 
+    def createRecipe(self, id, cookbook, dish, instructions, private=False):
+        recipe = Recipe(id, cookbook, dish, instructions, private=private)
+        self.recipes.append(recipe)
+        return recipe
+
     def removeRecipe(self, recipe):
         self.recipes.remove(recipe)
-        recipe.cookbook.removeRecipe(recipe)
-        recipe.dish.removeRecipe(recipe)
+        removeSecurityProxy(recipe.cookbook).removeRecipe(recipe)
+        removeSecurityProxy(recipe.dish).removeRecipe(recipe)
 
 
 # Define some globally accessible sample data.
 
=== modified file 'src/lazr/restful/example/base/tests/collection.txt'
--- src/lazr/restful/example/base/tests/collection.txt	2020-09-07 09:54:10 +0000
+++ src/lazr/restful/example/base/tests/collection.txt	2021-01-18 11:37:39 +0000
@@ -320,3 +320,30 @@
     HTTP/1.1 400 Bad Request
     ...
     No such operation: nosuchop
+
+POST operations may involve text fields.  These are marshalled via a
+multipart/form-data request, which requires line breaks to be represented as
+CRLF.  The server turns these into Unix-style LF.
+
+    >>> import six
+
+    >>> def create_recipe(id, cookbook_link, dish_link, instructions,
+    ...                   private=False):
+    ...     return webservice.named_post(
+    ...         "/recipes", "createRecipe", {},
+    ...         id=id, cookbook=cookbook_link, dish=dish_link,
+    ...         instructions=instructions, private=private)
+
+    >>> cookbook_link = webservice.get(
+    ...     quote("/cookbooks/The Joy of Cooking")).jsonBody()["self_link"]
+    >>> dish_link = webservice.get(
+    ...     quote("/dishes/Roast chicken")).jsonBody()["self_link"]
+    >>> response = create_recipe(
+    ...     20, cookbook_link, dish_link,
+    ...     "Recipe\r\ncontaining\rsome\nline\r\n\r\nbreaks")
+    >>> response.status
+    201
+    >>> recipe = webservice.get(response.getHeader("Location")).jsonBody()
+    >>> six.ensure_str(recipe["instructions"])
+    'Recipe\ncontaining\nsome\nline\n\nbreaks'
+    >>> _ = webservice.delete(recipe["self_link"])
 
=== modified file 'src/lazr/restful/marshallers.py'
--- src/lazr/restful/marshallers.py	2020-09-01 13:20:54 +0000
+++ src/lazr/restful/marshallers.py	2021-01-18 11:37:39 +0000
@@ -26,8 +26,9 @@
 
 from datetime import datetime
 from io import BytesIO
+import re
+
 import pytz
-
 import simplejson
 import six
 from six.moves.urllib.parse import unquote
@@ -329,6 +330,10 @@
         Converts the value to unicode.
         """
         value = six.text_type(value)
+        # multipart/form-data encoding of text fields is required (RFC 2046
+        # section 4.1.1) to use CRLF for line breaks.  Normalize to
+        # Unix-style LF.
+        value = re.sub(r'\r\n?', '\n', value)
         return super(TextFieldMarshaller, self)._marshall_from_request(value)