Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

response.download does not return /proc/* files #4944

Open
mleblebici opened this issue Jul 1, 2022 · 10 comments
Open

response.download does not return /proc/* files #4944

mleblebici opened this issue Jul 1, 2022 · 10 comments

Comments

@mleblebici
Copy link

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.

@dougwilson
Copy link
Contributor

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?

@mleblebici
Copy link
Author

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)

@dougwilson
Copy link
Contributor

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.

@mleblebici
Copy link
Author

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: pillarjs/send#212
PR Link: pillarjs/send#213

@dougwilson
Copy link
Contributor

dougwilson commented Jul 3, 2022

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.

@krzysdz
Copy link
Contributor

krzysdz commented Jul 3, 2022

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

@dougwilson
Copy link
Contributor

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

@mleblebici
Copy link
Author

Btw, you may want to check this one nodejs/node#43669

@paperluigis
Copy link

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.

@aelmardhi
Copy link

aelmardhi commented Sep 17, 2022

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants