fix: faulty webdav rename with some chars

Massimo Melina committed Mar 19, 2026 at 10:26 UTC bb9f805d3fd0bca47234e9cfe9c577022f78563f
2 files changed +40 -6
src/webdav.ts
+6 -2
@@ -6,7 +6,7 @@ import {
6 import {
7 HTTP_BAD_REQUEST, HTTP_CONFLICT, HTTP_CREATED, HTTP_METHOD_NOT_ALLOWED, HTTP_NO_CONTENT, HTTP_NOT_FOUND,
8 HTTP_PRECONDITION_FAILED, HTTP_SERVER_ERROR, HTTP_UNAUTHORIZED, HTTP_LOCKED, HTTP_FORBIDDEN,
9 - DAY, CFG, enforceFinal, pathEncode, prefix, getOrSet, Dict, Timeout, join as crossJoin, try_
9 + DAY, CFG, enforceFinal, pathEncode, prefix, getOrSet, Dict, Timeout, join as crossJoin, try_, safeDecodeURIComponent
10 } from './cross'
11 import { PassThrough } from 'stream'
12 import { mkdir, rm } from 'fs/promises'
@@ -135,7 +135,11 @@ export async function handledWebdav(ctx: Koa.Context) {
135 if (isLocked(dest, ctx)) return true
136 if (dirname(path) === dirname(dest)) // rename case. `path` is is encoded, so we test before decoding `dest`
137 try {
138 - await requestedRename(node, basename(decodeURI(dest)), ctx)
138 + // decode the single path segment so reserved chars like %2C become their real name on rename
139 + const newName = safeDecodeURIComponent(basename(dest), '')
140 + if (!newName)
141 + return ctx.status = HTTP_BAD_REQUEST
142 + await requestedRename(node, newName, ctx)
143 return ctx.status = HTTP_CREATED
144 }
145 catch(e:any) {
tests/test.ts
+34 -4
@@ -382,6 +382,36 @@ describe('webdav', () => {
382 await rmAny(destPath)
383 }
384 })
385 + test('webdav.move rename decodes escaped segment chars', async () => {
386 + for (const marker of [',', '#', '%']) {
387 + const name = `wd-move-${randomId(6)}.txt`
388 + const uri = `${UPLOAD_ROOT}${UPLOAD_DIR}/${name}`
389 + const renamedName = name.replace('.txt', `${marker}renamed.txt`)
390 + const renamed = `${UPLOAD_ROOT}${UPLOAD_DIR}/${pathEncode(renamedName)}`
391 + const destination = `${UPLOAD_ROOT}${UPLOAD_DIR}/${encodeURIComponent(renamedName)}`
392 + let destPath = ''
393 + try {
394 + destPath = await webdavUpload(uri, x => x?.uri === uri, 'test')()
395 + await req(uri, 201, {
396 + method: 'MOVE',
397 + auth,
398 + jar,
399 + headers: {
400 + destination: BASE_URL + destination,
401 + overwrite: 'F',
402 + 'user-agent': WEBDAV_UA,
403 + },
404 + })()
405 + await req(uri, 404)()
406 + await req(renamed, 200, { auth })()
407 + }
408 + finally {
409 + await rmAny(uploadUriToPath(uri))
410 + await rmAny(uploadUriToPath(renamed))
411 + await rmAny(destPath)
412 + }
413 + }
414 + })
415 test('webdav.escaping', req('/f1/hidden', data => XMLValidator.validate(data) === true, { method: 'PROPFIND', auth, jar: {}, headers: { depth: '1' } }))
416
417 function webdavUpload(uri: string, tester: Tester, body: string, userAgent=WEBDAV_UA) {
@@ -426,11 +456,11 @@ describe('webdav', () => {
456
457 })
458
429 -// do this before login, or max_dl_accounts config will override max_dl
459 +// do this before login, or max_dl.accounts config will override max_dl
460 describe('limits', () => {
461 const fn = ROOT + 'big'
462 before(() => writeFile(fn, BIG_CONTENT))
433 - test('max_dl', () => testMaxDl('/' + fn, 1, 2))
463 + test('max_dl', () => testMaxDl('/' + fn, 1, 2, { jar: {} }))
464 after(() => rm(fn))
465 })
466
@@ -926,7 +956,7 @@ function uploadUriToPath(uri: string) {
956 return ROOT + decodeURI(uri).replace(UPLOAD_ROOT, '')
957 }
958
929 -async function testMaxDl(uri: string, good: number, bad: number) {
959 +async function testMaxDl(uri: string, good: number, bad: number, reqOptions: ReqOptions={}) {
960 // make good+bad requests, and check results
961 await Promise.all(_.range(good + bad).map(i => req(uri + '?' + i, (_data, res) => {
962 if (res.statusCode === 429) {
@@ -940,7 +970,7 @@ async function testMaxDl(uri: string, good: number, bad: number) {
970 return
971 }
972 throw "unexpected status " + res.statusCode
943 - }, { throttle })() )) // slow down to ensure the attempted downloads are all concurrent
973 + }, { throttle, ...reqOptions })() )) // slow down to ensure the attempted downloads are all concurrent
974 }
975
976 type TesterFunction = ((data: any, fullResponse: any) => boolean | void) // true or void for ok, false or throw for error