reading nested multipart messages does not work correctly

[terencehonles]

Long story short

Multipart reader breaks after reading a sub multipart end boundary and starting the next part

Expected behaviour

Nested multipart reader reads a message created with the multipart writer correctly

Actual behaviour

ValueError: Invalid boundary b'', expected b'--b0b69248b3a345cf8256a8dd25f07874'

Steps to reproduce

Receive the multipart response from #1525

Server:

from aiohttp.multipart import MultipartWriter
from aiohttp.web import Response

def handle(request):
  with MultipartWriter('mixed') as root:
    with MultipartWriter('mixed') as subwriter1:
      subwriter1.append('first message')
    root.append(subwriter1, headers=subwriter1.headers)

    with MultipartWriter('mixed') as subwriter2:
      subwriter2.append('second message')
    root.append(subwriter2, headers=subwriter2.headers)
  return Response(body=b''.join(root.serialize()), headers=root.headers)

# ... create web app which responds with the handler above ...

Client:

import aiohttp
import asyncio
from aiohttp.multipart import BodyPartReader, MultipartReader

@asyncio.coroutine
def read_multipart(reader):
  while True:
    part = yield from reader.next()
    if part is None: break
    if isinstance(part, BodyPartReader):
      body = yield from part.read(decode=True)
      print('body part: %r' % body)
    else:
      print('nested part')
      yield from read_multipart(part)

@asyncio.coroutine
def request(url):
  response = yield from aiohttp.get(url)
  yield from read_multipart(MultipartReader.from_response(response))

# ... drive event loop and call request(handler_url) ...

Issue

Lines multipart.py:767 and multipart.py:969 have line endings which result in an empty line after the end boundary of a multipart message (valid). However the reader terminates after the end boundary and the parent reader now expects the next boundary, but what is found is the blank line from multipart.py:767

Possible fix

diff --git a/aiohttp/multipart.py b/aiohttp/multipart.py
index af7f19b1..82ad2306 100644
--- a/aiohttp/multipart.py
+++ b/aiohttp/multipart.py
@@ -639,6 +639,7 @@ class MultipartReader(object):
             pass
         elif chunk == self._boundary + b'--':
             self._at_eof = True
+            yield from self._readline()
         else:
             raise ValueError('Invalid boundary %r, expected %r'
                              % (chunk, self._boundary))

[fafhrd91]

@kxepal could you check this

[kxepal]

Hm...it seems like some of our tests lies us since we have covered nested parts case there. Will check it in a moment.

[terencehonles]

@kxepal the tests are hand coded strings and do not include a multipart epilogue. From what I have been seeing around when researching if the epilogue is required it does appear it is, but if you know otherwise please correct me.

[kxepal]

@terencehonles Ah, epilogue. The epilogue must not be used. So it wasn't counted upon. Thought not sure if it's better crash or silently swallow it.

[terencehonles]

@kxepal let's keep conversation on the PR #1532 for now

[lock]

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.