Do not crash on inaccurate progress reporting

jib-core/src/main/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcher.java

@@ -20,7 +20,6 @@
 import com.google.cloud.tools.jib.event.events.ProgressEvent;
 import com.google.cloud.tools.jib.event.progress.Allocation;
 import com.google.common.base.Preconditions;
-import com.google.common.base.Verify;
 import java.io.Closeable;
 
 /**
@@ -139,18 +138,28 @@ public void close() {
    * @param progressUnits units of progress
    */
   public void dispatchProgress(long progressUnits) {
-    decrementRemainingAllocationUnits(progressUnits);
-    eventDispatcher.dispatch(new ProgressEvent(allocation, progressUnits, buildStepType));
+    long unitsDecremented = decrementRemainingAllocationUnits(progressUnits);
+    eventDispatcher.dispatch(new ProgressEvent(allocation, unitsDecremented, buildStepType));
   }
 
-  private void decrementRemainingAllocationUnits(long units) {
+  /**
+   * Decrements remaining allocation units by {@code units} but no more than the remaining
+   * allocation units (which may be 0). Returns the actual units decremented, which never exceeds
+   * {@code units}.
+   *
+   * @param units units to decrement
+   * @return units actually decremented
+   */
+  private long decrementRemainingAllocationUnits(long units) {
     Preconditions.checkState(!closed);
 
-    remainingAllocationUnits -= units;
-    Verify.verify(
-        remainingAllocationUnits >= 0,
-        "Remaining allocation units less than 0 for '%s': %s",
-        allocation.getDescription(),
-        remainingAllocationUnits);
+    if (remainingAllocationUnits > units) {
+      remainingAllocationUnits -= units;
+      return units;
+    }
+
+    long actualDecrement = remainingAllocationUnits;
+    remainingAllocationUnits = 0;
+    return actualDecrement;
   }
 }

jib-core/src/main/java/com/google/cloud/tools/jib/event/progress/Allocation.java

