@samitouri / QOSami-HFS / commits / c2653dcc

fix: faulty webdav locks

Massimo Melina committed Mar 19, 2026 at 22:19 UTC c2653dcc2a252f387d03268cfdeb9bf87cbc3878
3 files changed +113 -14
src/serveGuiAndSharedFiles.ts
+3 -1
@@ -29,7 +29,7 @@ import { setCommentFor } from './comments'
29 import { basicWeb, detectBasicAgent } from './basicWeb'
30 import { customizedIcons, ICONS_FOLDER } from './icons'
31 import { getPluginInfo } from './plugins'
32 -import { handledWebdav } from './webdav'
32 +import { handledWebdav, releaseWebdavLock } from './webdav'
33
34 const serveFrontendFiles = serveGuiFiles(process.env.FRONTEND_PROXY, FRONTEND_URI)
35 const serveFrontendPrefixed = mount(FRONTEND_URI.slice(0,-1), serveFrontendFiles)
@@ -108,6 +108,8 @@ export const serveGuiAndSharedFiles: Koa.Middleware = async (ctx, next) => {
108 if ((await events.emitAsync('deleting', { node, ctx }))?.isDefaultPrevented())
109 return ctx.status = HTTP_FAILED_DEPENDENCY
110 await rm(source, { recursive: true })
111 + // webdav clients may forget UNLOCK on failures; successful delete must clear any lock tied to this path
112 + releaseWebdavLock(ctx.path)
113 void setCommentFor(source, '') // necessary only to clean a possible descript.ion or kvstorage
114 return ctx.status = HTTP_OK
115 } catch (e: any) {
src/webdav.ts
+42 -13
@@ -36,14 +36,27 @@ const LOCK_MAX_SECONDS = DAY / 1000
36 const xmlParser = new XMLParser({ ignoreAttributes: false, removeNSPrefix: true, trimValues: true })
37
38 const canOverwrite = new Set<string>()
39 -const locks = new Map<string, { token: string, timeout: NodeJS.Timeout, seconds: number }>()
39 +const locks = new Map<string, { token: string, timeout: NodeJS.Timeout, seconds: number, principal: string }>()
40
41 -function isLocked(path: string, ctx: Koa.Context) {
41 +export function releaseWebdavLock(path: string) {
42 const lock = locks.get(path)
43 if (!lock) return false
44 + clearTimeout(lock.timeout)
45 + locks.delete(path)
46 + return true
47 +}
48 +
49 +async function isLocked(path: string, ctx: Koa.Context) {
50 + const lock = locks.get(path)
51 + if (!lock) return false
52 + // if the resource is gone, keeping the lock only creates fake 423 responses
53 + if (!await urlToNode(path, ctx)) {
54 + releaseWebdavLock(path)
55 + return false
56 + }
57 const ifHeader = ctx.get('If')
58 const tokenHeader = ctx.get(TOKEN_HEADER)
46 - if (hasToken(ifHeader, lock.token) || hasToken(tokenHeader, lock.token))
59 + if (isSameLockPrincipal(lock, ctx) && (hasToken(ifHeader, lock.token) || hasToken(tokenHeader, lock.token)))
60 return false
61 ctx.status = HTTP_LOCKED
62 return true
@@ -54,6 +67,14 @@ function hasToken(header: string, token: string) {
67 return header.includes(`<${token}>`) || header.split(/[,;\s]+/).includes(token)
68 }
69
70 +function getWebdavPrincipal(ctx: Koa.Context) {
71 + return getCurrentUsername(ctx) || ''
72 +}
73 +
74 +function isSameLockPrincipal(lock: { principal: string }, ctx: Koa.Context) {
75 + return lock.principal === getWebdavPrincipal(ctx)
76 +}
77 +
78 export async function handledWebdav(ctx: Koa.Context) {
79 let {path} = ctx
80 path = path.replace(/^\/+/, '/') // double-slash is causing empty listing in filezilla-pro
@@ -76,7 +97,7 @@ export async function handledWebdav(ctx: Koa.Context) {
97 if (isWebdavAuthRequest && shouldChallengeWebdav())
98 return true
99 if (ctx.method === 'PUT') {
79 - if (isLocked(path, ctx)) return true
100 + if (await isLocked(path, ctx)) return true
101 const overwriteGraceKey = path + prefix('|', getCurrentUsername(ctx)) // bind temporary overwrite grace to the authenticated user so accounts cannot reuse each other's grace window
102 // Finder first creates an empty file (a test?) then wants to overwrite it, which requires deletion permission, but the user may not have it, causing a renamed upload. To solve, so we give it special permission for a few seconds.
103 const x = ctx.get('x-expected-entity-length') // field used by Finder's webdav on actual upload, after
@@ -99,7 +120,7 @@ export async function handledWebdav(ctx: Koa.Context) {
120 }
121 if (ctx.method === 'MKCOL') {
122 setWebdavHeaders()
102 - if (isLocked(path, ctx)) return true
123 + if (await isLocked(path, ctx)) return true
124 const node = await urlToNode(path, ctx)
125 if (node)
126 return ctx.status = HTTP_METHOD_NOT_ALLOWED
@@ -124,7 +145,7 @@ export async function handledWebdav(ctx: Koa.Context) {
145 }
146 if (ctx.method === 'MOVE') {
147 setWebdavHeaders()
127 - if (isLocked(path, ctx)) return true
148 + if (await isLocked(path, ctx)) return true
149 const node = await urlToNode(path, ctx)
150 if (!node) return
151 let dest = ctx.get('destination')
@@ -132,7 +153,7 @@ export async function handledWebdav(ctx: Koa.Context) {
153 if (i >= 0)
154 dest = dest.slice(dest.indexOf('/', i + 2))
155 dest = crossJoin(ctx.state.root || '', dest) // on Windows, we must use / as the delimiter to be able to compare with `path` below
135 - if (isLocked(dest, ctx)) return true
156 + if (await isLocked(dest, ctx)) return true
157 if (dirname(path) === dirname(dest)) // rename case. `path` is is encoded, so we test before decoding `dest`
158 try {
159 // decode the single path segment so reserved chars like %2C become their real name on rename
@@ -140,6 +161,7 @@ export async function handledWebdav(ctx: Koa.Context) {
161 if (!newName)
162 return ctx.status = HTTP_BAD_REQUEST
163 await requestedRename(node, newName, ctx)
164 + releaseWebdavLock(path) // RFC 4918 says MOVE must not carry locks to destination, so clear source lock on success
165 return ctx.status = HTTP_CREATED
166 }
167 catch(e:any) {
@@ -149,11 +171,13 @@ export async function handledWebdav(ctx: Koa.Context) {
171 if (moveRes instanceof Error)
172 return ctx.status = (moveRes as any).status || HTTP_SERVER_ERROR
173 const err = moveRes?.errors?.[0]
174 + if (!err)
175 + releaseWebdavLock(path) // successful move leaves old path invalid, therefore its lock must be dropped
176 return ctx.status = !err ? HTTP_CREATED : typeof err === 'number' ? err : HTTP_SERVER_ERROR
177 }
178 if (ctx.method === 'DELETE') {
179 setWebdavHeaders()
156 - if (isLocked(path, ctx)) return true
180 + if (await isLocked(path, ctx)) return true
181 return // allow default handling in serveGuiAndSharedFiles.ts
182 }
183 if (ctx.method === 'UNLOCK') {
@@ -162,8 +186,10 @@ export async function handledWebdav(ctx: Koa.Context) {
186 const lock = locks.get(path)
187 if (x !== lock?.token)
188 return ctx.status = HTTP_BAD_REQUEST
165 - clearTimeout(lock.timeout)
166 - locks.delete(path)
189 + // with force_webdav_login disabled a client may silently fall back to anonymous; keep lock ownership on the original principal
190 + if (!isSameLockPrincipal(lock, ctx))
191 + return ctx.status = HTTP_PRECONDITION_FAILED
192 + releaseWebdavLock(path)
193 ctx.set(TOKEN_HEADER, x)
194 if (IS_MAC)
195 urlToNode(path, ctx).then(x => x?.source && dotClean(dirname(x.source)))
@@ -183,9 +209,12 @@ export async function handledWebdav(ctx: Koa.Context) {
209 const lock = locks.get(path)
210 if (token !== lock?.token)
211 return ctx.status = HTTP_PRECONDITION_FAILED
212 + // same-token refresh from another principal would make abandoned locks effectively persistent
213 + if (!isSameLockPrincipal(lock, ctx))
214 + return ctx.status = HTTP_PRECONDITION_FAILED
215 // refresh lock – keep the same token on refresh so clients can continue using the lock they already hold
216 clearTimeout(lock.timeout)
188 - lock.timeout = setTimeout(() => locks.delete(path), seconds * 1000)
217 + lock.timeout = setTimeout(() => releaseWebdavLock(path), seconds * 1000)
218 lock.seconds = seconds
219 locks.set(path, lock)
220
@@ -205,8 +234,8 @@ export async function handledWebdav(ctx: Koa.Context) {
234 if (locks.has(path))
235 return ctx.status = HTTP_LOCKED
236 const newToken = 'urn:uuid:' + randomUUID()
208 - const timeout = setTimeout(() => locks.delete(path), seconds * 1000)
209 - locks.set(path, { token: newToken, timeout, seconds })
237 + const timeout = setTimeout(() => releaseWebdavLock(path), seconds * 1000)
238 + locks.set(path, { token: newToken, timeout, seconds, principal: getWebdavPrincipal(ctx) })
239 ctx.set(TOKEN_HEADER, newToken)
240 ctx.body = renderLockResponse(newToken, seconds)
241 return true
tests/test.ts
+68
@@ -382,6 +382,74 @@ describe('webdav', () => {
382 await rmAny(destPath)
383 }
384 })
385 + test('webdav.stale lock on missing resource is pruned', async () => {
386 + const name = `wd-stale-lock-${randomId(6)}.txt`
387 + const uri = `${UPLOAD_ROOT}${UPLOAD_DIR}/${name}`
388 + let destPath = ''
389 + try {
390 + destPath = await webdavUpload(uri, x => x?.uri === uri, 'test')()
391 + await webdavLock(uri)()
392 + // simulate external removal while client forgot to unlock: stale lock must not force 423 forever
393 + await rmAny(destPath)
394 + await req(uri, 404, { method: 'DELETE', auth, jar, headers: { 'user-agent': WEBDAV_UA } })()
395 + await req(uri, 404, { method: 'DELETE', auth, jar, headers: { 'user-agent': WEBDAV_UA } })()
396 + }
397 + finally {
398 + await rmAny(destPath)
399 + }
400 + })
401 + test('webdav.delete success clears lock for same path', async () => {
402 + const name = `wd-delete-clears-lock-${randomId(6)}.txt`
403 + const uri = `${UPLOAD_ROOT}${UPLOAD_DIR}/${name}`
404 + let destPath = ''
405 + let token = ''
406 + try {
407 + destPath = await webdavUpload(uri, x => x?.uri === uri, 'test')()
408 + await webdavLock(uri, (_data, res) => token = res.headers?.[TOKEN_HEADER] || '')()
409 + if (!token)
410 + throw "missing lock token"
411 + await req(uri, 200, { method: 'DELETE', auth, jar, headers: { If: `(<${token}>)`, 'user-agent': WEBDAV_UA } })()
412 + destPath = await webdavUpload(uri, x => x?.uri === uri, 'test2')()
413 + await req(uri, 200, { method: 'DELETE', auth, jar, headers: { 'user-agent': WEBDAV_UA } })()
414 + }
415 + finally {
416 + await rmAny(destPath)
417 + }
418 + })
419 + test('webdav.move success clears lock state', async () => {
420 + const name = `wd-move-clears-lock-${randomId(6)}.txt`
421 + const uri = `${UPLOAD_ROOT}${UPLOAD_DIR}/${name}`
422 + const renamedName = name.replace('.txt', '-renamed.txt')
423 + const renamed = `${UPLOAD_ROOT}${UPLOAD_DIR}/${renamedName}`
424 + let destPath = ''
425 + let renamedPath = ''
426 + let token = ''
427 + try {
428 + destPath = await webdavUpload(uri, x => x?.uri === uri, 'test')()
429 + await webdavLock(uri, (_data, res) => token = res.headers?.[TOKEN_HEADER] || '')()
430 + if (!token)
431 + throw "missing lock token"
432 + await req(uri, 201, {
433 + method: 'MOVE',
434 + auth,
435 + jar,
436 + headers: {
437 + destination: BASE_URL + renamed,
438 + overwrite: 'F',
439 + If: `(<${token}>)`,
440 + 'user-agent': WEBDAV_UA,
441 + },
442 + })()
443 + renamedPath = uploadUriToPath(renamed)
444 + await req(renamed, 200, { method: 'DELETE', auth, jar, headers: { 'user-agent': WEBDAV_UA } })()
445 + destPath = await webdavUpload(uri, x => x?.uri === uri, 'test2')()
446 + await req(uri, 200, { method: 'DELETE', auth, jar, headers: { 'user-agent': WEBDAV_UA } })()
447 + }
448 + finally {
449 + await rmAny(renamedPath)
450 + await rmAny(destPath)
451 + }
452 + })
453 test('webdav.move rename decodes escaped segment chars', async () => {
454 for (const marker of [',', '#', '%']) {
455 const name = `wd-move-${randomId(6)}.txt`