bmc: firmware updatePending: open(blob)

Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I0bb9de94c8afab5fd91e395b3dba69f22625331a
diff --git a/firmware_handler.cpp b/firmware_handler.cpp
index 3a109de..2213aef 100644
--- a/firmware_handler.cpp
+++ b/firmware_handler.cpp
@@ -316,6 +316,26 @@
         return false;
     }
 
+    /* When in this state, they can only open the updateBlobId */
+    if (state == UpdateState::updatePending)
+    {
+        if (path != updateBlobId)
+        {
+            return false;
+        }
+        else
+        {
+            /* Similarly to verifyBlodId, this is special. */
+            updateImage.flags = flags;
+            updateImage.state = Session::State::open;
+
+            lookup[session] = &updateImage;
+
+            fileOpen = true;
+            return true;
+        }
+    }
+
     /* Handle opening the verifyBlobId --> we know the image and hash aren't
      * open because of the fileOpen check.
      *
@@ -333,7 +353,6 @@
         lookup[session] = &verifyImage;
 
         fileOpen = true;
-
         return true;
     }
 
@@ -343,6 +362,18 @@
      * layout flash update or a UBI tarball.
      */
 
+    /* 2) there isn't, so what are they opening? */
+    if (path == activeImageBlobId || path == activeHashBlobId)
+    {
+        /* 2a) are they opening the active image? this can only happen if they
+         * already started one (due to canHandleBlob's behavior).
+         */
+        /* 2b) are they opening the active hash? this can only happen if they
+         * already started one (due to canHandleBlob's behavior).
+         */
+        return false;
+    }
+
     /* Check the flags for the transport mechanism: if none match we don't
      * support what they request.
      */
@@ -351,22 +382,6 @@
         return false;
     }
 
-    /* 2) there isn't, so what are they opening? */
-    if (path == activeImageBlobId)
-    {
-        /* 2a) are they opening the active image? this can only happen if they
-         * already started one (due to canHandleBlob's behavior).
-         */
-        return false;
-    }
-    else if (path == activeHashBlobId)
-    {
-        /* 2b) are they opening the active hash? this can only happen if they
-         * already started one (due to canHandleBlob's behavior).
-         */
-        return false;
-    }
-
     /* How are they expecting to copy this data? */
     auto d = std::find_if(
         transports.begin(), transports.end(),
diff --git a/test/firmware_state_updatepending_unittest.cpp b/test/firmware_state_updatepending_unittest.cpp
index 646cef0..999c92c 100644
--- a/test/firmware_state_updatepending_unittest.cpp
+++ b/test/firmware_state_updatepending_unittest.cpp
@@ -92,7 +92,38 @@
 }
 
 /*
- * deleteBlob(blob)
+ * open(blob) - because updatePending is in a fileOpen==false state, one can
+ * then open blobs. However, because we're in a special state, we will restrict
+ * them s.t. they can only open the updateBlobId.
+ */
+TEST_F(FirmwareHandlerUpdatePendingTest,
+       OpenUpdateBlobIdIsSuccessfulAndDoesNotChangeState)
+{
+    getToUpdatePending();
+
+    /* Opening the update blob isn't interesting, except it's required for
+     * commit() which triggers the update process.
+     */
+    EXPECT_TRUE(handler->open(session, flags, updateBlobId));
+}
+
+TEST_F(FirmwareHandlerUpdatePendingTest, OpenAnyBlobOtherThanUpdateFails)
+{
+    getToUpdatePending();
+
+    auto blobs = handler->getBlobIds();
+    for (const auto& blob : blobs)
+    {
+        if (blob == updateBlobId)
+        {
+            continue;
+        }
+        EXPECT_FALSE(handler->open(session, flags, blob));
+    }
+}
+
+/*
+ * TODO: deleteBlob(blob)
  */
 
 /*
@@ -104,10 +135,6 @@
  */
 
 /*
- * open(blob)
- */
-
-/*
  * close(session)
  */