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

support for /proc/* files #213

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mleblebici
Copy link

/proc/* files could not be read, because fs.stat was returning 0 size. With new getSize function, their size information is correct, so they can now be read.

/proc/* files could not be read, because fs.stat was returning 0 size. With new getSize function, their size information is correct, so they can now be read.
Copy link
Contributor

@dougwilson dougwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general comment is that it seems when stat reads the file size as zero, this new code will read the entire contents of the file in to memory. If that is necessary, there should be some kind of upper limit on it so it does not cause the web server to oom on a file. You probably can just track the read chunk lengths without sorting a complete buffere to get the length of.

}
} while (bytesRead !== 0);
fs.closeSync(fd);
return buf.toString('utf8', 0, pos).length;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will return the wrong size if the data contains any utf8 charachers, as the length is the number of characters in this line, but what is needed is the number of bytes.

let buf = tmpBuf;
let length = buf.length;
do {
bytesRead = fs.readSync(fd, buf, pos, buf.length - pos, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performing sync operations will halt processing of parallel requests of the http server. You will need to use the async version of the functions, not the sync versions.

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

Successfully merging this pull request may close these issues.

2 participants