Skip to content

Commit 24f5889

Browse files
vivianludrickclaude
andcommitted
review: check curl result, fall back to x64 when arm64 fetch fails
Per review on PR #32: this binding hardcodes the legacy S3 host, where the arm64 object may not exist yet — and curl's result was ignored, so an HTTP error body would be saved as the binary and satisfy binary_path()'s file_exists() check forever. Now: CURLOPT_FAILONERROR stops error bodies from reaching disk; a failed -darwin-arm64 download falls back to the x64 binary (works on Apple Silicon via Rosetta 2); if everything fails, the partial file is removed and LocalException is thrown instead of returning a poisoned path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c0567b2 commit 24f5889

1 file changed

Lines changed: 29 additions & 13 deletions

File tree

‎lib/LocalBinary.php‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,12 @@ private function platform_url(){
6969
}
7070

7171
public function download_binary($path) {
72-
$url = $this->platform_url();
72+
$urls = array($this->platform_url());
73+
// If the arm64 binary is not published to the legacy bucket yet, fall
74+
// back to the x64 binary, which still works on Apple Silicon via
75+
// Rosetta 2 — releasing must not turn a working download into a 404.
76+
if (substr($urls[0], -13) === '-darwin-arm64')
77+
$urls[] = str_replace('-darwin-arm64', '-darwin-x64', $urls[0]);
7378
if (!file_exists($path))
7479
mkdir($path, 0777, true);
7580

@@ -79,18 +84,29 @@ public function download_binary($path) {
7984
$dest_binary_name = $dest_binary_name. ".exe";
8085
}
8186
$dest_binary_path = $path. '/'. $dest_binary_name;
82-
$file = fopen($dest_binary_path , "w+");
83-
$ch = curl_init("");
84-
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
85-
curl_setopt($ch, CURLOPT_URL, $url);
86-
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
87-
curl_setopt($ch, CURLOPT_FILE, $file);
88-
$data = curl_exec ($ch);
89-
curl_close ($ch);
90-
91-
fclose($file);
92-
chmod($dest_binary_path, 0755);
93-
return $dest_binary_path;
87+
foreach ($urls as $url) {
88+
$file = fopen($dest_binary_path , "w+");
89+
$ch = curl_init("");
90+
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
91+
curl_setopt($ch, CURLOPT_URL, $url);
92+
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
93+
curl_setopt($ch, CURLOPT_FILE, $file);
94+
// Fail on HTTP >= 400 instead of writing the error body to disk —
95+
// a saved error body would satisfy the file_exists() check in
96+
// binary_path() and stick until the user deletes it by hand.
97+
curl_setopt($ch, CURLOPT_FAILONERROR, true);
98+
$data = curl_exec ($ch);
99+
curl_close ($ch);
100+
101+
fclose($file);
102+
if ($data !== false) {
103+
chmod($dest_binary_path, 0755);
104+
return $dest_binary_path;
105+
}
106+
// remove the empty/partial file so the next attempt (or next run) retries
107+
unlink($dest_binary_path);
108+
}
109+
throw new LocalException("Failed to download BrowserStackLocal binary from: " . implode(", ", $urls));
94110
}
95111

96112
private function get_available_dirs() {

0 commit comments

Comments
 (0)