Const correctness handle / RedactionInfo.

Helps us CHECK that they're always constructed with valid
RedactionInfo etc.

Test: atest FuseDaemonHostTest
Test: atest fuse_node_test

Bug: 147274248

Change-Id: I2cc369574d14136521201b4c8b99fe22e7ec0463

5 files changed, 28 insertions, 30 deletions

diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index c031d5846..5dfbb3f57 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -847,11 +847,7 @@ static void pf_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info* fi) {
         return;
     }
 
-    handle* h = new handle(path);
-    h->fd = fd;
-    h->ri = std::move(ri);
-
-    if (h->ri->isRedactionNeeded() || is_file_locked(h->fd, path)) {
+    if (ri->isRedactionNeeded() || is_file_locked(fd, path)) {
         // We don't want to use the FUSE VFS cache in two cases:
         // 1. When redaction is needed because app A with EXIF access might access
         // a region that should have been redacted for app B without EXIF access, but app B on
@@ -865,9 +861,9 @@ static void pf_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info* fi) {
         // b. Reading from a FUSE fd with caching enabled may not see the latest writes using the
         // lower fs fd because those writes did not go through the FUSE layer and reads from FUSE
         // after that write may be served from cache
-        if (h->ri->isRedactionNeeded()) {
+        if (ri->isRedactionNeeded()) {
             TRACE_FUSE(fuse) << "Using direct io for " << path << " because redaction is needed.";
-        } else if (is_file_locked(h->fd, path)) {
+        } else {
             TRACE_FUSE(fuse) << "Using direct io for " << path << " because the file is locked.";
         }
         fi->direct_io = true;
@@ -875,7 +871,7 @@ static void pf_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info* fi) {
         TRACE_FUSE(fuse) << "Using cache for " << path;
     }
 
-    h->cached = !fi->direct_io;
+    handle* h = new handle(path, fd, ri.release(), !fi->direct_io);
     node->AddHandle(h);
 
     fi->fh = ptr_to_id(h);
@@ -1350,13 +1346,11 @@ static void pf_create(fuse_req_t req,
         return;
     }
 
-    handle* h = new handle(child_path);
-    h->fd = fd;
-    // TODO(b/147274248): Assuming there will be no EXIF to redact.
+    // TODO(b/147274248): Assume there will be no EXIF to redact.
     // This prevents crashing during reads but can be a security hole if a malicious app opens an fd
     // to the file before all the EXIF content is written. We could special case reads before the
     // first close after a file has just been created.
-    h->ri = std::make_unique<RedactionInfo>();
+    handle* h = new handle(child_path, fd, new RedactionInfo(), true /* cached */);
     fi->fh = ptr_to_id(h);
     fi->keep_cache = 1;
 
diff --git a/jni/RedactionInfo.cpp b/jni/RedactionInfo.cpp
index 3494db742..d943c8aee 100644
--- a/jni/RedactionInfo.cpp
+++ b/jni/RedactionInfo.cpp
@@ -56,7 +56,7 @@ static void mergeOverlappingRedactionRanges(vector<RedactionRange>& ranges) {
  *
  * This function assumes redaction_ranges_ within RedactionInfo is sorted.
  */
-bool RedactionInfo::hasOverlapWithReadRequest(size_t size, off64_t off) {
+bool RedactionInfo::hasOverlapWithReadRequest(size_t size, off64_t off) const {
     if (!isRedactionNeeded() || off > redaction_ranges_.back().second ||
         off + size < redaction_ranges_.front().first) {
         return false;
@@ -79,11 +79,11 @@ void RedactionInfo::processRedactionRanges(int redaction_ranges_num,
     mergeOverlappingRedactionRanges(redaction_ranges_);
 }
 
-int RedactionInfo::size() {
+int RedactionInfo::size() const {
     return redaction_ranges_.size();
 }
 
-bool RedactionInfo::isRedactionNeeded() {
+bool RedactionInfo::isRedactionNeeded() const {
     return size() > 0;
 }
 
@@ -93,13 +93,13 @@ RedactionInfo::RedactionInfo(int redaction_ranges_num, const off64_t* redaction_
 }
 
 unique_ptr<vector<RedactionRange>> RedactionInfo::getOverlappingRedactionRanges(size_t size,
-                                                                                off64_t off) {
+                                                                                off64_t off) const {
     LOG(DEBUG) << "Computing redaction ranges for request: sz = " << size << " off = " << off;
     if (hasOverlapWithReadRequest(size, off)) {
         auto first_redaction = redaction_ranges_.end();
         auto last_redaction = redaction_ranges_.end();
         for (auto iter = redaction_ranges_.begin(); iter != redaction_ranges_.end(); ++iter) {
-            RedactionRange& rr = *iter;
+            const RedactionRange& rr = *iter;
             // Look for the first range that overlaps with the read request
             if (first_redaction == redaction_ranges_.end() && off <= rr.second &&
                 off + size >= rr.first) {
@@ -121,4 +121,4 @@ unique_ptr<vector<RedactionRange>> RedactionInfo::getOverlappingRedactionRanges(
     return std::make_unique<vector<RedactionRange>>();
 }
 }  // namespace fuse
-}  // namespace mediaprovider
\ No newline at end of file
+}  // namespace mediaprovider
diff --git a/jni/include/libfuse_jni/RedactionInfo.h b/jni/include/libfuse_jni/RedactionInfo.h
index 134ee90dd..5218e28a6 100644
--- a/jni/include/libfuse_jni/RedactionInfo.h
+++ b/jni/include/libfuse_jni/RedactionInfo.h
@@ -69,20 +69,20 @@ class RedactionInfo {
      * relevant redaction ranges, the vector will be empty.
      */
     std::unique_ptr<std::vector<RedactionRange>> getOverlappingRedactionRanges(size_t size,
-                                                                               off64_t off);
+                                                                               off64_t off) const;
     /**
      * Returns whether any ranges need to be redacted.
      */
-    bool isRedactionNeeded();
+    bool isRedactionNeeded() const;
     /**
      * Returns number of redaction ranges.
      */
-    int size();
+    int size() const;
 
   private:
     std::vector<RedactionRange> redaction_ranges_;
     void processRedactionRanges(int redaction_ranges_num, const off64_t* redaction_ranges);
-    bool hasOverlapWithReadRequest(size_t size, off64_t off);
+    bool hasOverlapWithReadRequest(size_t size, off64_t off) const;
 };
 
 }  // namespace fuse
diff --git a/jni/node-inl.h b/jni/node-inl.h
index 5db14134e..487583cea 100644
--- a/jni/node-inl.h
+++ b/jni/node-inl.h
@@ -34,17 +34,21 @@ namespace mediaprovider {
 namespace fuse {
 
 struct handle {
-    explicit handle(const std::string& path) : path(path), fd(-1), ri(nullptr), cached(true){};
+    explicit handle(const std::string& path, int fd, const RedactionInfo* ri, bool cached)
+        : path(path), fd(fd), ri(ri), cached(cached) {
+        CHECK(ri != nullptr);
+    }
+
     const std::string path;
-    int fd;
-    std::unique_ptr<RedactionInfo> ri;
-    bool cached;
+    const int fd;
+    const std::unique_ptr<const RedactionInfo> ri;
+    const bool cached;
 
     ~handle() { close(fd); }
 };
 
 struct dirhandle {
-    explicit dirhandle(DIR* dir) : d(dir), next_off(0){};
+    explicit dirhandle(DIR* dir) : d(dir), next_off(0) { CHECK(dir != nullptr); }
 
     DIR* const d;
     off_t next_off;
diff --git a/jni/node_test.cpp b/jni/node_test.cpp
index 1c7103c8f..c72c8ee1c 100644
--- a/jni/node_test.cpp
+++ b/jni/node_test.cpp
@@ -200,8 +200,7 @@ TEST_F(NodeTest, LookupAbsolutePath) {
 TEST_F(NodeTest, AddDestroyHandle) {
     unique_node_ptr node = CreateNode(nullptr, "/path");
 
-    handle* h = new handle("/path");
-    h->cached = true;
+    handle* h = new handle("/path", -1, new mediaprovider::fuse::RedactionInfo, true /* cached */);
     node->AddHandle(h);
     ASSERT_TRUE(node->HasCachedHandle());
 
@@ -212,6 +211,7 @@ TEST_F(NodeTest, AddDestroyHandle) {
     // the node in question.
     EXPECT_DEATH(node->DestroyHandle(h), "");
     EXPECT_DEATH(node->DestroyHandle(nullptr), "");
-    std::unique_ptr<handle> h2(new handle("/path2"));
+    std::unique_ptr<handle> h2(
+            new handle("/path2", -1, new mediaprovider::fuse::RedactionInfo, true /* cached */));
     EXPECT_DEATH(node->DestroyHandle(h2.get()), "");
 }