refactor: remove misleading underscore prefixes from WebDavClient fields

The fields _client, _base_url, _username, _password were all actively
used throughout the struct's methods. The underscore prefix convention
signals unused fields, which was misleading for readers.

https://claude.ai/code/session_013ooJht2HrZUTXgNJFU79cV
This commit is contained in:
Claude 2026-04-16 07:22:45 +00:00
parent ac72955d23
commit 5c04e50956
No known key found for this signature in database

View file

@ -20,10 +20,10 @@ pub struct RemoteFileInfo {
/// WebDAV client wrapping reqwest with basic auth. Credentials are zeroized on drop. /// WebDAV client wrapping reqwest with basic auth. Credentials are zeroized on drop.
pub struct WebDavClient { pub struct WebDavClient {
_client: Client, client: Client,
_base_url: String, base_url: String,
_username: Zeroizing<String>, username: Zeroizing<String>,
_password: Zeroizing<String>, password: Zeroizing<String>,
} }
impl WebDavClient { impl WebDavClient {
@ -43,17 +43,17 @@ impl WebDavClient {
.build() .build()
.map_err(|e| Error::WebDav(format!("Failed to build HTTP client: {}", e)))?; .map_err(|e| Error::WebDav(format!("Failed to build HTTP client: {}", e)))?;
Ok(Self { Ok(Self {
_client: client, client,
_base_url: base_url, base_url,
_username: Zeroizing::new(username.to_string()), username: Zeroizing::new(username.to_string()),
_password: Zeroizing::new(password.to_string()), password: Zeroizing::new(password.to_string()),
}) })
} }
fn full_url(&self, path: &str) -> String { fn full_url(&self, path: &str) -> String {
let path = path.trim_start_matches('/'); let path = path.trim_start_matches('/');
if path.is_empty() { if path.is_empty() {
self._base_url.clone() self.base_url.clone()
} else { } else {
// Percent-encode path segments while preserving '/' // Percent-encode path segments while preserving '/'
let encoded: String = path let encoded: String = path
@ -61,15 +61,15 @@ impl WebDavClient {
.map(percent_encode) .map(percent_encode)
.collect::<Vec<_>>() .collect::<Vec<_>>()
.join("/"); .join("/");
format!("{}/{}", self._base_url, encoded) format!("{}/{}", self.base_url, encoded)
} }
} }
/// Test connection by issuing a PROPFIND depth 0 on the root. /// Test connection by issuing a PROPFIND depth 0 on the root.
pub async fn test_connection(&self) -> Result<()> { pub async fn test_connection(&self) -> Result<()> {
let resp = self._client let resp = self.client
.request(reqwest::Method::from_bytes(b"PROPFIND").expect("PROPFIND is a valid HTTP method"), &self._base_url) .request(reqwest::Method::from_bytes(b"PROPFIND").expect("PROPFIND is a valid HTTP method"), &self.base_url)
.basic_auth(self._username.as_str(), Some(self._password.as_str())) .basic_auth(self.username.as_str(), Some(self.password.as_str()))
.header("Depth", "0") .header("Depth", "0")
.header("Content-Type", "application/xml") .header("Content-Type", "application/xml")
.body(PROPFIND_BODY) .body(PROPFIND_BODY)
@ -89,9 +89,9 @@ impl WebDavClient {
/// List files at a given path using PROPFIND depth 1. /// List files at a given path using PROPFIND depth 1.
pub async fn list_files(&self, path: &str) -> Result<Vec<RemoteFileInfo>> { pub async fn list_files(&self, path: &str) -> Result<Vec<RemoteFileInfo>> {
let url = self.full_url(path); let url = self.full_url(path);
let resp = self._client let resp = self.client
.request(reqwest::Method::from_bytes(b"PROPFIND").expect("PROPFIND is a valid HTTP method"), &url) .request(reqwest::Method::from_bytes(b"PROPFIND").expect("PROPFIND is a valid HTTP method"), &url)
.basic_auth(self._username.as_str(), Some(self._password.as_str())) .basic_auth(self.username.as_str(), Some(self.password.as_str()))
.header("Depth", "1") .header("Depth", "1")
.header("Content-Type", "application/xml") .header("Content-Type", "application/xml")
.body(PROPFIND_BODY) .body(PROPFIND_BODY)
@ -113,15 +113,15 @@ impl WebDavClient {
return Err(Error::WebDav("PROPFIND response too large (>10MB)".into())); return Err(Error::WebDav("PROPFIND response too large (>10MB)".into()));
} }
let body = String::from_utf8_lossy(&bytes); let body = String::from_utf8_lossy(&bytes);
parse_propfind_response(&body, &self._base_url, path) parse_propfind_response(&body, &self.base_url, path)
} }
/// Download a file's contents. /// Download a file's contents.
pub async fn get_file(&self, path: &str) -> Result<Vec<u8>> { pub async fn get_file(&self, path: &str) -> Result<Vec<u8>> {
let url = self.full_url(path); let url = self.full_url(path);
let resp = self._client let resp = self.client
.get(&url) .get(&url)
.basic_auth(self._username.as_str(), Some(self._password.as_str())) .basic_auth(self.username.as_str(), Some(self.password.as_str()))
.send() .send()
.await?; .await?;
@ -147,9 +147,9 @@ impl WebDavClient {
/// Upload a file. /// Upload a file.
pub async fn put_file(&self, path: &str, content: Vec<u8>) -> Result<()> { pub async fn put_file(&self, path: &str, content: Vec<u8>) -> Result<()> {
let url = self.full_url(path); let url = self.full_url(path);
let resp = self._client let resp = self.client
.put(&url) .put(&url)
.basic_auth(self._username.as_str(), Some(self._password.as_str())) .basic_auth(self.username.as_str(), Some(self.password.as_str()))
.body(content) .body(content)
.send() .send()
.await?; .await?;
@ -164,9 +164,9 @@ impl WebDavClient {
/// Delete a remote file. /// Delete a remote file.
pub async fn delete_file(&self, path: &str) -> Result<()> { pub async fn delete_file(&self, path: &str) -> Result<()> {
let url = self.full_url(path); let url = self.full_url(path);
let resp = self._client let resp = self.client
.delete(&url) .delete(&url)
.basic_auth(self._username.as_str(), Some(self._password.as_str())) .basic_auth(self.username.as_str(), Some(self.password.as_str()))
.send() .send()
.await?; .await?;
@ -183,9 +183,9 @@ impl WebDavClient {
/// Create a directory via MKCOL. /// Create a directory via MKCOL.
pub async fn create_dir(&self, path: &str) -> Result<()> { pub async fn create_dir(&self, path: &str) -> Result<()> {
let url = self.full_url(path); let url = self.full_url(path);
let resp = self._client let resp = self.client
.request(reqwest::Method::from_bytes(b"MKCOL").expect("MKCOL is a valid HTTP method"), &url) .request(reqwest::Method::from_bytes(b"MKCOL").expect("MKCOL is a valid HTTP method"), &url)
.basic_auth(self._username.as_str(), Some(self._password.as_str())) .basic_auth(self.username.as_str(), Some(self.password.as_str()))
.send() .send()
.await?; .await?;
@ -203,9 +203,9 @@ impl WebDavClient {
pub async fn move_resource(&self, from: &str, to: &str) -> Result<()> { pub async fn move_resource(&self, from: &str, to: &str) -> Result<()> {
let from_url = self.full_url(from); let from_url = self.full_url(from);
let to_url = self.full_url(to); let to_url = self.full_url(to);
let resp = self._client let resp = self.client
.request(reqwest::Method::from_bytes(b"MOVE").expect("MOVE is a valid HTTP method"), &from_url) .request(reqwest::Method::from_bytes(b"MOVE").expect("MOVE is a valid HTTP method"), &from_url)
.basic_auth(self._username.as_str(), Some(self._password.as_str())) .basic_auth(self.username.as_str(), Some(self.password.as_str()))
.header("Destination", &to_url) .header("Destination", &to_url)
.header("Overwrite", "F") .header("Overwrite", "F")
.send() .send()
@ -448,12 +448,12 @@ pub fn store_credentials(domain: &str, username: &str, password: &str) -> Result
let user_entry = keyring::Entry::new(&service, "username") let user_entry = keyring::Entry::new(&service, "username")
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?; .map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
user_entry.set_password(username) user_entry.setpassword(username)
.map_err(|e| Error::Credential(format!("Failed to store username: {}", e)))?; .map_err(|e| Error::Credential(format!("Failed to store username: {}", e)))?;
let pass_entry = keyring::Entry::new(&scoped_service, "password") let pass_entry = keyring::Entry::new(&scoped_service, "password")
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?; .map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
pass_entry.set_password(password) pass_entry.setpassword(password)
.map_err(|e| Error::Credential(format!("Failed to store password: {}", e)))?; .map_err(|e| Error::Credential(format!("Failed to store password: {}", e)))?;
// Clean up legacy unscoped password entry if present // Clean up legacy unscoped password entry if present
@ -478,18 +478,18 @@ pub fn load_credentials(domain: &str) -> Result<(Zeroizing<String>, Zeroizing<St
let user_entry = keyring::Entry::new(&service, "username") let user_entry = keyring::Entry::new(&service, "username")
.map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?; .map_err(|e| Error::Credential(format!("Failed to create keyring entry: {}", e)))?;
if let Ok(user) = user_entry.get_password() { if let Ok(user) = user_entry.getpassword() {
// Try scoped password key first (domain+username), fall back to legacy unscoped key // Try scoped password key first (domain+username), fall back to legacy unscoped key
let scoped_service = format!("com.onyx.webdav.{}::{}", domain, user); let scoped_service = format!("com.onyx.webdav.{}::{}", domain, user);
let found = keyring::Entry::new(&scoped_service, "password") let found = keyring::Entry::new(&scoped_service, "password")
.ok() .ok()
.and_then(|e| e.get_password().ok()) .and_then(|e| e.getpassword().ok())
.map(|p| (p, false)) .map(|p| (p, false))
.or_else(|| { .or_else(|| {
// Migration fallback: try legacy unscoped password entry // Migration fallback: try legacy unscoped password entry
keyring::Entry::new(&service, "password") keyring::Entry::new(&service, "password")
.ok() .ok()
.and_then(|e| e.get_password().ok()) .and_then(|e| e.getpassword().ok())
.map(|p| (p, true)) .map(|p| (p, true))
}); });
@ -497,7 +497,7 @@ pub fn load_credentials(domain: &str) -> Result<(Zeroizing<String>, Zeroizing<St
// Auto-migrate legacy credentials to scoped format // Auto-migrate legacy credentials to scoped format
if needs_migration { if needs_migration {
if let Ok(entry) = keyring::Entry::new(&scoped_service, "password") { if let Ok(entry) = keyring::Entry::new(&scoped_service, "password") {
let _ = entry.set_password(&pass); let _ = entry.setpassword(&pass);
} }
if let Ok(legacy) = keyring::Entry::new(&service, "password") { if let Ok(legacy) = keyring::Entry::new(&service, "password") {
let _ = legacy.delete_credential(); let _ = legacy.delete_credential();
@ -547,7 +547,7 @@ pub fn delete_credentials(domain: &str) -> Result<()> {
// Load username first so we can delete the scoped password entry // Load username first so we can delete the scoped password entry
let username = keyring::Entry::new(&service, "username") let username = keyring::Entry::new(&service, "username")
.ok() .ok()
.and_then(|e| e.get_password().ok()); .and_then(|e| e.getpassword().ok());
if let Some(user) = &username { if let Some(user) = &username {
let scoped_service = format!("com.onyx.webdav.{}::{}", domain, user); let scoped_service = format!("com.onyx.webdav.{}::{}", domain, user);