streamich / memfs

JavaScript file system utilities
http://streamich.github.io/memfs/
Apache License 2.0
1.77k stars 130 forks source link

fix: don't allow files to be nested inside other files #1052

Closed Kenny4297 closed 1 month ago

Kenny4297 commented 4 months ago

Resolves #1049

Fix issue with nested path write in Volume and add tests

Issue

The Volume class had an issue where files could incorrectly be nested inside other files. It was possible to read/write files nested inside other files which should not be allowed.

Solution

To resolve this issue, I updated the writeFileSync and readFileSync methods in the Volume class to check if the parent path of the file being written to is a file. If it is, we throw an ENOTDIR error. Additionally, we check if a path being read is a directory and throw an EISDIR error if so.

Changes Made

Updated writeFileSync in volume.ts:

writeFileSync(id: TFileId, data: TData, options?: opts.IWriteFileOptions): void {
    const opts = getWriteFileOptions(options);
    const flagsNum = flagsToNumber(opts.flag);
    const modeNum = modeToNumber(opts.mode);
    const buf = dataToBuffer(data, opts.encoding);

    // Check if the parent path is a file, and throw ENOTDIR error if so
    if (typeof id === 'string') {
        const parentPath = pathModule.dirname(id);
        if (this.existsSync(parentPath) && !this.statSync(parentPath).isDirectory()) {
            throw createError('ENOTDIR', 'writeFile', id);
        }
    }

    this.writeFileBase(id, buf, flagsNum, modeNum);
}

Updated readFileSync in volume.ts:

readFileSync(file: TFileId, options?: opts.IReadFileOptions | string): TDataOut {
    const opts = getReadFileOptions(options);
    const flagsNum = flagsToNumber(opts.flag);

    // Check if the path is a directory, and throw EISDIR error if so
    if (typeof file === 'string') {
        if (this.existsSync(file) && this.statSync(file).isDirectory()) {
            throw createError('EISDIR', 'readFile', file);
        }
    }

    return this.readFileBase(file, flagsNum, opts.encoding as BufferEncoding);
}

Added Tests

I added a new test file volume-issue.test.ts under src/tests/volume/ to cover these cases. They all pass:

import { Volume } from '../../volume';

describe('Volume issue tests', () => {
    let vol: Volume;

    beforeEach(() => {
        vol = new Volume();
    });

    test('should throw ENOTDIR error when writing to a nested path within a file', () => {
        vol.writeFileSync('/foo', 'hello');

        expect(() => {
            vol.writeFileSync('/foo/bar', 'world');
        }).toThrow(/ENOTDIR/);
    });

    test('should throw EISDIR error when trying to read a directory as a file', () => {
        vol.mkdirSync('/foo');

        expect(() => {
            vol.readFileSync('/foo');
        }).toThrow(/EISDIR/);
    });

    test('should read file contents correctly', () => {
        vol.writeFileSync('/foo', 'hello');
        const content = vol.readFileSync('/foo', 'utf8');
        expect(content).toBe('hello');
    });

    test('should throw ENOTDIR error when reading directory contents of a file', () => {
        vol.writeFileSync('/foo', 'hello');

        expect(() => {
            vol.readdirSync('/foo');
        }).toThrow(/ENOTDIR/);
    });
});

Thank you for allowing me to contribute to this project. I learned a lot, and if there is anything else that you need me to change, please let me know!

G-Rath commented 1 month ago

Closing since this got fixed in v4.12.0