download_strategy: compare cached file size to Content-Length

[EricFromCanada]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

Before using a cached download, also check that the Content-Length header returned from the URL matches the size of the file. This should catch instances where a newer version of a cask isn't downloaded because a cask's URL doesn't change between updates, i.e. Homebrew/homebrew-cask#204165.

Fixes Homebrew/homebrew-cask#204166, but there's bound to be edge cases.

[MikeMcQuaid]

Thanks! Some style tweaks and should be good to go.

[MikeMcQuaid]

Thanks again!

[dduugg]

This might be inscrutable, but if you want to short-circuit:

Suggested change

	use_cached_location = false if cached_location.exist? && (newer_last_modified || different_file_size)
	use_cached_location &&= cached_location.exist? && (newer_last_modified || different_file_size)

[dduugg]

Maybe not in this PR, but both the name and return type strongly suggest using a URLMetadata (or such) struct here instead.

[EricFromCanada]

Not sure how to get the tests to pass:

$ brew tests --only download_strategies/curl --fail-fast
Randomized with seed 42865
1 process for 1 spec, ~ 1 spec per process
.......F

Failures:

  1) CurlDownloadStrategy#fetch calls curl with default arguments
     Failure/Error: strategy.fetch
     
     Errno::ENOENT:
       No such file or directory @ rb_file_s_size - /private/tmp/homebrew-tests-20250311-35401-g5955c/cache/downloads/3d1c0ae7da22be9d83fb1eb774df96b7c4da71d3cf07e1cb28555cf9a5e5af70--foo.tar.gz
     # ./download_strategy.rb:426:in `size'
     # ./download_strategy.rb:426:in `size'
     # ./download_strategy.rb:426:in `fetch'
     # ./test/download_strategies/curl_spec.rb:48:in `block (3 levels) in <top (required)>'

[dduugg]

Not sure how to get the tests to pass:

$ brew tests --only download_strategies/curl --fail-fast
Randomized with seed 42865
1 process for 1 spec, ~ 1 spec per process
.......F

Failures:

  1) CurlDownloadStrategy#fetch calls curl with default arguments
     Failure/Error: strategy.fetch
     
     Errno::ENOENT:
       No such file or directory @ rb_file_s_size - /private/tmp/homebrew-tests-20250311-35401-g5955c/cache/downloads/3d1c0ae7da22be9d83fb1eb774df96b7c4da71d3cf07e1cb28555cf9a5e5af70--foo.tar.gz
     # ./download_strategy.rb:426:in `size'
     # ./download_strategy.rb:426:in `size'
     # ./download_strategy.rb:426:in `fetch'
     # ./test/download_strategies/curl_spec.rb:48:in `block (3 levels) in <top (required)>'

Ah, I think line 426 needs a cached_location.exist? guard

[EricFromCanada]

Ah, I think line 426 needs a cached_location.exist? guard

Odd that cached_location.mtime doesn't error out on the previous line though.

[dduugg]

Ah, I think line 426 needs a cached_location.exist? guard

Odd that cached_location.mtime doesn't error out on the previous line though.

I'm guessing that there's no last_modified within the test case