From 77d641216ae6ecb2188e2967e3f21f2a3fac19a2 Mon Sep 17 00:00:00 2001
From: Lee Hinman <lee@writequit.org>
Date: Tue, 21 Feb 2017 10:40:31 -0700
Subject: [PATCH] Handle long overflow when adding paths' totals

From #23093, we fixed the issue where a filesystem can be so large that it
overflows and returns a negative number. However, there is another issue when
adding a path as a sub-path to another `FsInfo.Path` object, when adding the
totals the values can still overflow.

This adds the same safety to return `Long.MAX_VALUE` instead of the negative
number, as well as a test exercising the logic.
---
 .../org/elasticsearch/monitor/fs/FsInfo.java  |  2 +-
 .../org/elasticsearch/monitor/fs/FsProbe.java |  6 ++++-
 .../monitor/fs/FsProbeTests.java              | 24 +++++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java b/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
index 33cb70a0d0ba9..7bb1e51cd2372 100644
--- a/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
+++ b/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
@@ -137,7 +137,7 @@ private double addDouble(double current, double other) {
         }
 
         public void add(Path path) {
-            total = addLong(total, path.total);
+            total = FsProbe.adjustForHugeFilesystems(addLong(total, path.total));
             free = addLong(free, path.free);
             available = addLong(available, path.available);
             if (path.spins != null && path.spins.booleanValue()) {
diff --git a/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java b/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java
index d079a7201686f..1fdae49a6f16b 100644
--- a/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java
+++ b/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java
@@ -136,7 +136,11 @@ List<String> readProcDiskStats() throws IOException {
     }
 
     /* See: https://bugs.openjdk.java.net/browse/JDK-8162520 */
-    private static long adjustForHugeFilesystems(long bytes) {
+    /**
+     * Take a large value intended to be positive, and if it has overflowed,
+     * return {@code Long.MAX_VALUE} instead of a negative number.
+     */
+    static long adjustForHugeFilesystems(long bytes) {
         if (bytes < 0) {
             return Long.MAX_VALUE;
         }
diff --git a/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java b/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java
index 007b6ce1fc776..3d4903e04724d 100644
--- a/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java
+++ b/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java
@@ -40,6 +40,7 @@
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.lessThan;
 import static org.hamcrest.Matchers.isEmptyOrNullString;
 import static org.hamcrest.Matchers.not;
 
@@ -91,6 +92,29 @@ public void testFsInfo() throws IOException {
         }
     }
 
+    public void testFsInfoOverflow() throws Exception {
+        FsInfo.Path pathStats = new FsInfo.Path("/foo/bar", null,
+                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+
+        // While not overflowing, keep adding
+        FsInfo.Path pathToAdd = new FsInfo.Path("/foo/baz", null,
+                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+        while ((pathStats.total + pathToAdd.total) > 0) {
+            // Add itself as a path, to increase the total bytes until it overflows
+            logger.info("--> adding {} bytes to {}, will be: {}", pathToAdd.total, pathStats.total, pathToAdd.total + pathStats.total);
+            pathStats.add(pathToAdd);
+            pathToAdd = new FsInfo.Path("/foo/baz", null,
+                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+        }
+
+        logger.info("--> adding {} bytes to {}, will be: {}", pathToAdd.total, pathStats.total, pathToAdd.total + pathStats.total);
+        assertThat(pathStats.total + pathToAdd.total, lessThan(0L));
+        pathStats.add(pathToAdd);
+
+        // Even after overflowing, it should not be negative
+        assertThat(pathStats.total, greaterThan(0L));
+    }
+
     public void testIoStats() {
         final AtomicReference<List<String>> diskStats = new AtomicReference<>();
         diskStats.set(Arrays.asList(