pwclient breaking on a Form Feed character

[Daniel Axtens]

Benjamin Copeland <ben.copeland at linaro.org> writes:

> My rather crude patch:
>
> diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py
> index 93f6035..220826a 100644
> --- a/patchwork/views/xmlrpc.py
> +++ b/patchwork/views/xmlrpc.py
> @@ -672,7 +672,8 @@ def patch_get_mbox(patch_id):
>      """
>      try:
>          patch = Patch.objects.get(id=patch_id)
> -        return patch_to_mbox(patch)
> +        s = patch_to_mbox(patch)
> +        return s.replace("\x0c", "")
>      except Patch.DoesNotExist:
>          return ''
>
> On 17 January 2018 at 22:52, Benjamin Copeland <ben.copeland at linaro.org> wrote:
>> On 17 January 2018 at 19:13, Stephen Finucane <stephen at that.guru> wrote:
>>> On Wed, 2018-01-17 at 11:46 +0000, Benjamin Copeland wrote:
>>>> Hi All,
>>>>
>>>> Been trying to get a patch but been unable due to:
>>>>
>>>> Traceback (most recent call last):
>>>>   File "pwclient", line 827, in <module>
>>>>     main()
>>>>   File "pwclient", line 780, in main
>>>>     action_get(rpc, patch_id)
>>>>   File "pwclient", line 301, in action_get
>>>>     s = rpc.patch_get_mbox(patch_id)
>>>>   File "/usr/lib64/python2.7/xmlrpclib.py", line 1243, in __call__
>>>>     return self.__send(self.__name, args)
>>>>   File "/usr/lib64/python2.7/xmlrpclib.py", line 1602, in __request
>>>>     verbose=self.__verbose
>>>>   File "/usr/lib64/python2.7/xmlrpclib.py", line 1283, in request
>>>>     return self.single_request(host, handler, request_body, verbose)
>>>>   File "/usr/lib64/python2.7/xmlrpclib.py", line 1316, in
>>>> single_request
>>>>     return self.parse_response(response)
>>>>   File "/usr/lib64/python2.7/xmlrpclib.py", line 1487, in
>>>> parse_response
>>>>     p.feed(data)
>>>>   File "/usr/lib64/python2.7/xmlrpclib.py", line 558, in feed
>>>>     self._parser.Parse(data, 0)
>>>> xml.parsers.expat.ExpatError: not well-formed (invalid token): line
>>>> 283, column 1
>>>>
>>>> After going quite a bit of debugging I noticed the patch has a ^L
>>>> (FF) in the diff. After deleting this out of the diff pwclient was
>>>> able to get the patch.
>>>>
>>>> This bug was present on Python3/2.
>>>
>>> Have you got a link to the offending patch? This will allow me to fix
>>> the issue and put together a fix to prevent regressions.
>>
>> https://patches-gcc.linaro.org/patch/411/


We saw this on another GCC related patch (binutils I think) last year,
when I was at OzLabs - a form feed character in a header. We didn't come
up with a good solution then.

Your proposed fix is - as you said - really quite a hack, and I'm
particularly uncomfortable with the idea that the diff you get from
pwclient != the diff patchwork stores.

The conclusion - which sadly was never documented - was that we should
just live with it for now and put our time into a REST API pwclient.

Fixing it properly is *hard* as it requires an encoding system, which
requires a patch to the server to encode and patch to pwclient to
decode. To avoid breaking existing pwclients, we would also need a new
endpoint. And XMLRPC is awful.

If it's a big problem, we'll come up with a fix proper, otherwise a
documentation fix and some better error handling might be the best we
can do.

Regards,
Daniel


>>
>>>
>>> Stephen
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork