Addressed ansjcy's comments

Signed-off-by: Peter Alfonsi <[email protected]>

server/src/main/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicy.java

@@ -54,18 +54,23 @@ protected void setThreshold(TimeValue threshold) { // protected so that we can m
 
     @Override
     public boolean checkData(BytesReference data) {
-        if (threshold.equals(TimeValue.ZERO)) {
-            return true;
-        }
         Long tookTimeNanos;
         try {
             tookTimeNanos = getPolicyInfoFn.apply(data).getTookTimeNanos();
         } catch (Exception e) {
-            // If we can't retrieve the took time for whatever reason, admit the data to be safe
-            return true;
+            // If we can't read a CachePolicyInfoWrapper from the BytesReference, reject the data
+            return false;
         }
+
         if (tookTimeNanos == null) {
-            // Received a null took time -> this QSR is from an old version which does not have took time, we should accept it
+            // If the wrapper contains null took time, reject the data
+            // This can happen if no CachePolicyInfoWrapper was written to the BytesReference, as the wrapper's constructor
+            // reads an optional long, which will end up as null in this case. This is why we should reject it.
+            return false;
+        }
+
+        if (threshold.equals(TimeValue.ZERO)) {
+            // If the policy is set to zero, admit any well-formed data
             return true;
         }
         TimeValue tookTime = TimeValue.timeValueNanos(tookTimeNanos);

server/src/test/java/org/opensearch/common/cache/tier/DiskTierTookTimePolicyTests.java

@@ -83,6 +83,38 @@ public void testTookTimePolicy() throws Exception {
         assertTrue(longResult);
     }
 
+    public void testMissingWrapper() throws Exception {
+        DiskTierTookTimePolicy tookTimePolicy = getTookTimePolicy();
+        tookTimePolicy.setThreshold(TimeValue.ZERO);
+        QuerySearchResult qsr = getQSR();
+        BytesStreamOutput out = new BytesStreamOutput();
+        qsr.writeTo(out);
+        BytesReference missingWrapper = out.bytes();
+        boolean allowedMissingWrapper = tookTimePolicy.checkData(missingWrapper);
+        assertFalse(allowedMissingWrapper);
+    }
+
+    public void testNullTookTime() throws Exception {
+        // Null took time should always be rejected (because it might be the result of a
+        // BytesReference without a CachePolicyInfoWrapper in front of it)
+
+        DiskTierTookTimePolicy zeroThreshold = getTookTimePolicy();
+        zeroThreshold.setThreshold(TimeValue.ZERO);
+        DiskTierTookTimePolicy nonZeroThreshold = getTookTimePolicy();
+        nonZeroThreshold.setThreshold(new TimeValue(10L));
+
+        Long nullTookTime = null;
+        CachePolicyInfoWrapper nullWrapper = new CachePolicyInfoWrapper(nullTookTime);
+        BytesStreamOutput out = new BytesStreamOutput();
+        nullWrapper.writeTo(out);
+        QuerySearchResult qsr = getQSR();
+        qsr.writeTo(out);
+        BytesReference data = out.bytes();
+
+        assertFalse(zeroThreshold.checkData(data));
+        assertFalse(nonZeroThreshold.checkData(data));
+    }
+
     public static QuerySearchResult getQSR() {
         // package-private, also used by IndicesRequestCacheTests.java
         // setup from QuerySearchResultTests.java