google / go-cloud

The Go Cloud Development Kit (Go CDK): A library and tools for open cloud development in Go.
https://gocloud.dev/
Apache License 2.0
9.45k stars 799 forks source link

blob/azureblob: Do not panic if Content-Length and Content-Range are missing #3445

Closed chancez closed 3 weeks ago

chancez commented 4 weeks ago

I found that when we're downloading an a gzipped object with Content-Encoding: gzip, AZB is transparently sending back an uncompressed object rather than the gzipped object, and when this happens, I also discovered that go-cloud fails and panics because the Content-Length and the Content-Range fields are not set in this situation.

To gracefully handle this, check if the ContentLength field is nil before using it, and return 0 if both ContentRange and ContentLength are missing. This unfortunately does mean Size() may return 0 in some cases, but I don't see a good way to fix that without a bigger change in design. With this patch, I can confirm our issue is resolved.

Unfortunately, I did not see a good reference for how I could write a unit test for this. It seems existing conformance tests rely on a live bucket, and there's no unit tests that mock the underlying response or server for any of the existing drivers that I could reference either. If you have a suggestion for testing, let me know, but currently no new tests are added as part of this PR.

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 73.43%. Comparing base (ba58ec7) to head (8799b3b).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3445 +/- ## ========================================== + Coverage 73.40% 73.43% +0.02% ========================================== Files 113 113 Lines 14948 14950 +2 ========================================== + Hits 10973 10978 +5 + Misses 3201 3198 -3 Partials 774 774 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.