express icon indicating copy to clipboard operation
express copied to clipboard

response.download does not return /proc/* files

Open mleblebici opened this issue 1 year ago • 10 comments

response.download() returns empty for file under /proc directory like /proc/meminfo, /proc/cpuinfo and others. /proc is a pseudo filesystem, but these files' content can be readable with fs.readFile. I think it should also be readable with response.download and response.sendFile.

mleblebici avatar Jul 01 '22 15:07 mleblebici

Hi @mleblebici yes, I agree they should be able to work. Unfortunately I just use Windows mainly and don't really have experience there on what would be wrong. Express is just using fs.createReadStream under the hood to read them. Would you be willing to make a pull request with a fix?

dougwilson avatar Jul 01 '22 15:07 dougwilson

Hi @dougwilson, it seems the problem stems from send module. Normally fs.createReadStream is able to read contents of /proc/* files, but when using send module, it introduces a problem somewhere. I used following code and it was not able to read the contents.

var http = require('http')
var send = require('send')

var server = http.createServer(function onRequest(req, res) {
	send(req, '/proc/version').pipe(res)
})

server.listen(1234)

mleblebici avatar Jul 02 '22 22:07 mleblebici

Hi @mleblebici thanks for investigating! Can you open a bug over there? Ideally if you can open a pull request with a fix, as I don't think I can personally fix it due to not having the /proc/ stuff on my system to use for testing what is wrong.

dougwilson avatar Jul 02 '22 22:07 dougwilson

Hi @dougwilson , I also investigated the problem with send library. It seems fs.stat returns size of 0 for /proc/* files. So, I added a getSize function to correctly get size of /proc/* files. It fixed the problem. But, as I am not a developer (especially not a JS developer), you should probably check to make sure it does not cause any performance issue or create any additional bug. Issue Link: https://github.com/pillarjs/send/issues/212 PR Link: https://github.com/pillarjs/send/pull/213

mleblebici avatar Jul 03 '22 12:07 mleblebici

Thanks! Have you checked with the Node.js project that fs.stat returning a size of 0 for those files is not simply a bug in their stat function? We'll want some kind of confirmation before landing a change like that.

dougwilson avatar Jul 03 '22 15:07 dougwilson

Virtual files such as /proc/meminfo don't have a known size and stat syscall returns size 0 for them.

krzysdz avatar Jul 03 '22 16:07 krzysdz

Thanks @krzysdz ! Well, that sucks, as normal files that are zero size also return zero :(

dougwilson avatar Jul 03 '22 16:07 dougwilson

Btw, you may want to check this one https://github.com/nodejs/node/issues/43669

mleblebici avatar Jul 04 '22 10:07 mleblebici

Thanks @krzysdz ! Well, that sucks, as normal files that are zero size also return zero :(

Well, if the file size is zero, maybe don't set the Content-Length header? The files in /proc//sys are usually small enough that unknown time-remaining is a non-issue here, and actual empty files will end the stream immediately anyway.

paperluigis avatar Aug 30 '22 12:08 paperluigis

the response.download function uses pillarjs/send which uses fs.stat to get the size of the file and set the http headers accordingly. then uses the size to generat fs.createReadStream options (start , end ) which is (0,0) this contributes to reciving an empty file. the reason fs.stat gets a zero size is that it uses unix system call stat which return size 0 for /proc files and other virtual system. a solution for this is to create special case handler for virtual files in pillarjs/send. I dont think that is needed as you can use fs directly.

I think using response.download to access /proc files should not be considered a good practice system information should not be accessed through a web server . they should at least get apprpriate processing.

const path = '/proc/meminfo';
// set the header so that the file will download instead of show in the browser 
res.setHeader('Content-Disposition',' attachment; filename="meminfo"')
fs.createReadStream(path, 'utf-8').pipe(res);

aelmardhi avatar Sep 17 '22 09:09 aelmardhi