fix: middlewares called too early on upload

Massimo Melina committed May 22, 2024 at 15:13 UTC 95ce81417490b0e29f2a8525df1c0376dcf913c6
2 files changed +49 -31
src/serveGuiAndSharedFiles.ts
+10 -5
@@ -48,7 +48,8 @@ export const serveGuiAndSharedFiles: Koa.Middleware = async (ctx, next) => {
48 ctx.state.uploadPath = decPath
49 const dest = uploadWriter(folder, rest, ctx)
50 if (dest) {
51 - await pipeline(ctx.req, dest)
51 + void pipeline(ctx.req, dest)
52 + await dest.lockMiddleware // we need to wait more than just the stream
53 ctx.body = {}
54 }
55 return
@@ -63,6 +64,7 @@ export const serveGuiAndSharedFiles: Koa.Middleware = async (ctx, next) => {
64 return ctx.status = HTTP_BAD_REQUEST
65 ctx.body = {}
66 ctx.state.uploads = []
67 + let locks: Promise<any>[] = []
68 const form = formidable({
69 maxFileSize: Infinity,
70 allowEmptyFiles: true,
@@ -70,13 +72,16 @@ export const serveGuiAndSharedFiles: Koa.Middleware = async (ctx, next) => {
72 const fn = (f as any).originalFilename
73 ctx.state.uploadPath = decodeURI(ctx.path) + fn
74 ctx.state.uploads!.push(fn)
73 - return uploadWriter(node!, fn, ctx)
74 - || new Writable({ write(data,enc,cb) { cb() } }) // just discard data
75 + const ret = uploadWriter(node!, fn, ctx)
76 + if (!ret)
77 + return new Writable({ write(data,enc,cb) { cb() } }) // just discard data
78 + locks.push(ret.lockMiddleware)
79 + return ret
80 }
81 })
77 - return new Promise<void>(res => form.parse(ctx.req, err => {
82 + return new Promise<any>(res => form.parse(ctx.req, err => {
83 if (err) console.error(String(err))
79 - res()
84 + res(Promise.all(locks))
85 }))
86 }
87 const { get } = ctx.query
src/upload.ts
+39 -26
@@ -3,7 +3,7 @@ import Koa from 'koa'
3 import { HTTP_CONFLICT, HTTP_FOOL, HTTP_PAYLOAD_TOO_LARGE, HTTP_RANGE_NOT_SATISFIABLE, HTTP_SERVER_ERROR } from './const'
4 import { basename, dirname, extname, join } from 'path'
5 import fs from 'fs'
6 -import { Callback, dirTraversal, escapeHTML, loadFileAttr, storeFileAttr, try_ } from './misc'
6 +import { Callback, dirTraversal, escapeHTML, loadFileAttr, pendingPromise, storeFileAttr, try_ } from './misc'
7 import { notifyClient } from './frontEndApis'
8 import { defineConfig } from './config'
9 import { getDiskSpaceSync } from './util-os'
@@ -13,6 +13,7 @@ import { getCurrentUsername } from './auth'
13 import { setCommentFor } from './comments'
14 import _ from 'lodash'
15 import events from './events'
16 +import { rename } from 'fs/promises'
17
18 export const deleteUnfinishedUploadsAfter = defineConfig<undefined|number>('delete_unfinished_uploads_after', 86_400)
19 export const minAvailableMb = defineConfig('min_available_mb', 100)
@@ -101,35 +102,47 @@ export function uploadWriter(base: VfsNode, path: string, ctx: Koa.Context) {
102 cancelDeletion(tempName)
103 ctx.state.uploadDestinationPath = tempName
104 trackProgress()
105 + const obj = { ctx, writeStream }
106 + events.emit('uploadStart', obj)
107 + const lockMiddleware = pendingPromise()
108 writeStream.once('close', async () => {
105 - if (ctx.req.aborted) {
106 - if (resumable) // we don't want to be left with 2 temp files
107 - return delayedDelete(tempName, 0)
108 - const sec = deleteUnfinishedUploadsAfter.get()
109 - return _.isNumber(sec) && delayedDelete(tempName, sec)
109 + try {
110 + if (ctx.req.aborted) {
111 + if (resumable) // we don't want to be left with 2 temp files
112 + return delayedDelete(tempName, 0)
113 + const sec = deleteUnfinishedUploadsAfter.get()
114 + return _.isNumber(sec) && delayedDelete(tempName, sec)
115 + }
116 + let dest = fullPath
117 + if (dontOverwriteUploading.get() && !await overwriteAnyway() && fs.existsSync(dest)) {
118 + const ext = extname(dest)
119 + const base = dest.slice(0, -ext.length || Infinity)
120 + let i = 1
121 + do dest = `${base} (${i++})${ext}`
122 + while (fs.existsSync(dest))
123 + }
124 + try {
125 + await rename(tempName, dest)
126 + ctx.state.uploadDestinationPath = dest
127 + setUploadMeta(dest, ctx)
128 + if (ctx.query.comment)
129 + setCommentFor(dest, escapeHTML(String(ctx.query.comment)))
130 + if (resumable)
131 + delayedDelete(resumable, 0)
132 + events.emit('uploadFinished', obj)
133 + }
134 + catch (err: any) {
135 + setUploadMeta(tempName, ctx)
136 + console.error("couldn't rename temp to", dest, String(err))
137 + }
138 }
111 - let dest = fullPath
112 - if (dontOverwriteUploading.get() && fs.existsSync(dest) && !await overwriteAnyway()) {
113 - const ext = extname(dest)
114 - const base = dest.slice(0, -ext.length || Infinity)
115 - let i = 1
116 - do dest = `${base} (${i++})${ext}`
117 - while (fs.existsSync(dest))
139 + finally {
140 + lockMiddleware.resolve()
141 }
119 - ctx.state.uploadDestinationPath = dest
120 - return fs.rename(tempName, dest, err => {
121 - setUploadMeta(err ? tempName : dest, ctx)
122 - if (err)
123 - console.error("couldn't rename temp to", dest, String(err))
124 - else if (ctx.query.comment)
125 - setCommentFor(dest, escapeHTML(String(ctx.query.comment)))
126 - if (resumable)
127 - delayedDelete(resumable, 0)
128 - })
142 })
130 - const obj = { ctx, writeStream }
131 - events.emit('uploadStart', obj)
132 - return obj.writeStream
143 + return Object.assign(obj.writeStream, {
144 + lockMiddleware
145 + })
146
147 async function overwriteAnyway() {
148 if (ctx.query.overwrite === undefined // legacy pre-0.52