Skip to content

fix: WebDAV 优先走后端代理,解决浏览器 CORS 跨域问题#4

Merged
lostiv merged 1 commit into
mainfrom
fix/webdv
May 8, 2026
Merged

fix: WebDAV 优先走后端代理,解决浏览器 CORS 跨域问题#4
lostiv merged 1 commit into
mainfrom
fix/webdv

Conversation

@lostiv
Copy link
Copy Markdown
Owner

@lostiv lostiv commented May 8, 2026

当后端可用时,WebDAVService 通过 POST /api/proxy/webdav 代理请求, 避免浏览器直接跨域访问 WebDAV 服务器。后端不可用时回退到浏览器直连。

Summary by CodeRabbit

  • New Features

    • Added optional backend proxy support for WebDAV operations, enabling reliable file uploads, downloads, directory creation, and file management while avoiding browser CORS limitations.
  • Improvements

    • Enhanced error handling with consistent response formatting for file operations.
    • Improved XML parsing with fallback mechanisms for better reliability.

当后端可用时,WebDAVService 通过 POST /api/proxy/webdav 代理请求,
避免浏览器直接跨域访问 WebDAV 服务器。后端不可用时回退到浏览器直连。
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

WebDAVService is extended with optional backend proxy routing to bypass browser CORS constraints. A useProxy flag determines whether operations use proxy-delegated requests (proxyFetch) or direct browser calls. Path utilities normalize relative paths, and individual methods (testConnection, uploadFile, ensureDirectoryExists, downloadFile, fileExists, listFiles) now conditionally route through the proxy with HTTP status-specific error handling while preserving direct request fallbacks.

Changes

WebDAV Proxy Support

Layer / File(s) Summary
Proxy Infrastructure & Primitives
src/services/webdavService.ts
Imports backend adapter, introduces useProxy flag, computes normalized basePath, and implements proxyFetch helper for delegating WebDAV requests through backend proxy.
Path Utilities & Error Handling
src/services/webdavService.ts
Adds getFullPath and getRelativePath utilities using basePath for normalized path handling; scaffolds network error handling around handleNetworkError for proxy-aware error mapping.
Connection & Reachability Testing
src/services/webdavService.ts
testConnection prefers proxy HEAD then PROPFIND when proxy enabled, with fallback to direct browser logic; adjusted timeout error handling for fetch failures.
Write Operations
src/services/webdavService.ts
uploadFile routes through proxy PUT with status mapping (401/403/404/507 to errors); ensureDirectoryExists creates directories via proxy MKCOL. Direct flows preserved with expanded error-message matching for authentication/authorization/storage failures.
Read Operations
src/services/webdavService.ts
downloadFile supports proxy GET with JSON/text normalization and status handling (404→null, 401→auth error); fileExists uses proxy HEAD; listFiles prefers proxy PROPFIND with improved abort/timeout error handling.
XML Parsing
src/services/webdavService.ts
Reworked parsePropfindXml to parse DOM first collecting .json filenames, then falls back to regex extraction from displayname and href tags.
Server Information
src/services/webdavService.ts
getServerInfo continues direct OPTIONS request (no proxy integration); returns parsed headers (Server, DAV) or {} on failure.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Whiskers twitching with proxy delight,
A bunny crafts paths that skip CORS blight,
Backend routes flow where browsers can't roam—
Two roads for requests, both leading back home! 🛣️✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: routing WebDAV operations through a backend proxy to resolve browser CORS limitations, which aligns perfectly with the changeset's primary objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/webdv

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/services/webdavService.ts (1)

174-199: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the timeout active for the fallback PROPFIND.

The 10s timer only covers the initial HEAD. If HEAD returns non-OK and the server stalls on PROPFIND, this path can hang indefinitely.

