Merge pull request #127 from patrickbrophy/posc-fixes

Add file size verification to POSC plugin to prevent incomplete uploads

Author: patrickbrophy

src/Posc.cc

@@ -186,6 +186,16 @@ bool PoscFileSystem::Config(const char *configfn) {
 
 int PoscFileSystem::Create(const char *tid, const char *path, mode_t mode,
 						   XrdOucEnv &env, int opts) {
+	// The open flags are passed in opts >> 8. If O_CREAT or O_TRUNC are set,
+	// POSC will handle the file creation in Open(), so we should NOT create
+	// the file here at the final destination. This prevents an empty file
+	// from appearing in the exported directory during upload.
+	int open_flags = opts >> 8;
+	if (open_flags & (O_CREAT | O_TRUNC)) {
+		m_log->Log(LogMask::Debug, "POSC",
+				   "Skipping Create for POSC-handled file:", path);
+		return 0;
+	}
 	return VerifyPath(path, &XrdOss::Create, tid, path, mode, env, opts);
 }
 
@@ -709,6 +719,7 @@ int PoscFile::Close(long long *retsz) {
 	auto close_rv = wrapDF.Close(retsz);
 	if (close_rv) {
 		m_oss.Unlink(m_posc_filename.c_str(), 0, m_posc_env.get());
+		m_posc_filename.clear();
 		return close_rv;
 	}
 
@@ -719,9 +730,36 @@ int PoscFile::Close(long long *retsz) {
 				  m_posc_filename.c_str(), strerror(-rv));
 
 		m_oss.Unlink(m_posc_filename.c_str(), 0, m_posc_env.get());
+		m_posc_filename.clear();
 		return -EIO;
 	}
 
+	// Expected file size is advisory; if it is present, verify it matches
+	// before persisting Otherwise, we don't know the expected file size, so we
+	// don't verify it and just persist the file.
+	if (m_expected_size > 0) {
+		struct stat sb;
+		rv = m_oss.Stat(m_posc_filename.c_str(), &sb, 0, m_posc_env.get());
+		if (rv) {
+			m_log.Log(LogMask::Error, "POSC", "Failed to stat POSC file",
+					  m_posc_filename.c_str(), strerror(-rv));
+			m_oss.Unlink(m_posc_filename.c_str(), 0, m_posc_env.get());
+			m_posc_filename.clear();
+			return -EIO;
+		}
+		if (sb.st_size != m_expected_size) {
+			std::stringstream ss;
+			m_log.Log(LogMask::Error, "POSC", ss.str().c_str(),
+					  m_posc_filename.c_str());
+			m_oss.Unlink(m_posc_filename.c_str(), 0, m_posc_env.get());
+			m_posc_filename.clear();
+			return -EIO;
+		}
+	}
+
+	// At this point, we either don't know the expected file size, or the file
+	// size matches the expected size. So we can persist the file.
+
 	rv = m_oss.Rename(m_posc_filename.c_str(), m_orig_filename.c_str(),
 					  m_posc_env.get(), m_posc_env.get());
 	if (rv) {
@@ -731,9 +769,10 @@ int PoscFile::Close(long long *retsz) {
 		m_log.Log(LogMask::Error, "POSC", ss.str().c_str());
 
 		m_oss.Unlink(m_posc_filename.c_str(), 0, m_posc_env.get());
-
+		m_posc_filename.clear();
 		return -EIO;
 	}
+	m_posc_filename.clear();
 	return 0;
 }
 
@@ -744,7 +783,7 @@ int PoscFile::Open(const char *path, int Oflag, mode_t Mode, XrdOucEnv &env) {
 		return -ENOENT;
 	}
 
-	if ((Oflag & O_CREAT) == 0) {
+	if ((Oflag & (O_CREAT | O_TRUNC)) == 0) {
 		return wrapDF.Open(path, Oflag, Mode, env);
 	}
 
@@ -776,14 +815,27 @@ int PoscFile::Open(const char *path, int Oflag, mode_t Mode, XrdOucEnv &env) {
 	auto envbuff = env.Env(envlen);
 	m_posc_env.reset(new XrdOucEnv(envbuff, envlen, m_posc_entity.get()));
 	m_posc_mode = Mode;
+
+	// Extract expected file size from oss.asize if available
+	char *asize_char = env.Get("oss.asize");
+	if (asize_char) {
+		char *endptr;
+		long long asize = strtoll(asize_char, &endptr, 10);
+		if (endptr != asize_char && *endptr == '\0' && asize >= 0) {
+			m_expected_size = static_cast<off_t>(asize);
+			m_log.Log(LogMask::Debug, "POSC", "Expected file size:",
+					  std::to_string(m_expected_size).c_str());
+		}
+	}
+
 	m_posc_mtime.store(
 		std::chrono::system_clock::now().time_since_epoch().count(),
 		std::memory_order_relaxed);
 	for (int idx = 0; idx < 10; ++idx) {
 		m_posc_filename = m_posc_fs.GeneratePoscFile(path, env);
 
-		auto rv =
-			wrapDF.Open(m_posc_filename.c_str(), Oflag | O_EXCL, 0600, env);
+		auto rv = wrapDF.Open(m_posc_filename.c_str(), Oflag | O_EXCL | O_CREAT,
+							  0600, env);
 		if (rv >= 0) {
 			m_log.Log(LogMask::Debug, "POSC", "Opened POSC file",
 					  m_posc_filename.c_str());
@@ -805,10 +857,11 @@ int PoscFile::Open(const char *path, int Oflag, mode_t Mode, XrdOucEnv &env) {
 			m_log.Log(LogMask::Debug, "POSC",
 					  "POSC sub-directory is needed for file creation:",
 					  posc_dir.c_str());
-			if (m_oss.Mkdir(posc_dir.c_str(), 0700, 1, &env) != 0) {
+			auto mkdir_rv = m_oss.Mkdir(posc_dir.c_str(), 0700, 1, &env);
+			if (mkdir_rv != 0) {
 				m_log.Log(LogMask::Error, "POSC",
 						  "Failed to create POSC sub-directory",
-						  posc_dir.c_str(), strerror(errno));
+						  posc_dir.c_str(), strerror(-mkdir_rv));
 				return -EIO;
 			}
 		} else if (rv == -EINTR) {

src/Posc.hh

@@ -211,6 +211,7 @@ class PoscFile final : public XrdOssWrapDF {
 	std::atomic<std::chrono::system_clock::duration::rep> m_posc_mtime{0};
 	std::string m_posc_filename;
 	std::string m_orig_filename;
+	off_t m_expected_size{-1};  // Expected file size from oss.asize, -1 if unknown
 
 	// Static members to keep track of all PoscFile instances.
 	// Periodically, the filesystem object will iterate through

test/posc_tests.cc

@@ -112,6 +112,7 @@ TEST_F(TestPosc, BasicFileVisibility) {
 	std::unique_ptr<XrdOssDF> fp(posc_fs->newFile());
 	ASSERT_NE(fp, nullptr) << "Failed to create new file object";
 
+	env.Put("oss.asize", "0");
 	fp->Open("/testfile.txt", O_CREAT | O_RDWR, 0644, env);
 
 	auto rv = posc_fs->Stat("/testfile.txt", nullptr, 0, &env);
@@ -130,8 +131,9 @@ TEST_F(TestPosc, BasicFileVisibility) {
 		<< "Stat on created file did not return regular file";
 	ASSERT_EQ(sb.st_size, 0) << "Stat on created file did not return size 0";
 
-	fp->Open("/testfile2.txt", O_CREAT | O_RDWR, 0644, env);
 	auto write_size = strlen("Hello, POSC!");
+	env.Put("oss.asize", std::to_string(write_size).c_str());
+	fp->Open("/testfile2.txt", O_CREAT | O_RDWR, 0644, env);
 	ASSERT_EQ(fp->Write("Hello, POSC!", 0, write_size), write_size);
 
 	rv = posc_fs->Stat("/testfile2.txt", nullptr, 0, &env);
@@ -327,6 +329,7 @@ TEST_F(TestPosc, CreateENOENT) {
 
 	posc_fs->Mkdir("/subdir", 0755, 1, &env);
 
+	env.Put("oss.asize", "0");
 	rv = fp->Open("/subdir/testfile.txt", O_CREAT | O_RDWR, 0644, env);
 	ASSERT_EQ(rv, 0) << "Open on newly created directory failed: "
 					 << strerror(-rv);