From 57cdc69738c2a0fe3a0a3476f250985f1795f838 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 6 Jan 2023 18:21:25 -0700 Subject: [PATCH 1/3] Fix integer overflow for czCount --- CHANGELOG.md | 1 + .../core/router/StatTracker.java | 7 ++- .../core/router/StatTrackerTest.java | 43 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index c83a70bb2f..9a0ccb6eb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7120](https://github.com/apache/trafficcontrol/pull/7120) *Docs* Update t3c documentation regarding parent.config parent_retry. ### Fixed +- [#7252](https://github.com/apache/trafficcontrol/issues/7252) *Traffic Router* Fixed integer overflow for `czCount`. - [#7246](https://github.com/apache/trafficcontrol/issues/7246) *Docs* Fixed docs for /jobs response description in APIv4 and APIv5. - [#6229](https://github.com/apache/trafficcontrol/issues/6229) *Traffic Ops* Fixed error message for assignment of non-existent parameters to a profile. - [#7231](https://github.com/apache/trafficcontrol/pull/7231) *Traffic Ops, Traffic Portal* Fixed `sharedUserNames` display while retrieving CDN locks. diff --git a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/router/StatTracker.java b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/router/StatTracker.java index 610a007d33..e587a4524f 100644 --- a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/router/StatTracker.java +++ b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/router/StatTracker.java @@ -373,7 +373,7 @@ public void saveTrack(final Track t) { totalDnsCount++; totalDnsTime += t.time; } - map = dnsMap; + map = getDnsMap(); if (t.resultDetails == Track.ResultDetails.LOCALIZED_DNS) { return; @@ -387,7 +387,7 @@ public void saveTrack(final Track t) { totalHttpCount++; totalHttpTime += t.time; } - map = httpMap; + map = getHttpMap(); } map.putIfAbsent(fqdn, new Tallies()); incTally(t, map.get(fqdn)); @@ -402,6 +402,9 @@ private static void incTally(final Track t, final Tallies tallies) { break; case CZ: tallies.czCount++; + if (tallies.czCount < 0) { + tallies.czCount = Integer.MAX_VALUE; + } break; case GEO: tallies.geoCount++; diff --git a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java new file mode 100644 index 0000000000..3e795ae049 --- /dev/null +++ b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java @@ -0,0 +1,43 @@ +/* + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.traffic_control.traffic_router.core.router; + +import org.junit.Test; +import java.util.HashMap; +import java.util.Map; +import static org.powermock.api.mockito.PowerMockito.spy; +import static org.powermock.api.mockito.PowerMockito.when; +import static org.springframework.test.util.AssertionErrors.assertEquals; + +public class StatTrackerTest { + @Test + public void testIncTally() { + StatTracker tracker = spy(new StatTracker()); + StatTracker.Tallies tallies = new StatTracker.Tallies(); + StatTracker.Track track = StatTracker.getTrack(); + + tallies.setCzCount(Integer.MAX_VALUE); + + Map map = new HashMap<>(); + map.put("blah", tallies); + when(tracker.getDnsMap()).thenReturn(map); + + track.setRouteType(StatTracker.Track.RouteType.DNS, "blah"); + track.setResult(StatTracker.Track.ResultType.CZ); + tracker.saveTrack(track); + assertEquals("expected czCount to be max integer value but got " + tallies.getCzCount(), Integer.MAX_VALUE, tallies.getCzCount()); + } +} From ae5bcd0526e7307cfd2de35054b2c90deb671cbd Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Mon, 9 Jan 2023 11:23:17 -0700 Subject: [PATCH 2/3] code review --- .../traffic_router/core/router/StatTracker.java | 8 ++++---- .../traffic_router/core/router/StatTrackerTest.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/router/StatTracker.java b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/router/StatTracker.java index e587a4524f..1f2e9b5c2b 100644 --- a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/router/StatTracker.java +++ b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/router/StatTracker.java @@ -36,10 +36,10 @@ public class StatTracker { public static class Tallies { - public int getCzCount() { + public long getCzCount() { return czCount; } - public void setCzCount(final int czCount) { + public void setCzCount(final long czCount) { this.czCount = czCount; } public int getGeoCount() { @@ -97,7 +97,7 @@ public void setRegionalAlternateCount(final int regionalAlternateCount) { this.regionalAlternateCount = regionalAlternateCount; } - public int czCount; + public long czCount; public int geoCount; public int deepCzCount; public int missCount; @@ -403,7 +403,7 @@ private static void incTally(final Track t, final Tallies tallies) { case CZ: tallies.czCount++; if (tallies.czCount < 0) { - tallies.czCount = Integer.MAX_VALUE; + tallies.czCount = Long.MAX_VALUE; } break; case GEO: diff --git a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java index 3e795ae049..99ed0d2b1a 100644 --- a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java +++ b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java @@ -29,7 +29,7 @@ public void testIncTally() { StatTracker.Tallies tallies = new StatTracker.Tallies(); StatTracker.Track track = StatTracker.getTrack(); - tallies.setCzCount(Integer.MAX_VALUE); + tallies.setCzCount(Long.MAX_VALUE); Map map = new HashMap<>(); map.put("blah", tallies); @@ -38,6 +38,6 @@ public void testIncTally() { track.setRouteType(StatTracker.Track.RouteType.DNS, "blah"); track.setResult(StatTracker.Track.ResultType.CZ); tracker.saveTrack(track); - assertEquals("expected czCount to be max integer value but got " + tallies.getCzCount(), Integer.MAX_VALUE, tallies.getCzCount()); + assertEquals("expected czCount to be max integer value but got " + tallies.getCzCount(), Long.MAX_VALUE, tallies.getCzCount()); } } From 41ecfa7c10ac9cb1a831890e1e472c6b92b57ae5 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Mon, 9 Jan 2023 11:45:16 -0700 Subject: [PATCH 3/3] fix changelog, test message --- CHANGELOG.md | 2 +- .../traffic_router/core/router/StatTrackerTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27f410ca8f..a266f123b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7120](https://github.com/apache/trafficcontrol/pull/7120) *Docs* Update t3c documentation regarding parent.config parent_retry. ### Fixed -- [#7252](https://github.com/apache/trafficcontrol/issues/7252) *Traffic Router* Fixed integer overflow for `czCount`. +- [#7252](https://github.com/apache/trafficcontrol/issues/7252) *Traffic Router* Fixed integer overflow for `czCount`, by resetting the count to max value when it overflows. - [#7221](https://github.com/apache/trafficcontrol/issues/7221) *Docs* Fixed request structure spec in cdn locks description in APIv4. - [#7225](https://github.com/apache/trafficcontrol/issues/7225) *Docs* Fixed docs for /roles response description in APIv4. - [#7246](https://github.com/apache/trafficcontrol/issues/7246) *Docs* Fixed docs for /jobs response description in APIv4 and APIv5. diff --git a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java index 99ed0d2b1a..3c8c8b0faf 100644 --- a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java +++ b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/router/StatTrackerTest.java @@ -38,6 +38,6 @@ public void testIncTally() { track.setRouteType(StatTracker.Track.RouteType.DNS, "blah"); track.setResult(StatTracker.Track.ResultType.CZ); tracker.saveTrack(track); - assertEquals("expected czCount to be max integer value but got " + tallies.getCzCount(), Long.MAX_VALUE, tallies.getCzCount()); + assertEquals("expected czCount to be max long value but got " + tallies.getCzCount(), Long.MAX_VALUE, tallies.getCzCount()); } }