From 28d0cbbdfdf3682adf1fe0c560e7379559fcd621 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Thu, 15 Jan 2015 17:29:54 -0800 Subject: [PATCH] test: fix test-fs-access.js On non-windows supported platforms, fs.access(readOnlyFile, W_OK, ...) is expected to fail, but always succeeds if node runs as the super user, which is often the case for tests running on our continuous integration platform. This change makes the test try to change its process user id to nobody on non-windows platforms so that the above mentioned test can pass and still perform the actual desired test. If changing the process user id to a nobody is not possible, then the test checks that fs.access(readOnlyFile, W_OK, ...) actually succeeds. Fixes #9033. Reviewed-By: Colin Ihrig Reviewed-By: Timothy J Fontaine --- test/simple/test-fs-access.js | 54 ++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/test/simple/test-fs-access.js b/test/simple/test-fs-access.js index 3c0ac6162df3..83af67b9a2e7 100644 --- a/test/simple/test-fs-access.js +++ b/test/simple/test-fs-access.js @@ -25,8 +25,9 @@ var fs = require('fs'); var path = require('path'); var doesNotExist = __filename + '__this_should_not_exist'; var readOnlyFile = path.join(common.tmpDir, 'read_only_file'); +var readWriteFile = path.join(common.tmpDir, 'read_write_file'); -var removeFile = function(file) { +function removeFile(file) { try { fs.unlinkSync(file); } catch (err) { @@ -34,13 +35,47 @@ var removeFile = function(file) { } }; -var createReadOnlyFile = function(file) { +function createFileWithPerms(file, mode) { removeFile(file); fs.writeFileSync(file, ''); - fs.chmodSync(file, 0444); + fs.chmodSync(file, mode); }; -createReadOnlyFile(readOnlyFile); +createFileWithPerms(readOnlyFile, 0444); +createFileWithPerms(readWriteFile, 0666); + +/* + * On non-Windows supported platforms, fs.access(readOnlyFile, W_OK, ...) + * always succeeds if node runs as the super user, which is sometimes the + * case for tests running on our continuous testing platform agents. + * + * In this case, this test tries to change its process user id to a + * non-superuser user so that the test that checks for write access to a + * read-only file can be more meaningful. + * + * The change of user id is done after creating the fixtures files for the same + * reason: the test may be run as the superuser within a directory in which + * only the superuser can create files, and thus it may need superuser + * priviledges to create them. + * + * There's not really any point in resetting the process' user id to 0 after + * changing it to 'nobody', since in the case that the test runs without + * superuser priviledge, it is not possible to change its process user id to + * superuser. + * + * It can prevent the test from removing files created before the change of user + * id, but that's fine. In this case, it is the responsability of the + * continuous integration platform to take care of that. + */ +var hasWiteAccessForReadonlyFile = false; +if (process.platform !== 'win32' && process.getuid() === 0) { + hasWiteAccessForReadonlyFile = true; + try { + process.setuid('nobody'); + hasWiteAccessForReadonlyFile = false; + } catch (err) { + } +} assert(typeof fs.F_OK === 'number'); assert(typeof fs.R_OK === 'number'); @@ -66,8 +101,12 @@ fs.access(readOnlyFile, fs.F_OK | fs.R_OK, function(err) { }); fs.access(readOnlyFile, fs.W_OK, function(err) { - assert.notEqual(err, null, 'error should exist'); - assert.strictEqual(err.path, readOnlyFile); + if (hasWiteAccessForReadonlyFile) { + assert.equal(err, null, 'error should not exist'); + } else { + assert.notEqual(err, null, 'error should exist'); + assert.strictEqual(err.path, readOnlyFile); + } }); assert.throws(function() { @@ -85,7 +124,7 @@ assert.doesNotThrow(function() { assert.doesNotThrow(function() { var mode = fs.F_OK | fs.R_OK | fs.W_OK; - fs.accessSync(__filename, mode); + fs.accessSync(readWriteFile, mode); }); assert.throws(function() { @@ -96,4 +135,5 @@ assert.throws(function() { process.on('exit', function() { removeFile(readOnlyFile); + removeFile(readWriteFile); });