Suggested fix
-      const controller = new AbortController();
-      const timeoutId = setTimeout(() => controller.abort(), 10000);
+      const controller = new AbortController();
+      const timeoutId = setTimeout(() => controller.abort(), 10000);

       try {
         const headResponse = await fetch(dirUrl, {
           method: 'HEAD',
           headers: {
             'Authorization': this.getAuthHeader(),
           },
           signal: controller.signal,
         });

-        clearTimeout(timeoutId);
-
         if (headResponse.ok) return true;

         const propfindResponse = await fetch(dirUrl, {
           method: 'PROPFIND',
           headers: {
             'Authorization': this.getAuthHeader(),
             'Depth': '0',
           },
+          signal: controller.signal,
         });

         return propfindResponse.ok || propfindResponse.status === 207;
       } catch (fetchError: unknown) {
-        clearTimeout(timeoutId);
-
         if ((fetchError as Error).name === 'AbortError') {
           throw new Error('连接超时。请检查WebDAV服务器是否可访问。');
         }

         throw fetchError;
+      } finally {
+        clearTimeout(timeoutId);
       }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/webdavService.ts` around lines 174 - 199, The current timeout
only wraps the initial HEAD request (using controller and timeoutId) but is not
passed to the fallback PROPFIND, so PROPFIND can hang; fix by ensuring the
PROPFIND request also uses an AbortController with the same timeout semantics —
either reuse the existing controller and keep the timeout active until after
both responses (clearTimeout only after PROPFIND completes) or create a new
AbortController/timeout for the PROPFIND; update the fetch call that assigns
propfindResponse to include signal: controller.signal (or the new controller)
and ensure clearTimeout(timeoutId) is called after the final request completes.
🧹 Nitpick comments (1)
src/services/webdavService.ts (1)

619-626: ⚡ Quick win

Route OPTIONS through the proxy too.

This is still a browser-side WebDAV request, so the same CORS setup this PR is fixing will make getServerInfo() return {} even when the proxied operations succeed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/webdavService.ts` around lines 619 - 626, getServerInfo()
currently calls fetch(this.config.url) with method 'OPTIONS' which bypasses the
proxy and will fail CORS; change getServerInfo to use the same proxy routing as
the other WebDAV methods (i.e., send the OPTIONS request to the proxy endpoint
instead of this.config.url and include the original target URL/credentials the
proxy expects — follow the same pattern used elsewhere in this file for proxied
requests) so the OPTIONS call is routed through the proxy and CORS is avoided.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/services/webdavService.ts`:
- Around line 20-26: Create a centralized "proxy then direct" helper to match
testConnection()'s fallback behavior: update proxyFetch to either call a new
helper (e.g., proxyThenDirect) or implement try/catch around backend.proxyWebDAV
calls so uploadFile, downloadFile, fileExists, and listFiles no longer await
proxyFetch directly without handling failures; instead have those methods call
the helper which first attempts backend.proxyWebDAV(this.config.id, ...) and on
any thrown error falls back to the existing direct-browser paths used in this
class (the same direct handlers testConnection() uses), ensuring the helper
returns the Response or result to the calling methods and preserves signature
compatibility with proxyFetch.

---

Outside diff comments:
In `@src/services/webdavService.ts`:
- Around line 174-199: The current timeout only wraps the initial HEAD request
(using controller and timeoutId) but is not passed to the fallback PROPFIND, so
PROPFIND can hang; fix by ensuring the PROPFIND request also uses an
AbortController with the same timeout semantics — either reuse the existing
controller and keep the timeout active until after both responses (clearTimeout
only after PROPFIND completes) or create a new AbortController/timeout for the
PROPFIND; update the fetch call that assigns propfindResponse to include signal:
controller.signal (or the new controller) and ensure clearTimeout(timeoutId) is
called after the final request completes.

---

Nitpick comments:
In `@src/services/webdavService.ts`:
- Around line 619-626: getServerInfo() currently calls fetch(this.config.url)
with method 'OPTIONS' which bypasses the proxy and will fail CORS; change
getServerInfo to use the same proxy routing as the other WebDAV methods (i.e.,
send the OPTIONS request to the proxy endpoint instead of this.config.url and
include the original target URL/credentials the proxy expects — follow the same
pattern used elsewhere in this file for proxied requests) so the OPTIONS call is
routed through the proxy and CORS is avoided.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e2663214-13af-4e78-ba05-61c36e2124e6

📥 Commits

Reviewing files that changed from the base of the PR and between bb3f5c3 and 43b8fca.

📒 Files selected for processing (1)
  • src/services/webdavService.ts

Comment on lines +20 to +26
private async proxyFetch(
method: string,
fullPath: string,
body?: string,
headers?: Record<string, string>,
): Promise<Response> {
return backend.proxyWebDAV(this.config.id, method, fullPath, body, headers);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

backend_file="$(fd -i '^backendAdapter\.ts$' src/services . | head -n 1)"
if [ -z "${backend_file:-}" ]; then
  echo "backendAdapter.ts not found" >&2
  exit 1
fi

echo "=== backend adapter: $backend_file ==="
sed -n '1,260p' "$backend_file"

echo
echo "=== proxyWebDAV / isAvailable references ==="
rg -n -C3 '\bproxyWebDAV\b|\bisAvailable\b' "$backend_file" src/services/webdavService.ts

Repository: lostiv/GithubStarsManager

Length of output: 11228


🏁 Script executed:

wc -l src/services/webdavService.ts

Repository: lostiv/GithubStarsManager

Length of output: 103


🏁 Script executed:

cat -n src/services/webdavService.ts

Repository: lostiv/GithubStarsManager

Length of output: 24021


Inconsistent proxy failure handling — only testConnection() falls back to direct access.

The proxy branches in uploadFile() (lines 237–254), downloadFile() (lines 363–376), fileExists() (lines 430–433), and listFiles() (lines 456–478) all await proxyFetch() directly without try-catch fallback. In contrast, testConnection() wraps its proxy calls in a try-catch that falls back to direct browser access. When backend.proxyWebDAV() fails transiently, these methods throw immediately, even though the direct path exists in the class and would likely succeed.

backend.proxyWebDAV() does not flip backend.isAvailable on failure—it only checks the initial state. Centralize a proxy-then-direct helper pattern here to match testConnection()'s approach.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/webdavService.ts` around lines 20 - 26, Create a centralized
"proxy then direct" helper to match testConnection()'s fallback behavior: update
proxyFetch to either call a new helper (e.g., proxyThenDirect) or implement
try/catch around backend.proxyWebDAV calls so uploadFile, downloadFile,
fileExists, and listFiles no longer await proxyFetch directly without handling
failures; instead have those methods call the helper which first attempts
backend.proxyWebDAV(this.config.id, ...) and on any thrown error falls back to
the existing direct-browser paths used in this class (the same direct handlers
testConnection() uses), ensuring the helper returns the Response or result to
the calling methods and preserves signature compatibility with proxyFetch.

@lostiv lostiv merged commit e6e2c3a into main May 8, 2026
1 check passed
@lostiv lostiv deleted the fix/webdv branch May 8, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant