EBADF from fs.promises.readFile() and O_CREAT

[CynicalBusiness]

Had a bunch of tests failing with EBADF even after updating to ^4.10 (as mentioned in #245), finally managed to track it down.

When trying to mock fs.promises.readFile() with the fs.constants.O_CREAT flag, an EBADF is raised if the file does not previously exist. If the file is created manually or the flag is not used, it reads normally or raises ENOENT as it should.

Reproducable:

import mock from "mock-fs";
import { promises, constants } from "fs";

mock({ "foo.txt": "bar" });
const str1 = await promises.readFile("foo.txt"); // works fine
const str2 = await promises.readFile("bar.txt"); // ENOENT, as expected
const str3 = await promises.readFile("foo.txt", constants.O_CREAT | cosntants.O_RDONLY); // works
const str4 = await promises.readFile("bar.txt", constants.O_CREAT | constants.O_RDONLY); // EBADF, unexpected behavior.

node: 12.6.0
mock-fs: 4.10.4

[3cp]

O_RDONLY is 0, so constants.O_CREAT | constants.O_RDONLY is literally just constants.O_CREAT.

mockfs got confused about the read mode.

mock-fs/lib/descriptor.js

Lines 82 to 89 in ad93ac5

	FileDescriptor.prototype.isRead = function() {
	// special treatment because O_RDONLY is 0
	return (
	this._flags === constants.O_RDONLY ||
	this._flags === (constants.O_RDONLY | constants.O_SYNC) ||
	(this._flags & constants.O_RDWR) === constants.O_RDWR
	);
	};

The implementation should be fixed and simplified as

FileDescriptor.prototype.isRead = function() {
  return (this._flags & constants.O_WRONLY) !== constants.O_WRONLY;
};

If I understand open(2) man page correctly. http://man7.org/linux/man-pages/man2/open.2.html

The argument flags must include one of the following access modes:
O_RDONLY, O_WRONLY, or O_RDWR.  These request opening the file read-
only, write-only, or read/write, respectively.

O_RDONLY is 0, O_WRONLY is 1, O_RDWR is 2, the flag uses the 2 bits for 3 states: 0 (0b00) / 1 (0b01) / 2 (0b10), the 2 bits will never be 3 (0b11).

@tschaub does this fix make sense?

[3cp]

Note this is not a new bug on promises api, it happens on fs.readFile and fs.readFileSync too.

[tschaub]

Thanks for the report @CynicalBusiness and for the proposed fix @3cp.

Let's come up with a valid test case first. The examples given in the description above (promises.readFile("foo.txt", constants.O_CREAT | cosntants.O_RDONLY)) aren't valid because they provide a number as the second argument to readFile instead of an object or a string. This would throw TypeError [ERR_INVALID_ARG_TYPE]: The "options" argument must be one of type string or Object. Received type number.

[3cp]

Based on what I tested, although nodejs doc did not say, the readFile/sync/promise APIs support numeric option as if it's {flag: number}.

[tschaub]

Fix published in [email protected]. Thanks, @3cp.