Node: Developer error exposing a bug with fs.writeFile

[prettydiff]

https://nodejs.org/dist/latest-v7.x/docs/api/fs.html#fs_fs_writefile_file_data_options_callback

The API for fs.writeFile is fs.writeFile(filename, filedata, callback). The problem is encountered when the developer goofs and gets the arguments in the wrong order: fs.writeFile(filedata, filename, callback). When filedata is an invalid filesystem path fs.writeFile still executes on the correct file as specified by the filename argument, but it writes an empty string and does not execute the callback.

I understand this is developer error and not Node error, but there are stability and possibly security implications in this behavior.

[addaleax]

Can you give an example? What types are you passing in (Buffers or strings)/what do you mean by “invalid” (something that doesn’t exist)? I tried a few simple variations but couldn’t reproduce this…

[prettydiff]

The string I passed in for filedata (writeFile's actual filename argument):

{"internal":{"prettydiff":{"location":"c:\\Users\\austincheney\\biddle\\internal\\prettydiff\\","version":"2.1.18","published":"http://prettydiff.com/downloads/prettydiff"},"jslint":{"location":"c:\\Users\\austincheney\\biddle\\internal\\jslint\\","version":"2017-04-10","published":"http://prettydiff.com/downloads/jslint"}}}

The filepath was an absolute path. I have noticed this behavior on both Windows and OSX. I have not tried to reproduce this behavior with a buffer. I suspect a variety of strings that complicate file system traversal could reproduce this behavior.

[addaleax]

'use strict';
const fs = require('fs');

fs.writeFile('{"internal":{"prettydiff":{"location":"c:\\Users\\austincheney\\biddle\\internal\\prettydiff\\","version":"2.1.18","published":"http://prettydiff.com/downloads/prettydiff"},"jslint":{"location":"c:\\Users\\austincheney\\biddle\\internal\\jslint\\","version":"2017-04-10","published":"http://prettydiff.com/downloads/jslint"}}}',
             '/tmp/foobar.json',
             function(err) {
               console.log('callback called with', err);
             });

results in

callback called with { Error: ENOENT: no such file or directory, open '{"internal":{"prettydiff":{"location":"c:\Users\austincheney\biddle\internal\prettydiff\","version":"2.1.18","published":"http://prettydiff.com/downloads/prettydiff"},"jslint":{"location":"c:\Users\austincheney\biddle\internal\jslint\","version":"2017-04-10","published":"http://prettydiff.com/downloads/jslint"}}}'
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '{"internal":{"prettydiff":{"location":"c:\\Users\\austincheney\\biddle\\internal\\prettydiff\\","version":"2.1.18","published":"http://prettydiff.com/downloads/prettydiff"},"jslint":{"location":"c:\\Users\\austincheney\\biddle\\internal\\jslint\\","version":"2017-04-10","published":"http://prettydiff.com/downloads/jslint"}}}' }

for me, and there are no files created that I could find – so it seems like everything works?

[refack]

I tried to be trickier, removed non path valid cars from the content

'use strict';
const fs = require('fs');

var content = '{internal{prettydiff{locationctempprettydiffversion2118published}jslint{locationctemplintversion2017-04-10published}}}';

fs.writeFile(content, 'c:\\temp\\prettydiff',
    function(err, val) {
        err && console.log('callback called with', err);
        val && console.log('callback called with', val)
    }
);

and indeed a file named D:\code\4node\internal{prettydiff{locationctempprettydiffversion2118published}jslint{locationctemplintversion2017-04-10published}}} was created.

But I don't see robust way of getting out of this...

[prettydiff]

I don't know if it matters, but when I produce this error the file in question already exists opposed to being created by this method.

Here is the code which produces this condition: https://github.com/prettydiff/biddle/blob/master/biddle.js#L4976

It is very complicated to reproduce.

[addaleax]

I’d say @refack is right, I doubt there’s a good way for node to tell whether you accidentally switched the parameters or not…

[prettydiff]

Thanks for looking into this. I am going to close this for now as it doesn't appear to be an issue. If I can gather additional evidence or simplify reproduction I will reopen the issue.