Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(40)

Unified Diff: base/files/file_util_unittest.cc

Issue 2687713003: Stop base::OpenFile from leaking fds/handles into child procs. (Closed)
Patch Set: gab comments Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/files/file_util_posix.cc ('k') | base/files/file_util_win.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/files/file_util_unittest.cc
diff --git a/base/files/file_util_unittest.cc b/base/files/file_util_unittest.cc
index 7ef60a4b29881d0016adf5564956f5d1c749bd0a..5c918f88880e922d59ef24478d66701445bfad29 100644
--- a/base/files/file_util_unittest.cc
+++ b/base/files/file_util_unittest.cc
@@ -7,11 +7,15 @@
#include <algorithm>
#include <fstream>
+#include <initializer_list>
#include <set>
#include <utility>
#include <vector>
#include "base/base_paths.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback_helpers.h"
#include "base/environment.h"
#include "base/files/file_enumerator.h"
#include "base/files/file_path.h"
@@ -246,6 +250,30 @@ std::wstring ReadTextFile(const FilePath& filename) {
return std::wstring(contents);
}
+// Sets |is_inheritable| to indicate whether or not |stream| is set up to be
+// inerhited into child processes (i.e., HANDLE_FLAG_INHERIT is set on the
+// underlying handle on Windows, or FD_CLOEXEC is not set on the underlying file
+// descriptor on POSIX). Calls to this function must be wrapped with
+// ASSERT_NO_FATAL_FAILURE to properly abort tests in case of fatal failure.
+void GetIsInheritable(FILE* stream, bool* is_inheritable) {
+#if defined(OS_WIN)
+ HANDLE handle = reinterpret_cast<HANDLE>(_get_osfhandle(_fileno(stream)));
+ ASSERT_NE(INVALID_HANDLE_VALUE, handle);
+
+ DWORD info = 0;
+ ASSERT_EQ(TRUE, ::GetHandleInformation(handle, &info));
+ *is_inheritable = ((info & HANDLE_FLAG_INHERIT) != 0);
+#elif defined(OS_POSIX)
+ int fd = fileno(stream);
+ ASSERT_NE(-1, fd);
+ int flags = fcntl(fd, F_GETFD, 0);
+ ASSERT_NE(-1, flags);
+ *is_inheritable = ((flags & FD_CLOEXEC) == 0);
+#else
+#error Not implemented
+#endif
+}
+
TEST_F(FileUtilTest, FileAndDirectorySize) {
// Create three files of 20, 30 and 3 chars (utf8). ComputeDirectorySize
// should return 53 bytes.
@@ -1711,6 +1739,26 @@ TEST_F(FileUtilTest, GetTempDirTest) {
}
#endif // OS_WIN
+// Test that files opened by OpenFile are not set up for inheritance into child
+// procs.
+TEST_F(FileUtilTest, OpenFileNoInheritance) {
+ FilePath file_path(temp_dir_.GetPath().Append(FPL("a_file")));
+
+ for (const char* mode : {"wb", "r,ccs=UNICODE"}) {
+ SCOPED_TRACE(mode);
+ ASSERT_NO_FATAL_FAILURE(CreateTextFile(file_path, L"Geepers"));
+ FILE* file = OpenFile(file_path, mode);
+ ASSERT_NE(nullptr, file);
+ {
+ ScopedClosureRunner file_closer(Bind(IgnoreResult(&CloseFile), file));
+ bool is_inheritable = true;
+ ASSERT_NO_FATAL_FAILURE(GetIsInheritable(file, &is_inheritable));
+ EXPECT_FALSE(is_inheritable);
+ }
+ ASSERT_TRUE(DeleteFile(file_path, false));
+ }
+}
+
TEST_F(FileUtilTest, CreateTemporaryFileTest) {
FilePath temp_files[3];
for (int i = 0; i < 3; i++) {
« no previous file with comments | « base/files/file_util_posix.cc ('k') | base/files/file_util_win.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698