requests / toolbelt

A toolbelt of useful classes and functions to be used with python-requests
https://toolbelt.readthedocs.org
Other
989 stars 186 forks source link

MultipartEncoder can't handle mmap.mmap object correctly #346

Open yx1994 opened 1 year ago

yx1994 commented 1 year ago

MultipartEncoder can't handle mmap.mmap object correctly.

MultipartEncoder can't handle mmap.mmap object correctly. For example,

from requests_toolbelt import MultipartEncoder
import mmap

m = MultipartEncoder(
    fields={
            'field': ('filename', mmap.mmap(-1, 10))}
    )

m.read()

it will fall into an infinity loop in m.read().

I review those source code, find that the coerce_data function did't consider about the mmap.mmap object. Then the total_len function work different from other io object (always return the origin size). Then the bytes_left_to_write function always return True, fall into the infinity loop.

In my opinion, judge the mmap.mmap object by isinstance(data, mmap.mmap) or hasattr(data, 'madvise'), or other method?

Now, I fix my code by manually wrapped with FileWrapper, as

from requests_toolbelt.multipart.encoder import MultipartEncoder, FileWrapper
import mmap

m = MultipartEncoder(
    fields={
            'field': ('filename', FileWrapper(mmap.mmap(-1, 10)))}
    )

m.read()

I have little experience to pull requests. Could someone fix it from this package?

sigmavirus24 commented 1 year ago

Nothing considered it because it wasn't intended to be supported. We could more explicitly error on this, @pquentin

pquentin commented 1 year ago

Well, if wrapping it as a FileWrapper works maybe we could do that automatically in coerce_data?

sigmavirus24 commented 1 year ago

If we land this for urllib3, sure. I'm not convinced this library needs feature work, even if it's low hanging fruit like this