jorgeajimenezl / aiodav

Python Async WebDAV Client
MIT License
20 stars 8 forks source link

Content-Length may be missed in response header #10

Closed SudakovIvan closed 1 year ago

SudakovIvan commented 1 year ago

Since ACCEPT_ENCODING: 'gzip, deflate' is the default in aiohttp for requests, the apache webdav server may add Transfer-Encoding: chunked to the HTTP header instead of Content-Length. In this case, there will be an error when executing the download_to method. This bug can be reproduced by downloading a 500 megabyte file

SudakovIvan commented 1 year ago

Thanks for such a quick fix! Another improvement could be getting the total value from self.info?

Now its value can be None, and if the file is compressed on transfer, then the content_length(and total) will be less than the size of the resulting local file.

jorgeajimenezl commented 1 year ago

Hi, thank to you for detect the bug :).

Now its value can be None, > and if the file is compressed on transfer, then the content_length(and total) will be less than the size of the resulting local file.

I understand, but if I make a call to info, not only would it be one more call that you don't expect to happen, but also when reporting the progress, in case it is a compressed file, you would never have 100% on the progress, because the total is contrasted with the amount of bytes received. E.g. a 100B file compressed to 80B the last call to progress would be at most: progress(current=80, total=100).

SudakovIvan commented 1 year ago

As far as I understand, the current value is calculated on an already uncompressed buffer, isn't it?

It is also possible to call info instead of exists when implementing downliad_to. How do you think?

jorgeajimenezl commented 1 year ago

Well, technically yes, you are right, I made the pertinent changes to the https://github.com/jorgeajimenezl/aiodav/tree/issue10 branch. I can't test it right now because I'll be busy for a few days, if you could test it to see if it works correctly it would be very helpful 😅

SudakovIvan commented 1 year ago

I will be on vacation next week, but after that I will definitely test your changes, thanks

SudakovIvan commented 1 year ago

I tested your changes - it works.

But I have a question - why so?


        try:
            total = int(info["size"])
        except:
            total = response.content_length

size can be omitted in server response for files?

jorgeajimenezl commented 1 year ago

Yes, that's right, I re-read the corresponding RFC and this is so, thank you very much for the clarification again, :)

SudakovIvan commented 1 year ago

Ok, thanks. Are you planning a release soon?

jorgeajimenezl commented 1 year ago

I released it today, the changes should be available in version 0.1.12

SudakovIvan commented 1 year ago

Ok, thanks. This one can probably be closed...