Skip to content

Commit 60dd6b1

Browse files
tmmampagent
andauthored
fix(zbugs): take the ws:// zero-cache address the backend prints (#176)
zbugs' backend prints zero-cache as ws://127.0.0.1:4848, and the clients only take http:// and https://. zbugs-swift and zbugs-gpui crashed on it, as on any address the client refused, and zbugs-rune stayed in Error. Each app now maps ws:// to http:// and wss:// to https:// before it makes the client. An address with another scheme, or no host, runs the client offline and says why: zbugs-swift with a warning over the list, and in the Servers sheet, whose Save it disables; zbugs-gpui with a notification, and by refusing it in Servers; zbugs-rune in its Servers sheet and in the connection badge. Amp-Thread-ID: https://ampcode.com/threads/T-744fb220-85d0-410a-b6e5-d39a2e621de5 Co-authored-by: Amp <amp@ampcode.com>
1 parent fb63ae5 commit 60dd6b1

10 files changed

Lines changed: 135 additions & 14 deletions

File tree

‎apps/zbugs-gpui/src/app.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,9 @@ impl ZbugsApp {
474474
let q = USER_PREF.call(&SKIN_TONE_PREF.to_owned(), Some(a));
475475
Live::new(&session.zero, q, &ttl(), cx)
476476
});
477+
// A zero-cache address the client can't take, from the environment
478+
// or the saved file: the client runs offline (`Session::new`).
479+
let error = zbugs_model::cache_http_url(&session.cache_url).err();
477480
let mut app = ZbugsApp {
478481
session,
479482
filters,
@@ -500,7 +503,7 @@ impl ZbugsApp {
500503
comment,
501504
scroll: UniformListScrollHandle::new(),
502505
connection,
503-
error: None,
506+
error,
504507
view_timer: None,
505508
title: String::new(),
506509
seen: None,
@@ -569,6 +572,11 @@ impl ZbugsApp {
569572
cx.notify();
570573
return false;
571574
}
575+
if let Err(e) = zbugs_model::cache_http_url(&cache_url) {
576+
self.error = Some(e);
577+
cx.notify();
578+
return false;
579+
}
572580
save_setting("cache-url", Some(&cache_url));
573581
save_setting("api-url", Some(&api_url));
574582
let token = self.session.token.clone();

‎apps/zbugs-gpui/src/session.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ impl Session {
102102
auth.clone(),
103103
client_mutators(),
104104
ZeroOptions {
105-
cache_url: Some(cache_url.clone()),
105+
// A bad address, from the environment or the saved file,
106+
// runs offline: the app shows why (`ZbugsApp::new`).
107+
cache_url: zbugs_model::cache_http_url(&cache_url).ok(),
106108
user_id: auth.as_ref().map(|a| a.sub.clone()),
107109
auth: token.clone(),
108110
mutate_url: Some(format!("{api_url}/api/mutate")),

‎apps/zbugs-model/src/lib.rs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,47 @@ pub mod schema;
1717
pub use auth::{AuthData, make_id};
1818
pub use mutators::{Context, MutateRequest, client_mutators};
1919
pub use queries::ZERO_PROJECT_NAME;
20+
21+
/// The zero-cache address as the client takes it. zbugs' backend prints it
22+
/// as `ws://`, and the client only takes `http://` and `https://`, so `ws://`
23+
/// becomes `http://` and `wss://` becomes `https://`. Any other scheme, or
24+
/// no host, is an error to show.
25+
pub fn cache_http_url(url: &str) -> Result<String, String> {
26+
let url = url.trim();
27+
let (scheme, rest) = url.split_once("://").unwrap_or(("", ""));
28+
let scheme = match scheme.to_ascii_lowercase().as_str() {
29+
"http" | "ws" => "http",
30+
"https" | "wss" => "https",
31+
_ => "",
32+
};
33+
if scheme.is_empty() || rest.is_empty() || rest.starts_with(['/', '?', '#']) {
34+
return Err(format!(
35+
"{url:?} isn't a zero-cache address. Use an http://, https://, ws://, or wss:// URL, such as http://127.0.0.1:4848."
36+
));
37+
}
38+
Ok(format!("{scheme}://{rest}"))
39+
}
40+
41+
#[cfg(test)]
42+
mod tests {
43+
use super::cache_http_url;
44+
45+
#[test]
46+
fn cache_http_url_takes_websocket_schemes() {
47+
let ok = |u: &str| cache_http_url(u).unwrap();
48+
assert_eq!(ok("ws://127.0.0.1:4848"), "http://127.0.0.1:4848");
49+
assert_eq!(ok(" WSS://zero.example/x "), "https://zero.example/x");
50+
assert_eq!(ok("http://h:1/p"), "http://h:1/p");
51+
assert_eq!(ok("https://h"), "https://h");
52+
for bad in [
53+
"",
54+
"127.0.0.1:4848",
55+
"ftp://h",
56+
"ws://",
57+
"http:///p",
58+
"wsx://h",
59+
] {
60+
assert!(cache_http_url(bad).is_err(), "{bad}");
61+
}
62+
}
63+
}

‎apps/zbugs-rune/src/main.rs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,12 @@ impl App {
339339
self.settings.token = None;
340340
}
341341
self.me = None;
342+
// A bad address, from the environment or the saved settings, fails
343+
// the client, and the connection badge says why.
344+
let cache_url = zbugs_model::cache_http_url(&self.settings.cache_url)
345+
.unwrap_or_else(|_| self.settings.cache_url.clone());
342346
self.sync.connect(Session {
343-
cache_url: self.settings.cache_url.clone(),
347+
cache_url,
344348
api_url: self.settings.api_url.clone(),
345349
token: self.settings.token.clone(),
346350
user_id: self.auth.as_ref().map(|a| a.sub.clone()),
@@ -1088,8 +1092,12 @@ impl App {
10881092
pub fn save_servers(&mut self) {
10891093
let cache = self.cache_edit.text.trim().trim_end_matches('/').to_owned();
10901094
let api = self.api_edit.text.trim().trim_end_matches('/').to_owned();
1091-
if !cache.starts_with("http") || !api.starts_with("http") {
1092-
self.sheet_error = Some("Both are http:// or https:// URLs.".into());
1095+
if let Err(e) = zbugs_model::cache_http_url(&cache) {
1096+
self.sheet_error = Some(e);
1097+
return;
1098+
}
1099+
if !api.starts_with("http") {
1100+
self.sheet_error = Some("The zbugs API is an http:// or https:// URL.".into());
10931101
return;
10941102
}
10951103
self.settings.cache_url = cache.clone();

‎apps/zbugs-rune/src/sync.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,8 @@ impl<S: 'static, C: Clone + 'static> Worker<S, C> {
255255
) {
256256
Ok(z) => z,
257257
Err(e) => {
258-
// Only a mutator registered twice fails here.
258+
// A mutator registered twice, or a cache URL that isn't
259+
// http:// or https://, fails here.
259260
let reason = e.message();
260261
let f = self.on_connection;
261262
self.ui
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import Foundation
2+
3+
/// The zero-cache address as the client takes it. zbugs' backend prints it
4+
/// as `ws://`, and the client only takes `http://` and `https://`, so `ws://`
5+
/// becomes `http://` and `wss://` becomes `https://`. Nil for any other
6+
/// scheme, or no host: `cacheURLError` says why.
7+
public func cacheClientURL(_ url: String) -> String? {
8+
let url = url.trimmingCharacters(in: .whitespacesAndNewlines)
9+
guard let separator = url.range(of: "://") else { return nil }
10+
let rest = url[separator.upperBound...]
11+
guard let first = rest.first, !"/?#".contains(first) else { return nil }
12+
switch url[..<separator.lowerBound].lowercased() {
13+
case "http", "ws": return "http://" + rest
14+
case "https", "wss": return "https://" + rest
15+
default: return nil
16+
}
17+
}
18+
19+
/// Why the client can't take `url` as the zero-cache address, or nil when it
20+
/// can.
21+
public func cacheURLError(_ url: String) -> String? {
22+
cacheClientURL(url) == nil
23+
? "\"\(url)\" isn't a zero-cache address. Use an http://, https://, ws://, or wss:// URL, such as http://127.0.0.1:4848."
24+
: nil
25+
}

‎apps/zbugs-swift/Sources/ZBugsUI/AppModel.swift‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@
7777
restart()
7878
}
7979

80+
/// Why the zero-cache address doesn't work, for a warning; the client
81+
/// runs offline until it's fixed.
82+
public var cacheURLError: String? { ZBugs.cacheURLError(cacheURL) }
83+
8084
/// Whether a server is set to this device itself, where the app can't
8185
/// reach the backend: see `isThisDevice`.
8286
public var serversAreThisDevice: Bool {
@@ -147,15 +151,18 @@
147151
private static func makeZero(
148152
cacheURL: String, apiURL: String, token: String?, auth: AuthData?
149153
) -> Zero<AuthData?> {
150-
var options = ZeroOptions(cacheURL: cacheURL, userID: auth?.sub, auth: token)
154+
// A bad address runs offline, as `cacheURLError` says, rather than
155+
// failing the client.
156+
var options = ZeroOptions(
157+
cacheURL: cacheClientURL(cacheURL), userID: auth?.sub, auth: token)
151158
options.mutateURL = apiURL + "/api/mutate"
152159
options.queryURL = apiURL + "/api/query"
153160
options.logLevel = .info
154161
do {
155162
return try Zero(schema: .zbugs, context: auth, mutators: Mutators.all, options: options)
156163
} catch {
157-
// Only a mutator registered twice fails; storage falls back to
158-
// memory on its own.
164+
// Only a mutator registered twice fails: the cache URL is one it
165+
// takes, and storage falls back to memory on its own.
159166
preconditionFailure("zbugs' mutators: \(error)")
160167
}
161168
}

‎apps/zbugs-swift/Sources/ZBugsUI/RootView.swift‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@
8585
.toolbar { toolbar }
8686
// Otherwise the list only says it's connecting, as it never will.
8787
.safeAreaInset(edge: .bottom) {
88-
if model.serversAreThisDevice && model.zero.connection.state != .connected {
89-
ServersWarning { sheet = .servers }
88+
if let error = model.cacheURLError {
89+
ServersWarning(message: error) { sheet = .servers }
90+
} else if model.serversAreThisDevice && model.zero.connection.state != .connected {
91+
ServersWarning(message: AppModel.thisDeviceWarning) { sheet = .servers }
9092
}
9193
}
9294
} detail: {
@@ -300,13 +302,14 @@
300302
}
301303
}
302304

303-
/// Servers set to this device, with a way to change them.
305+
/// Servers the app can't reach, with a way to change them.
304306
private struct ServersWarning: View {
307+
let message: String
305308
let change: () -> Void
306309

307310
var body: some View {
308311
VStack(alignment: .leading, spacing: 8) {
309-
SwiftUI.Label(AppModel.thisDeviceWarning, systemImage: "exclamationmark.triangle.fill")
312+
SwiftUI.Label(message, systemImage: "exclamationmark.triangle.fill")
310313
.symbolRenderingMode(.multicolor)
311314
Button("Servers…", action: change)
312315
.buttonStyle(.borderedProminent)

‎apps/zbugs-swift/Sources/ZBugsUI/Sheets.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,10 @@
145145
TextField("zero-cache", text: $cacheURL)
146146
TextField("zbugs API", text: $apiURL)
147147
} footer: {
148-
if AppModel.isThisDevice(cacheURL) || AppModel.isThisDevice(apiURL) {
148+
if let error = cacheURLError(cacheURL) {
149+
Text(error)
150+
.foregroundStyle(.red)
151+
} else if AppModel.isThisDevice(cacheURL) || AppModel.isThisDevice(apiURL) {
149152
Text(AppModel.thisDeviceWarning)
150153
.foregroundStyle(.orange)
151154
}
@@ -163,6 +166,7 @@
163166
model.setServers(cacheURL: cacheURL, apiURL: apiURL)
164167
dismiss()
165168
}
169+
.disabled(cacheURLError(cacheURL) != nil)
166170
}
167171
}
168172
.onAppear {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import Testing
2+
import ZBugs
3+
4+
@Suite struct CacheURLTests {
5+
@Test func takesWebSocketSchemesAsTheirHTTPOnes() {
6+
#expect(cacheClientURL("ws://127.0.0.1:4848") == "http://127.0.0.1:4848")
7+
#expect(cacheClientURL(" WSS://zero.example/x ") == "https://zero.example/x")
8+
#expect(cacheClientURL("http://h:1/p") == "http://h:1/p")
9+
#expect(cacheClientURL("https://h") == "https://h")
10+
}
11+
12+
@Test func rejectsOtherSchemesAndMissingHosts() {
13+
for bad in ["", "127.0.0.1:4848", "ftp://h", "ws://", "http:///p", "wsx://h"] {
14+
#expect(cacheClientURL(bad) == nil, "\(bad)")
15+
#expect(cacheURLError(bad) != nil, "\(bad)")
16+
}
17+
#expect(cacheURLError("ws://127.0.0.1:4848") == nil)
18+
}
19+
}

0 commit comments

Comments
 (0)