Skip to content

Commit 4925aff

Browse files
Address PR review comments
- serverStatus.jsp: wrap dynamic substitutions in <c:out> so exception messages and monitor stats can't smuggle HTML. Closes the rmiError XSS path; the rest is defense in depth. - DatabaseQueueMonitor: snapshot queueLevel/queueSize once so the message, stats, CloudWatch metric, and threshold check all see the same observation. - SystemCpuMonitor: render "N/A" instead of a negative percentage when JMX can't sample the CPU load. - PredictabilityMonitor: drop duplicate ArrayList/Collection/List imports left over from a 2015 merge.
1 parent 8367286 commit 4925aff

4 files changed

Lines changed: 23 additions & 20 deletions

File tree

transitclock/src/main/java/org/transitclock/monitoring/DatabaseQueueMonitor.java

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,30 +73,36 @@ protected boolean triggered() {
7373
return false;
7474

7575
DataDbLogger dbLogger = core.getDbLogger();
76-
76+
77+
// Snapshot once: queueLevel/queueSize change under load and we want
78+
// the message, stats, CloudWatch metric, and threshold check to
79+
// agree on a single observation.
80+
double queueLevel = dbLogger.queueLevel();
81+
int queueSize = dbLogger.queueSize();
82+
7783
setMessage("Database queue fraction="
78-
+ StringUtils.twoDigitFormat(dbLogger.queueLevel())
84+
+ StringUtils.twoDigitFormat(queueLevel)
7985
+ " while max allowed fraction="
8086
+ StringUtils.twoDigitFormat(maxQueueFraction.getValue())
81-
+ ", and items in queue=" + dbLogger.queueSize()
87+
+ ", and items in queue=" + queueSize
8288
+ ".",
83-
dbLogger.queueLevel());
89+
queueLevel);
8490

85-
addStat("Queue level", StringUtils.percentFormat(dbLogger.queueLevel()));
91+
addStat("Queue level", StringUtils.percentFormat(queueLevel));
8692
addStat("Max allowed", StringUtils.percentFormat(maxQueueFraction.getValue()));
87-
addStat("Items queued", String.valueOf(dbLogger.queueSize()));
93+
addStat("Items queued", String.valueOf(queueSize));
94+
95+
cloudwatchService.saveMetric("PredictionDatabaseQueuePercentageLevel", queueLevel, 1, CloudwatchService.MetricType.AVERAGE, CloudwatchService.ReportingIntervalTimeUnit.MINUTE, false);
8896

89-
cloudwatchService.saveMetric("PredictionDatabaseQueuePercentageLevel", dbLogger.queueLevel(), 1, CloudwatchService.MetricType.AVERAGE, CloudwatchService.ReportingIntervalTimeUnit.MINUTE, false);
90-
9197
// Determine the threshold for triggering. If already triggered
9298
// then lower the threshold by maxQueueFractionGap in order
9399
// to prevent lots of e-mail being sent out if the value is
94100
// dithering around maxQueueFraction.
95101
double threshold = maxQueueFraction.getValue();
96102
if (wasTriggered())
97103
threshold -= maxQueueFractionGap.getValue();
98-
99-
return dbLogger.queueLevel() > threshold;
104+
105+
return queueLevel > threshold;
100106
}
101107

102108
/* (non-Javadoc)

transitclock/src/main/java/org/transitclock/monitoring/PredictabilityMonitor.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@
2929
import org.transitclock.utils.EmailSender;
3030
import org.transitclock.utils.StringUtils;
3131

32-
import java.util.ArrayList;
33-
import java.util.Collection;
34-
import java.util.List;
35-
3632
/**
3733
* Monitors how many vehicles are predictable compared to how many active blocks
3834
* there currently are.

transitclock/src/main/java/org/transitclock/monitoring/SystemCpuMonitor.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ protected boolean triggered() {
121121
+ ".",
122122
cpuLoad);
123123

124-
addStat("CPU load", StringUtils.percentFormat(cpuLoad));
124+
// JMX returns a negative value when CPU load cannot be sampled.
125+
addStat("CPU load", cpuLoad >= 0.0 ? StringUtils.percentFormat(cpuLoad) : "N/A");
125126
addStat("Limit", StringUtils.percentFormat(cpuThreshold.getValue()));
126127

127128
// Determine the threshold for triggering. If already triggered

transitclockWebapp/src/main/webapp/status/serverStatus.jsp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ try {
5757
<c:when test="${not empty rmiError}">
5858
<div class="rounded-lg border border-red-200 bg-red-50 p-4">
5959
<h2 class="text-sm font-semibold text-red-900"><fmt:message key="div.coreUnreachable" /></h2>
60-
<p class="mt-1 text-sm text-red-800 font-mono break-all">${rmiError}</p>
60+
<p class="mt-1 text-sm text-red-800 font-mono break-all"><c:out value="${rmiError}"/></p>
6161
</div>
6262
</c:when>
6363
<c:otherwise>
@@ -66,22 +66,22 @@ try {
6666
<c:if test="${not empty monitorResult.message}">
6767
<article class="bg-white border border-gray-200 rounded-lg p-4">
6868
<h2 class="text-xs font-semibold uppercase tracking-wider text-gray-500">
69-
${monitorResult.type}
69+
<c:out value="${monitorResult.type}"/>
7070
</h2>
7171
<c:choose>
7272
<c:when test="${not empty monitorResult.stats}">
7373
<dl class="mt-3 grid grid-cols-2 gap-x-4 gap-y-1.5 text-sm">
7474
<c:forEach var="stat" items="${monitorResult.stats}">
7575
<div class="flex items-baseline gap-1.5 col-span-2 sm:col-span-1">
76-
<dt class="text-gray-500">${stat.key}:</dt>
77-
<dd class="font-mono text-gray-900">${stat.value}</dd>
76+
<dt class="text-gray-500"><c:out value="${stat.key}"/>:</dt>
77+
<dd class="font-mono text-gray-900"><c:out value="${stat.value}"/></dd>
7878
</div>
7979
</c:forEach>
8080
</dl>
8181
</c:when>
8282
<c:otherwise>
8383
<p class="mt-2 text-sm text-gray-900 leading-relaxed tabular-nums">
84-
${monitorResult.message}
84+
<c:out value="${monitorResult.message}"/>
8585
</p>
8686
</c:otherwise>
8787
</c:choose>

0 commit comments

Comments
 (0)