fix: (regression 0.52.1) APIs create_folder rename move_files comment won't work if the filename contains %
Massimo Melina committed
Jan 22, 2026 at 22:13 UTC
30a984dc8f23464ea559204af75beb23511b3efb
3 files changed
+37
-9
src/cross.ts
+1
-1
@@ -460,7 +460,7 @@ export async function promiseBestEffort<T>(promises: Promise<T>[]) {
460
export function pathEncode(s: string) {
461
return s.replace(/[:&#'"% ?\\]/g, escape) // escape() is not utf8, but we are encoding only ascii chars
462
}
463
-//unused function pathDecode(s: string) { return decodeURI(s).replace(/%23/g, '#') }
463
+export function pathDecode(s: string) { return decodeURI(s).replace(/%23/g, '#') }
464
465
// run at a specific point in time, also solving the limit of setTimeout, which doesn't work with +32bit delays
466
export function runAt(ts: number, cb: Callback) {
src/frontEndApis.ts
+11
-6
@@ -17,7 +17,7 @@ import fs from 'fs'
17
import { mkdir, rename, copyFile, unlink } from 'fs/promises'
18
import { basename, dirname, join } from 'path'
19
import { getUploadMeta } from './upload'
20
-import { apiAssertTypes, popKey } from './misc'
20
+import { apiAssertTypes, pathDecode, pathEncode, popKey } from './misc'
21
import { getCommentFor, setCommentFor } from './comments'
22
import { SendListReadable } from './SendList'
23
import { ctxAdminAccess } from './adminApis'
@@ -62,7 +62,8 @@ export const frontEndApis: ApiHandlers = {
62
63
async create_folder({ uri, name }, ctx) {
64
apiAssertTypes({ string: { uri, name } })
65
- ctx.logExtra(null, { name, target: decodeURI(uri) })
65
+ try { ctx.logExtra(null, { name, target: pathDecode(uri) }) }
66
+ catch { return new ApiError(HTTP_BAD_REQUEST) }
67
if (!isValidFileName(name))
68
return new ApiError(HTTP_BAD_REQUEST, 'bad name')
69
const parentNode = await urlToNode(uri, ctx)
@@ -80,9 +81,11 @@ export const frontEndApis: ApiHandlers = {
81
}
82
},
83
84
+ // dest is not encoded
85
async rename({ uri, dest }, ctx) {
86
apiAssertTypes({ string: { uri, dest } })
85
- ctx.logExtra(null, { target: decodeURI(uri), destination: decodeURI(dest) })
87
+ try { ctx.logExtra(null, { target: pathDecode(uri), destination: dest }) }
88
+ catch { return new ApiError(HTTP_BAD_REQUEST) }
89
const node = await urlToNode(uri, ctx)
90
if (!node)
91
return new ApiError(HTTP_NOT_FOUND)
@@ -92,7 +95,7 @@ export const frontEndApis: ApiHandlers = {
95
return new ApiError(ctx.status)
96
if (!node.source)
97
return new ApiError(HTTP_FAILED_DEPENDENCY)
95
- const destNode = await urlToNode(dest, ctx, node.parent)
98
+ const destNode = await urlToNode(pathEncode(dest), ctx, node.parent)
99
if (destNode && statusCodeForMissingPerm(destNode, 'can_delete', ctx)) // if destination exists, you need delete permission
100
return new ApiError(ctx.status)
101
try {
@@ -112,7 +115,8 @@ export const frontEndApis: ApiHandlers = {
115
116
async move_files({ uri_from, uri_to }, ctx, override) {
117
apiAssertTypes({ array: { uri_from }, string: { uri_to } })
115
- ctx.logExtra(null, { target: uri_from.map(decodeURI), destination: decodeURI(uri_to) })
118
+ try { ctx.logExtra(null, { target: uri_from.map(pathDecode), destination: pathDecode(uri_to) }) }
119
+ catch { return new ApiError(HTTP_BAD_REQUEST) }
120
const destNode = await urlToNode(uri_to, ctx)
121
const err = !destNode ? HTTP_NOT_FOUND
122
: !nodeIsFolder(destNode) ? HTTP_METHOD_NOT_ALLOWED
@@ -154,7 +158,8 @@ export const frontEndApis: ApiHandlers = {
158
159
async comment({ uri, comment }, ctx) {
160
apiAssertTypes({ string: { uri, comment } })
157
- ctx.logExtra(null, { target: decodeURI(uri) })
161
+ try { ctx.logExtra(null, { target: pathDecode(uri) }) }
162
+ catch { return new ApiError(HTTP_BAD_REQUEST) }
163
const node = await urlToNode(uri, ctx)
164
if (!node)
165
return new ApiError(HTTP_NOT_FOUND)
tests/test.ts
+25
-2
@@ -5,10 +5,10 @@ import { createReadStream, existsSync, statfsSync, statSync } from 'fs'
5
import { basename, dirname, resolve } from 'path'
6
import { exec } from 'child_process'
7
import _ from 'lodash'
8
-import { findDefined, randomId, try_, tryJson, UPLOAD_TEMP_HASH, wait } from '../src/cross'
8
+import { findDefined, pathEncode, randomId, try_, tryJson, UPLOAD_TEMP_HASH, wait } from '../src/cross'
9
import { httpStream, stream2string, XRequestOptions } from '../src/util-http'
10
import { ThrottledStream, ThrottleGroup } from '../src/ThrottledStream'
11
-import { mkdir, rm, rename, writeFile } from 'fs/promises'
11
+import { mkdir, rm, rename, writeFile, access } from 'fs/promises'
12
import { Readable } from 'stream'
13
/*
14
import { PORT, srv } from '../src'
@@ -212,6 +212,11 @@ describe('basics', () => {
212
.finally(() => defaultBaseUrl = BASE_URL)
213
})
214
215
+ test('create_folder.bad encoding', reqApi('comment', { uri: '%a' }, 400))
216
+ test('comment.bad encoding', reqApi('comment', { uri: '%a', comment: 'anything' }, 400))
217
+ test('rename.bad encoding', reqApi('rename', { uri: '%a', dest: 'anything' }, 400))
218
+ test('move_files.bad encoding', reqApi('move_files', { uri_from: ['%a'], uri_to: '%a' }, 400))
219
+
220
test('folder size', reqApi('get_folder_size', { uri: 'f1/page' }, res => res.bytes === 6328 ))
221
test('folder size.cant', reqApi('get_folder_size', { uri: 'for-admins' }, 401))
222
@@ -242,8 +247,15 @@ describe('accounts', () => {
247
248
describe('after-login', () => {
249
before(() => login(username))
250
+ const trickyChars = '%strange#'
251
test('create_folder', reqApi('create_folder', { uri: UPLOAD_ROOT, name: 'temp' }, 200))
252
test('create_folder.empty name', reqApi('create_folder', { uri: UPLOAD_ROOT, name: '' }, 409))
253
+ test('create_folder.tricky chars', async () => {
254
+ await reqApi('create_folder', { uri: UPLOAD_ROOT, name: trickyChars }, 200)()
255
+ const dest = resolve(__dirname, trickyChars)
256
+ await access(dest)
257
+ await rm(dest, { recursive: true })
258
+ })
259
test('inherit.perm', reqList('/for-admins/', { inList:['alfa.txt'] }))
260
test('inherit.disabled', reqList('/for-disabled/', 401))
261
test('rename.to existing folder', async () => {
@@ -390,6 +402,17 @@ describe('after-login', () => {
402
test('reupload', reqUpload(UPLOAD_DEST, 200))
403
test('delete.method', req(UPLOAD_DEST, 200, { method: 'DELETE' }))
404
test('delete.miss deleted', req(UPLOAD_DEST, 404, { method: 'delete' }))
405
+ test('rename.tricky chars', async () => {
406
+ const dest = trickyChars
407
+ await mkdir(resolve(__dirname, UPLOAD_DIR), { recursive: true })
408
+ const fn = resolve(__dirname, UPLOAD_RELATIVE)
409
+ await writeFile(fn, 'z')
410
+ try {
411
+ await reqApi('rename', { uri: UPLOAD_DEST, dest }, 200)() // dest is not encoded
412
+ await reqApi('rename', { uri: dirname(UPLOAD_DEST) + '/' + pathEncode(dest), dest: basename(UPLOAD_DEST) }, 200)()
413
+ }
414
+ finally { await rm(fn) }
415
+ })
416
const declaredSize = BIG_CONTENT.length / 2
417
test('upload.too much', reqUpload(UPLOAD_DEST, (x,res)=> {
418
if (res.statusCode === 400) return // status 400 is caused by nodejs itself, intercepting the mismatch, but it's probably an unreliable race condition