Repeated client request with multipart FormData results in field duplication

[ulrikjohansson]

Long story short

We've run into a problem where a repeated client session request with the same arguments end up not sending the same request data, but duplicted fields.
Reproducible request are important to us because we're using a circuit breaker library with our client requests that do retries on certain errors (like ServerDisconnectedError and TimeoutError)

Expected behaviour

When I repeat a call to ClientSession.request with the exact same arguments I expect to send a copy of the first request

Actual behaviour

When using a FormData object with a field that gets the _is_multipart property set to True, the form fields are duplicated. The machinery inside FormData._gen_form_data just appends the form fields again on each request using it.
`

Steps to reproduce

import asyncio
from aiohttp import ClientSession, FormData
from copy import deepcopy

from aiohttp import __version__ as version
print(f"aiohttp version: {version}")

async def run():
    async with ClientSession() as session:
        url = "http://httpbin.org/anything"
        data = FormData()
        data.add_field("test", "test_value", content_type="application/json")

        for n in range(2):
            await session.post(url, data=data)
            print(f"writer part count: {len(data._writer._parts)} <-- this should stay at 1")

loop = asyncio.get_event_loop()
loop.run_until_complete(run())

Workaround

A workaround for this issue is passing a deepcopy of the FormData object to ClientSession.request

Your environment

aiohttp version: tested with 3.5.4 & 3.6.2
aiohttp component: client
os: linux (ubuntu 19.10)

[asvetlov]

Very interesting, thanks.
FormData was never designed to be reused but implicit duplication is not appropriate.

Need to dig more what exact lines in aiohttp sources produce the error.
I see two possible solutions:

1. Don't mutate FormData on sending, which would be tricky especially for sending files.
2. Mark FormData instance as processed, raise an exception in the second usage. Simple and robust.

A champion is wanted for the issue, I expected that the fix for 2) is not hard at all.

[AtomsForPeace]

This should be fixed now in master

[asvetlov]

Fixed by #4351