@@ -65,7 +65,7 @@ public static Allocation newRoot(String description, long allocationUnits) {
 
   private Allocation(String description, long allocationUnits, @Nullable Allocation parent) {
     this.description = description;
-    this.allocationUnits = allocationUnits;
+    this.allocationUnits = allocationUnits < 0 ? 0 : allocationUnits;
     this.parent = parent;
 
     this.fractionOfRoot = (parent == null ? 1.0 : parent.fractionOfRoot) / allocationUnits;

jib-core/src/test/java/com/google/cloud/tools/jib/builder/ProgressEventDispatcherTest.java

@@ -18,7 +18,6 @@
 
 import com.google.cloud.tools.jib.event.EventDispatcher;
 import com.google.cloud.tools.jib.event.events.ProgressEvent;
-import com.google.common.base.VerifyException;
 import java.util.List;
 import org.junit.Assert;
 import org.junit.Test;
@@ -59,37 +58,58 @@ public void testDispatch() {
   }
 
   @Test
-  public void testDispatch_tooMuchProgress() {
+  public void testDispatch_safeWithtooMuchProgress() {
     try (ProgressEventDispatcher progressEventDispatcher =
         ProgressEventDispatcher.newRoot(mockEventDispatcher, "allocation description", 10)) {
-      progressEventDispatcher.dispatchProgress(10);
-      try {
-        progressEventDispatcher.dispatchProgress(10);
-        Assert.fail();
-
-      } catch (VerifyException ex) {
-        Assert.assertEquals(
-            "Remaining allocation units less than 0 for 'allocation description': -10",
-            ex.getMessage());
-      }
+      progressEventDispatcher.dispatchProgress(6);
+      progressEventDispatcher.dispatchProgress(8);
+      progressEventDispatcher.dispatchProgress(1);
     }
+
+    ArgumentCaptor<ProgressEvent> eventsCaptor = ArgumentCaptor.forClass(ProgressEvent.class);
+    Mockito.verify(mockEventDispatcher, Mockito.times(4)).dispatch(eventsCaptor.capture());
+    List<ProgressEvent> progressEvents = eventsCaptor.getAllValues();
+
+    Assert.assertSame(progressEvents.get(0).getAllocation(), progressEvents.get(1).getAllocation());
+    Assert.assertSame(progressEvents.get(1).getAllocation(), progressEvents.get(2).getAllocation());
+    Assert.assertSame(progressEvents.get(2).getAllocation(), progressEvents.get(3).getAllocation());
+
+    Assert.assertEquals(10, progressEvents.get(0).getAllocation().getAllocationUnits());
+
+    Assert.assertEquals(0, progressEvents.get(0).getUnits());
+    Assert.assertEquals(6, progressEvents.get(1).getUnits());
+    Assert.assertEquals(4, progressEvents.get(2).getUnits());
+    Assert.assertEquals(0, progressEvents.get(3).getUnits());
   }
 
   @Test
-  public void testDispatch_tooManyChildren() {
+  public void testDispatch_safeWithTooManyChildren() {
     try (ProgressEventDispatcher progressEventDispatcher =
-        ProgressEventDispatcher.newRoot(mockEventDispatcher, "allocation description", 1)) {
-      progressEventDispatcher.newChildProducer();
-
-      try {
-        progressEventDispatcher.newChildProducer();
-        Assert.fail();
-
-      } catch (VerifyException ex) {
-        Assert.assertEquals(
-            "Remaining allocation units less than 0 for 'allocation description': -1",
-            ex.getMessage());
-      }
+            ProgressEventDispatcher.newRoot(mockEventDispatcher, "allocation description", 1);
+        ProgressEventDispatcher ignored1 =
+            progressEventDispatcher.newChildProducer().create(BuildStepType.ALL, "ignored", 5);
+        ProgressEventDispatcher ignored2 =
+            progressEventDispatcher.newChildProducer().create(BuildStepType.ALL, "ignored", 4)) {
+      // empty
     }
+
+    ArgumentCaptor<ProgressEvent> eventsCaptor = ArgumentCaptor.forClass(ProgressEvent.class);
+    Mockito.verify(mockEventDispatcher, Mockito.times(5)).dispatch(eventsCaptor.capture());
+    List<ProgressEvent> progressEvents = eventsCaptor.getAllValues();
+
+    Assert.assertEquals(1, progressEvents.get(0).getAllocation().getAllocationUnits());
+    Assert.assertEquals(5, progressEvents.get(1).getAllocation().getAllocationUnits());
+    Assert.assertEquals(4, progressEvents.get(2).getAllocation().getAllocationUnits());
+
+    // child1 (of allocation 5) opening and closing
+    Assert.assertSame(progressEvents.get(1).getAllocation(), progressEvents.get(4).getAllocation());
+    // child1 (of allocation 4) opening and closing
+    Assert.assertSame(progressEvents.get(2).getAllocation(), progressEvents.get(3).getAllocation());
+
+    Assert.assertEquals(0, progressEvents.get(0).getUnits()); // 0-progress sent when root creation
+    Assert.assertEquals(0, progressEvents.get(1).getUnits()); // when child1 creation
+    Assert.assertEquals(0, progressEvents.get(2).getUnits()); // when child2 creation
+    Assert.assertEquals(4, progressEvents.get(3).getUnits()); // when child2 closes
+    Assert.assertEquals(5, progressEvents.get(4).getUnits()); // when child1 closes
   }
 }

jib-gradle-plugin/CHANGELOG.md

@@ -11,7 +11,8 @@ All notable changes to this project will be documented in this file.
 
 ### Fixed
 
-- Fixed an issue where setting `allowInsecureRegistries` may fail to try HTTP.
+- Fixed an issue where setting `allowInsecureRegistries` may fail to try HTTP. ([#1517](https://github.com/GoogleContainerTools/jib/issues/1517))
+- Crash on talking to servers that do not set or send incorrect `Content-Length` HTTP header value. ([#1512](https://github.com/GoogleContainerTools/jib/issues/1512))
 
 ## 1.0.1
 

jib-maven-plugin/CHANGELOG.md

@@ -11,7 +11,8 @@ All notable changes to this project will be documented in this file.
 
 ### Fixed
 
-- Fixed an issue where setting `<allowInsecureRegistries>` may fail to try HTTP.
+- Fixed an issue where setting `<allowInsecureRegistries>` may fail to try HTTP. ([#1517](https://github.com/GoogleContainerTools/jib/issues/1517))
+- Crash on talking to servers that do not set or send incorrect `Content-Length` HTTP header value. ([#1512](https://github.com/GoogleContainerTools/jib/issues/1512))
 
 ## 1.0.1