expressjs / serve-static

Serve static files
MIT License
1.38k stars 227 forks source link

Corrupted file served if file is changed during serving. #155

Open wdanilo opened 1 year ago

wdanilo commented 1 year ago

Hi, I've got such a code for my server:

import path from 'node:path'
import url from 'node:url'
import portfinder from 'portfinder'
import connect from 'connect'
import { WebSocketServer } from 'ws'
import serveStatic from 'serve-static'
import logger from 'morgan'

export const DEFAULT_PORT = 8080

let dirname = path.dirname(url.fileURLToPath(import.meta.url))
// Path of a file that needs to be injected into the bundle for live-reload to work.
export const LIVE_RELOAD_LISTENER_PATH = path.join(dirname, 'live-reload.js')

export async function start({ root, assets, port }) {
    assets = assets ?? path.join(root, 'assets')

    const freePort = await portfinder.getPortPromise({ port: port ?? DEFAULT_PORT })

    const setHeaders = (res) => {
        res.setHeader('Cache-Control', 'no-store')
    }

    const app = connect()
        .use(logger('dev', { skip: (req, res) => res.statusCode < 400 }))
        .use(serveStatic(root, {setHeaders}))
        .use('/assets', serveStatic(assets, {setHeaders}))

    const server = app.listen(freePort)
    const wsServer = new WebSocketServer({ server, clientTracking: true, path: '/live-reload' })

    var serverUrl = `http://localhost:${freePort}`
    console.log('Serving %s', serverUrl)

    return {
        port: freePort,
        reload() {
            wsServer.clients.forEach(sock => sock.send('reload'))
        },
    }
}

And we have observed a very interesting behavior. Namely, when it is serving our WASM file (100MB+), if the WASM is being re-build just when serving starts, then it is served to the browser corrupted. What's interesting is that after it is rebuilt, browser refreshes dont help - the same corrupted file is being served. What helps is restarting the above server to serve non-corrupted file. Does anyone have any idea why this happens and if this can be prevented somehow? I'm OK with serving it in a corrupted state for the first time (during rebuild), but I want the file to be served correctly after page reload.

dougwilson commented 1 year ago

Hi @wdanilo that is strange indeed! This module doesn't perform any type of internal caching of anything, from the file content to any other metadata. Each request for a given file is looked up in the file system and read a-new every time. The reading is done using the fs.createReadStream Node.js API (https://nodejs.org/docs/latest-v17.x/api/fs.html#fscreatereadstreampath-options) and the data from that stream (created new for every request) is directed passed to the Node.js HTTP response object (https://nodejs.org/docs/latest-v17.x/api/http.html#class-httpserverresponse).

I could try and help debug the cause if I can get the exact code and steps to reproduce the issue, though.

wdanilo commented 1 year ago

@dougwilson thanks so much for fast reply, so detailed answer, and that you are willing to help us. I appreciate it a lot! Unfortunately, this issue is not easily reproducible on our end, as it happens sometimes, when certain conditions are met (we are unsure what are these conditions yet, we have only observed that it happens when the WASM file is being regenerated during serving). We will try to configure the code to reproduce it on every build and we will get back to you :)

autopulated commented 1 year ago

The problem is likely that you are modifying the file in-place on the filesystem when regenerating it. Instead, generate the new version under a different filename, and then move it to replace the old one. This way a single read stream of the file will either get the complete old or complete new version of file.

keithchew commented 10 months ago

Hi @wdanilo @dougwilson

Under the hood, serve-static uses the send package, and there is a race condition in that package, ie if the file contents changes after the fs.stat call but while streaming is still going to the client, the contents received by the client will be corrupted. Reference to the issue here:

https://github.com/pillarjs/send/issues/122

The relevant code snippet:

SendStream.prototype.sendFile = function sendFile (path) {
  ...
  fs.stat(path, function onstat (err, stat) {
    ...
    ***** corruption if file contents changed here *****
    self.send(path, stat)
  })

I am also experiencing this issue, where Express serves a PNG back to the browser. If the PNG content gets overwritten while Chrome is getting the image, it will be appear corrupted on the webpage.

Note: Even if you did a "write to temp file" and atomically rename to destination file (ie what @autopulated suggested), it is still susceptible to the race condition above.