Add tests for CurlDownloadStrategy#tarball_path
Sometimes, `brew cask fetch`/`install` fails with an error message similar to this: ``` ==> Downloading https://w3g3a5v6.ssl.hwcdn.net/upload2/game/214692/735 Error: Download failed on Cask 'steamed-hams' with message: Operation not supported @ rb_sysopen - /Users/claudia/Documents/dev/brew/var/homebrew/locks/steamed-hams--1.0 .com&Expires=1520937180&Signature=CGmDulxL8pmutKTlCleNTUY%2FyO9Xyl5u9y VZUE0uWrjadjuz67Jp7zx3H7NEOhSyOhu8nzicEHRBjr3uSoOJzwkLC8LBLKnz%2B2X%2B iq5m6IdwSVFcLp2Q1Hr2kR7ETn3rF1DIq5o0lHCyzMmyNe5giEKJNW8WF0KXriULhzLTWL SA3ZTLCIofAdRiiGje1kNYY3C0SBqymQB8CG3ONn5kj7CIGbxrDOq5xI2ZSJdIyPysSX7S LvEDBw2KdR24q9t1wfjS9LUzelf5TWk6ojj8p9%2FHjl%2Fi%2FVCXNN4o1mW%2FMayy2t TY1qcC%2FTmqI1ulZS8SNuaSgr9Iys9oDF1%2BPK%2B4Sg==&hwexp=1520937440&hwsi g=55bc66884b925ef22f8673c33bfcc33b.incomplete.lock ``` To reproduce the issue, check out this branch and run the `cask/download_strategy` test suite: ``` brew tests --only=cask/download_strategy ``` Or take a real-life example, which would be a Cask whose URL has **1.** no `.` character anywhere in the URL path itself (outside of the domain name), **and 2.** at least one query parameter with a `.` character in it; **and 3.** other query parameters following that with a combined length of more than 255 characters. This combination may be uncommon but it exists, especially in [URLs that change on every visit](1002d41242/doc/cask_language_reference/stanzas/url.md (urls-that-change-on-every-visit)), for example [`steamed-hams`](9d7df499cd/Casks/steamed-hams.rb) from my `claui/cask-games` tap: $ brew tap claui/cask-games $ brew cask fetch steamed-hams In a nutshell, **URL query strings sometimes look like a very long file extension to Homebrew,** which it then proceeds to use as a file name. In `CurlDownloadStrategy`, Homebrew seems to apply a heuristic to the cask URL in order to figure out a possibly meaningful file extension. Sometimes this heuristic produces immensely long file extensions, especially when there’s a query parameter with a `.` character in it but not in the URL path itself (outside of the domain name). Homebrew then believes that everything after the `.` is a file extension. In one of the later steps, it tries to create a lock file containing that extension, which fails because HFS+ cannot handle files whose base file name has more than 255 characters. One solution would be to improve Homebrew’s extension detector a bit so that it won’t cross individual URL query param boundaries any longer. This is done by adding `&` as a stop character: ``` def ext Pathname.new(@url).extname[/[^?&]+/] end ``` This appears to fix the issue for most (if not all) practical purposes.
This commit is contained in:
parent
d5799a5a68
commit
ba830df4e6
@ -93,6 +93,66 @@ describe "download strategies", :cask do
|
||||
expect(curl_args.each_cons(2)).to include(["-e", "http://somehost/also"])
|
||||
end
|
||||
end
|
||||
|
||||
context "with a file name trailing the URL path" do
|
||||
describe "#tarball_path" do
|
||||
subject { downloader.tarball_path }
|
||||
its(:extname) { is_expected.to eq(".dmg") }
|
||||
end
|
||||
end
|
||||
|
||||
context "with no discernible file name in it" do
|
||||
let(:url) { "http://example.com/download" }
|
||||
|
||||
describe "#tarball_path" do
|
||||
subject { downloader.tarball_path }
|
||||
its(:to_path) { is_expected.to end_with("some-cask--1.2.3.4") }
|
||||
end
|
||||
end
|
||||
|
||||
context "with a file name trailing the first query parameter" do
|
||||
let(:url) { "http://example.com/download?file=cask.zip&a=1" }
|
||||
|
||||
describe "#tarball_path" do
|
||||
subject { downloader.tarball_path }
|
||||
its(:extname) { is_expected.to eq(".zip") }
|
||||
end
|
||||
end
|
||||
|
||||
context "with a file name trailing the second query parameter" do
|
||||
let(:url) { "http://example.com/dl?a=1&file=cask.zip&b=2" }
|
||||
|
||||
describe "#tarball_path" do
|
||||
subject { downloader.tarball_path }
|
||||
its(:extname) { is_expected.to eq(".zip") }
|
||||
end
|
||||
end
|
||||
|
||||
context "with an unusually long query string" do
|
||||
let(:url) do
|
||||
[
|
||||
"https://node49152.ssl.fancycdn.example.com",
|
||||
"/fancycdn/node/49152/file/upload/download",
|
||||
"?cask_class=zf920df",
|
||||
"&cask_group=2348779087242312",
|
||||
"&cask_archive_file_name=cask.zip",
|
||||
"&signature=CGmDulxL8pmutKTlCleNTUY%2FyO9Xyl5u9yVZUE0",
|
||||
"uWrjadjuz67Jp7zx3H7NEOhSyOhu8nzicEHRBjr3uSoOJzwkLC8L",
|
||||
"BLKnz%2B2X%2Biq5m6IdwSVFcLp2Q1Hr2kR7ETn3rF1DIq5o0lHC",
|
||||
"yzMmyNe5giEKJNW8WF0KXriULhzLTWLSA3ZTLCIofAdRiiGje1kN",
|
||||
"YY3C0SBqymQB8CG3ONn5kj7CIGbxrDOq5xI2ZSJdIyPysSX7SLvE",
|
||||
"DBw2KdR24q9t1wfjS9LUzelf5TWk6ojj8p9%2FHjl%2Fi%2FVCXN",
|
||||
"N4o1mW%2FMayy2tTY1qcC%2FTmqI1ulZS8SNuaSgr9Iys9oDF1%2",
|
||||
"BPK%2B4Sg==",
|
||||
].join("")
|
||||
end
|
||||
|
||||
describe "#tarball_path" do
|
||||
subject { downloader.tarball_path }
|
||||
its(:extname) { is_expected.to eq(".zip") }
|
||||
its("to_path.length") { is_expected.to be_between(0, 255) }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe Hbc::CurlPostDownloadStrategy do
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user