You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by an...@apache.org on 2026/09/30 15:29:24 UTC
(zookeeper) 03/05: Quota: protect quota-stats data with format validation
This is an automated email from the ASF dual-hosted git repository.
anmolnar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
commit 000ce4e7e8bb71502c3ea15b11847f5cd335a8d5
Author: Andor Molnar <an...@cloudera.com>
AuthorDate: Mon Sep 21 12:45:22 2026 -0500
Quota: protect quota-stats data with format validation
---
.../src/main/java/org/apache/zookeeper/Quotas.java | 11 +-
.../main/java/org/apache/zookeeper/StatsTrack.java | 74 ++++++++++++-
.../java/org/apache/zookeeper/server/DataTree.java | 55 +++++++---
.../zookeeper/server/PrepRequestProcessor.java | 46 ++++++++
.../org/apache/zookeeper/server/DataTreeTest.java | 118 +++++++++++++++++++++
.../java/org/apache/zookeeper/test/QuotasTest.java | 14 +++
.../org/apache/zookeeper/test/StatsTrackTest.java | 86 +++++++++++++++
7 files changed, 387 insertions(+), 17 deletions(-)
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/Quotas.java b/zookeeper-server/src/main/java/org/apache/zookeeper/Quotas.java
index eb83cfea54..1292b5df2c 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/Quotas.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/Quotas.java
@@ -78,9 +78,18 @@ public static String statPath(String path) {
* return the real path associated with this
* quotaPath.
* @param quotaPath the quotaPath which's started with /zookeeper/quota
- * @return the real path associated with this quotaPath.
+ * @return the real path associated with this quotaPath, or an empty string
+ * if the given path is not actually under /zookeeper/quota. Returning
+ * "" rather than throwing keeps a stray {@code zookeeper_limits} node
+ * elsewhere under /zookeeper (or the /zookeeper/quota root itself)
+ * from raising StringIndexOutOfBoundsException on the
+ * transaction-apply path, which would kill the SyncRequestProcessor
+ * critical thread and leave the dataDir unbootable on replay.
*/
public static String trimQuotaPath(String quotaPath) {
+ if (quotaPath == null || !quotaPath.startsWith(quotaZookeeper)) {
+ return "";
+ }
return quotaPath.substring(quotaZookeeper.length());
}
}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/StatsTrack.java b/zookeeper-server/src/main/java/org/apache/zookeeper/StatsTrack.java
index 02fffb753f..bd86422e73 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/StatsTrack.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/StatsTrack.java
@@ -26,12 +26,16 @@
import java.util.Objects;
import java.util.regex.Pattern;
import org.apache.zookeeper.common.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* a class that represents the stats associated with quotas
*/
public class StatsTrack {
+ private static final Logger LOG = LoggerFactory.getLogger(StatsTrack.class);
+
private static final String countStr = "count";
private static final String countHardLimitStr = "countHardLimit";
@@ -54,7 +58,11 @@ public StatsTrack() {
* @param stat the byte[] stat to be initialized with
*/
public StatsTrack(byte[] stat) {
- this(new String(stat, StandardCharsets.UTF_8));
+ // A null payload (jute length -1) must be tolerated here: this is on the
+ // transaction-apply/replay path, and new String(null, ...) would NPE,
+ // killing the SyncRequestProcessor critical thread and bricking the
+ // dataDir just like a malformed value. Treat null as empty/uninitialized.
+ this(stat == null ? null : new String(stat, StandardCharsets.UTF_8));
}
/**
@@ -70,9 +78,71 @@ public StatsTrack(String stat) {
}
String[] keyValuePairs = PAIRS_SEPARATOR.split(stat);
for (String keyValuePair : keyValuePairs) {
+ // NOTE: split with the default (limit 0) is intentional and must be
+ // kept: toString() emits a compatibility sentinel trailing '=' (e.g.
+ // "bytes=5=") that the default split silently drops, recovering "5".
+ // Changing the limit would break the round-trip of hard-limit values.
+ String[] kv = keyValuePair.split("=");
+ if (kv.length < 2 || StringUtils.isEmpty(kv[0])) {
+ // Malformed entry (e.g. "x", or "count=" with no value). This
+ // data is applied from an already-committed transaction, so
+ // throwing here would kill the SyncRequestProcessor critical
+ // thread and, because the transaction is durable, make the
+ // dataDir unbootable on replay. Skip the entry defensively
+ // instead (treated as uninitialized, i.e. -1); updateQuotaStat
+ // rewrites the node with a well-formed value, so it self-heals.
+ LOG.warn("Ignoring malformed quota stat entry: '{}'", keyValuePair);
+ continue;
+ }
+ String value = StringUtils.isEmpty(kv[1]) ? "-1" : kv[1];
+ try {
+ this.stats.put(kv[0], Long.parseLong(value));
+ } catch (NumberFormatException e) {
+ LOG.warn("Ignoring quota stat entry with non-numeric value: '{}'", keyValuePair);
+ }
+ }
+ }
+
+ /**
+ * Strictly validate that {@code data} is a well-formed stats/limits value,
+ * i.e. a (possibly empty) list of {@code key=long} pairs separated by
+ * {@code ,} or {@code ;}. Unlike the constructor, this performs no repair —
+ * it is used at request-preparation time to reject malformed writes to the
+ * quota {@code zookeeper_stats}/{@code zookeeper_limits} nodes before they
+ * are ever committed, so clients get a clear error and the quota bookkeeping
+ * never ingests garbage.
+ *
+ * @param data the candidate node data (UTF-8 encoded); empty is accepted
+ * (treated as uninitialized), null is rejected
+ * @return true if the data parses cleanly as StatsTrack content
+ */
+ public static boolean isValidStatsData(byte[] data) {
+ if (data == null) {
+ // null is never legitimate for a stats/limits node and NPEs the
+ // byte[] constructor on the apply path — reject it at prep time.
+ return false;
+ }
+ if (data.length == 0) {
+ return true;
+ }
+ String stat = new String(data, StandardCharsets.UTF_8);
+ String[] keyValuePairs = PAIRS_SEPARATOR.split(stat);
+ for (String keyValuePair : keyValuePairs) {
+ // Same tokenization as the constructor so the compatibility sentinel
+ // trailing '=' (e.g. "bytes=5=") is accepted, not rejected.
String[] kv = keyValuePair.split("=");
- this.stats.put(kv[0], Long.parseLong(StringUtils.isEmpty(kv[1]) ? "-1" : kv[1]));
+ if (kv.length < 2 || StringUtils.isEmpty(kv[0])) {
+ return false;
+ }
+ if (!StringUtils.isEmpty(kv[1])) {
+ try {
+ Long.parseLong(kv[1]);
+ } catch (NumberFormatException e) {
+ return false;
+ }
+ }
}
+ return true;
}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
index 20402156fe..fa0c53a425 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
@@ -499,14 +499,25 @@ public void createNode(final String path, byte[] data, List<ACL> acl, long ephem
}
// now check if its one of the zookeeper node child
if (parentName.startsWith(quotaZookeeper)) {
- // now check if it's the limit node
- if (Quotas.limitNode.equals(childName)) {
- // this is the limit node
- // get the parent and add it to the trie
- pTrie.addPath(Quotas.trimQuotaPath(parentName));
- }
- if (Quotas.statNode.equals(childName)) {
- updateQuotaForPath(Quotas.trimQuotaPath(parentName));
+ boolean isLimitNode = Quotas.limitNode.equals(childName);
+ boolean isStatNode = Quotas.statNode.equals(childName);
+ if (isLimitNode || isStatNode) {
+ // The quota namespace between /zookeeper/quota and the
+ // limit/stat node must be non-empty. A limit/stat node created
+ // directly under /zookeeper/quota trims to "", and
+ // PathTrie.addPath("") throws, killing the SyncRequestProcessor
+ // critical thread on the apply path (and unbootable on replay).
+ String quotaPrefix = Quotas.trimQuotaPath(parentName);
+ if (quotaPrefix.isEmpty()) {
+ LOG.warn("Ignoring quota {} node with empty namespace under {}",
+ isLimitNode ? "limit" : "stat", parentName);
+ } else if (isLimitNode) {
+ // this is the limit node
+ // get the parent and add it to the trie
+ pTrie.addPath(quotaPrefix);
+ } else {
+ updateQuotaForPath(quotaPrefix);
+ }
}
}
@@ -590,10 +601,19 @@ public void deleteNode(String path, long zxid) throws NoNodeException {
}
}
- if (parentName.startsWith(procZookeeper) && Quotas.limitNode.equals(childName)) {
- // delete the node in the trie.
- // we need to update the trie as well
- pTrie.deletePath(Quotas.trimQuotaPath(parentName));
+ // Only limit nodes actually under /zookeeper/quota are ever registered
+ // in the path trie (createNode uses the same prefix). Mirror that prefix
+ // here — using the wider /zookeeper prefix let a stray zookeeper_limits
+ // node elsewhere under /zookeeper reach trimQuotaPath and throw
+ // StringIndexOutOfBoundsException on the apply path. The empty-namespace
+ // case (node directly under /zookeeper/quota) is skipped too, since
+ // deletePath("") would likewise throw.
+ if (parentName.startsWith(quotaZookeeper) && Quotas.limitNode.equals(childName)) {
+ String quotaPrefix = Quotas.trimQuotaPath(parentName);
+ if (!quotaPrefix.isEmpty()) {
+ // delete the node in the trie; we need to update the trie as well
+ pTrie.deletePath(quotaPrefix);
+ }
}
// also check to update the quotas for this node
@@ -1260,8 +1280,15 @@ private void traverseNode(String path) {
// get the real node and update
// the count and the bytes
String realPath = path.substring(Quotas.quotaZookeeper.length(), path.indexOf(endString));
- updateQuotaForPath(realPath);
- this.pTrie.addPath(realPath);
+ // A limit node directly under /zookeeper/quota yields an empty
+ // realPath; pTrie.addPath("") throws and aborts startup while
+ // rebuilding the trie from a snapshot. Skip such stray nodes.
+ if (!realPath.isEmpty()) {
+ updateQuotaForPath(realPath);
+ this.pTrie.addPath(realPath);
+ } else {
+ LOG.warn("Ignoring quota limit node with empty namespace: {}", path);
+ }
}
return;
}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java
index 0eb266bb7e..8aa4375b5d 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java
@@ -38,6 +38,8 @@
import org.apache.zookeeper.KeeperException.Code;
import org.apache.zookeeper.MultiOperationRecord;
import org.apache.zookeeper.Op;
+import org.apache.zookeeper.Quotas;
+import org.apache.zookeeper.StatsTrack;
import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.ZooDefs.OpCode;
import org.apache.zookeeper.common.ConfigException;
@@ -381,6 +383,7 @@ protected void pRequest2Txn(int type, long zxid, Request request, Record record)
validatePath(path, request.sessionId);
nodeRecord = getRecordForPath(path);
zks.checkACL(request.cnxn, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo, path, null);
+ validateQuotaData(path, setDataRequest.getData());
zks.checkQuota(path, nodeRecord.data, setDataRequest.getData(), OpCode.setData);
int newVersion = checkAndIncVersion(nodeRecord.stat.getVersion(), setDataRequest.getVersion(), path);
request.setTxn(new SetDataTxn(path, setDataRequest.getData(), newVersion));
@@ -687,6 +690,7 @@ private void pRequest2TxnCreate(int type, Request request, Record record) throws
throw new KeeperException.NoChildrenForEphemeralsException(path);
}
int newCversion = parentRecord.stat.getCversion() + 1;
+ validateQuotaData(path, data);
zks.checkQuota(path, null, data, OpCode.create);
if (type == OpCode.createContainer) {
request.setTxn(new CreateContainerTxn(path, data, listACL, newCversion));
@@ -731,6 +735,48 @@ private void validatePath(String path, long sessionId) throws BadArgumentsExcept
}
}
+ /**
+ * Reject a create/setData whose target is a quota {@code zookeeper_stats} or
+ * {@code zookeeper_limits} node when either the node is placed directly under
+ * {@code /zookeeper/quota} (an empty namespace) or its data is not well-formed
+ * StatsTrack content. These nodes live under the world-writable
+ * {@code /zookeeper/quota} subtree and are consumed on the transaction-apply
+ * path ({@link org.apache.zookeeper.server.DataTree#updateQuotaStat} and the
+ * path-trie registration); rejecting bad writes here stops them from ever
+ * being committed. The apply path is additionally hardened in
+ * {@link StatsTrack} and {@link DataTree}, so this is defense-in-depth rather
+ * than the sole guard.
+ *
+ * @param path the target node path
+ * @param data the proposed node data
+ * @throws BadArgumentsException if the path is a quota stat/limit node with
+ * an empty namespace or malformed {@code data}
+ */
+ private void validateQuotaData(String path, byte[] data) throws BadArgumentsException {
+ if (path == null || !path.startsWith(Quotas.quotaZookeeper + "/")) {
+ return;
+ }
+ boolean isStatNode = path.endsWith("/" + Quotas.statNode);
+ boolean isLimitNode = path.endsWith("/" + Quotas.limitNode);
+ if (!isStatNode && !isLimitNode) {
+ return;
+ }
+ // A stat/limit node must sit under a non-empty namespace, i.e.
+ // /zookeeper/quota/<ns>/zookeeper_{stats,limits}. Placed directly under
+ // /zookeeper/quota it trims to an empty prefix and crashes the apply
+ // path (PathTrie.addPath("")).
+ String parent = path.substring(0, path.lastIndexOf('/'));
+ if (Quotas.trimQuotaPath(parent).isEmpty()) {
+ LOG.warn("Rejecting quota {} node with empty namespace: {}",
+ isLimitNode ? "limit" : "stat", path);
+ throw new BadArgumentsException(path);
+ }
+ if (!StatsTrack.isValidStatsData(data)) {
+ LOG.warn("Rejecting malformed quota data write to {}", path);
+ throw new BadArgumentsException(path);
+ }
+ }
+
private String getParentPathAndValidate(String path) throws BadArgumentsException {
int lastSlash = path.lastIndexOf('/');
if (lastSlash == -1 || path.indexOf('\0') != -1 || zks.getZKDatabase().isSpecialPath(path)) {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java
index 89a2f17b75..57f8de4171 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java
@@ -45,6 +45,7 @@
import org.apache.zookeeper.KeeperException.NoNodeException;
import org.apache.zookeeper.KeeperException.NodeExistsException;
import org.apache.zookeeper.Quotas;
+import org.apache.zookeeper.StatsTrack;
import org.apache.zookeeper.ZKTestCase;
import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.common.PathTrie;
@@ -687,4 +688,121 @@ private void testSerializeLastProcessedZxid(boolean enableForSerialize, boolean
}
}
+ /**
+ * Regression test for the quota-stats poisoning crash: malformed data
+ * committed to a {@code zookeeper_stats} node must not throw on the
+ * transaction-apply path (updateQuotaStat), which previously killed the
+ * SyncRequestProcessor critical thread and left the dataDir unbootable.
+ * The node must additionally self-heal into a well-formed value.
+ */
+ @Test
+ @Timeout(value = 60)
+ public void testUpdateQuotaStatSurvivesMalformedStatsData() throws Exception {
+ DataTree dt = new DataTree();
+
+ // register /ns as a quota namespace (limit node create adds it to the trie)
+ dt.createNode("/ns", new byte[0], null, -1, 1, 1, 1);
+ dt.createNode(Quotas.quotaPath("/ns"), null, null, -1, 1, 1, 1);
+ dt.createNode(Quotas.limitPath("/ns"),
+ new StatsTrack("count=10,bytes=1000").getStatsBytes(), null, -1, 1, 1, 1);
+ dt.createNode(Quotas.statPath("/ns"),
+ new StatsTrack().getStatsBytes(), null, -1, 1, 1, 1);
+
+ // poison: commit malformed data to the stats node (as an unauthenticated
+ // setData would). This write is not itself under the quota prefix, so it
+ // stores silently without triggering updateQuotaStat.
+ dt.setData(Quotas.statPath("/ns"), "x".getBytes(), 1, 2, 1);
+ assertEquals("x", new String(dt.getNode(Quotas.statPath("/ns")).data));
+
+ // detonating write: an ordinary create under the namespace re-parses the
+ // poisoned stats node in updateQuotaStat. Must NOT throw.
+ dt.createNode("/ns/child", new byte[3], null, -1, 2, 3, 1);
+
+ // the stats node self-healed into a well-formed StatsTrack value
+ byte[] healed = dt.getNode(Quotas.statPath("/ns")).data;
+ assertTrue(StatsTrack.isValidStatsData(healed),
+ "stats node should be well-formed after recovery, was: " + new String(healed));
+
+ // a further write still succeeds (no lingering poison)
+ dt.setData("/ns/child", new byte[5], 1, 4, 1);
+ assertTrue(StatsTrack.isValidStatsData(dt.getNode(Quotas.statPath("/ns")).data));
+ }
+
+ /**
+ * Companion to the above for a null stats payload (jute length -1): the
+ * byte[] StatsTrack constructor must not NPE on the apply/replay path.
+ */
+ @Test
+ @Timeout(value = 60)
+ public void testUpdateQuotaStatSurvivesNullStatsData() throws Exception {
+ DataTree dt = new DataTree();
+ dt.createNode("/ns", new byte[0], null, -1, 1, 1, 1);
+ dt.createNode(Quotas.quotaPath("/ns"), null, null, -1, 1, 1, 1);
+ dt.createNode(Quotas.limitPath("/ns"),
+ new StatsTrack("count=10,bytes=1000").getStatsBytes(), null, -1, 1, 1, 1);
+ dt.createNode(Quotas.statPath("/ns"),
+ new StatsTrack().getStatsBytes(), null, -1, 1, 1, 1);
+
+ // poison the stats node with a null payload
+ dt.setData(Quotas.statPath("/ns"), null, 1, 2, 1);
+ assertNull(dt.getNode(Quotas.statPath("/ns")).data);
+
+ // detonating write must not NPE in StatsTrack(byte[])
+ dt.createNode("/ns/child", new byte[3], null, -1, 2, 3, 1);
+ assertTrue(StatsTrack.isValidStatsData(dt.getNode(Quotas.statPath("/ns")).data));
+ }
+
+ /**
+ * A quota limit/stat node created directly under /zookeeper/quota trims to
+ * an empty namespace; previously PathTrie.addPath("") (and, on reload,
+ * traverseNode) threw and bricked the server. Creating them must now be a
+ * no-op registration rather than a crash.
+ */
+ @Test
+ @Timeout(value = 60)
+ public void testCreateQuotaNodeWithEmptyNamespaceDoesNotCrash() throws Exception {
+ DataTree dt = new DataTree();
+
+ // limit node directly under /zookeeper/quota (empty namespace)
+ dt.createNode(Quotas.quotaZookeeper + "/" + Quotas.limitNode,
+ new StatsTrack("count=10").getStatsBytes(), null, -1, 1, 1, 1);
+ // stat node variant (updateQuotaForPath(""))
+ dt.createNode(Quotas.quotaZookeeper + "/" + Quotas.statNode,
+ new StatsTrack().getStatsBytes(), null, -1, 1, 2, 1);
+
+ // no bogus empty prefix registered -> ordinary writes keep working
+ dt.createNode("/ok", new byte[1], null, -1, 1, 3, 1);
+ assertNotNull(dt.getNode("/ok"));
+
+ // and the stray limit node can be deleted without crashing the trie
+ dt.deleteNode(Quotas.quotaZookeeper + "/" + Quotas.limitNode, 4);
+ assertNull(dt.getNode(Quotas.quotaZookeeper + "/" + Quotas.limitNode));
+ }
+
+ /**
+ * A {@code zookeeper_limits} node created directly under /zookeeper (a
+ * sibling of /zookeeper/quota, not under it) passes the create-side quota
+ * guard silently, but deleting it previously reached
+ * Quotas.trimQuotaPath("/zookeeper") — substring(16) on a 10-char string —
+ * and threw StringIndexOutOfBoundsException on the apply path, permanently
+ * bricking the dataDir. Neither create nor delete may crash. Empty data.
+ */
+ @Test
+ @Timeout(value = 60)
+ public void testLimitNodeDirectlyUnderZookeeperDoesNotCrash() throws Exception {
+ DataTree dt = new DataTree();
+
+ String strayLimit = Quotas.procZookeeper + "/" + Quotas.limitNode; // /zookeeper/zookeeper_limits
+ dt.createNode(strayLimit, new byte[0], null, -1, 1, 1, 1);
+ assertNotNull(dt.getNode(strayLimit));
+
+ // the detonating delete must not throw
+ dt.deleteNode(strayLimit, 2);
+ assertNull(dt.getNode(strayLimit));
+
+ // server still healthy for ordinary writes
+ dt.createNode("/ok", new byte[1], null, -1, 1, 3, 1);
+ assertNotNull(dt.getNode("/ok"));
+ }
+
}
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuotasTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuotasTest.java
index b887e28fba..79111028f9 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuotasTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuotasTest.java
@@ -46,5 +46,19 @@ public void testQuotaPathPath() {
public void testTrimQuotaPath() {
assertEquals("/foo", Quotas.trimQuotaPath("/zookeeper/quota/foo"));
assertEquals("/bar", Quotas.trimQuotaPath("/zookeeper/quota/bar"));
+ assertEquals("/a/b", Quotas.trimQuotaPath("/zookeeper/quota/a/b"));
+ }
+
+ @Test
+ public void testTrimQuotaPathIsBoundsSafe() {
+ // Paths not under /zookeeper/quota must not throw
+ // StringIndexOutOfBoundsException (they reach this method via the
+ // transaction-apply path and would otherwise kill the server).
+ assertEquals("", Quotas.trimQuotaPath("/zookeeper"));
+ assertEquals("", Quotas.trimQuotaPath("/z"));
+ assertEquals("", Quotas.trimQuotaPath(""));
+ assertEquals("", Quotas.trimQuotaPath("/some/other/path"));
+ // the /zookeeper/quota root itself trims to an empty namespace
+ assertEquals("", Quotas.trimQuotaPath("/zookeeper/quota"));
}
}
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/StatsTrackTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/StatsTrackTest.java
index 4bf69befa3..5fae21cf54 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/StatsTrackTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/StatsTrackTest.java
@@ -132,4 +132,90 @@ public void testUpwardCompatibility() {
Assert.assertEquals(-1, st.getByteHardLimit());
Assert.assertEquals(-1, st.getCountHardLimit());
}
+
+ // ------------------------------------------------------------------
+ // Hardening against malformed quota-stats data (ZOOKEEPER quota-poison):
+ // a StatsTrack parsed on the transaction-apply path must never throw, or
+ // it kills the SyncRequestProcessor critical thread and, since the trigger
+ // txn is durable, the dataDir becomes unbootable on replay.
+ // ------------------------------------------------------------------
+
+ @Test
+ public void testMalformedValueNoDelimiterDoesNotThrow() {
+ // "x" has no '=' -> previously ArrayIndexOutOfBoundsException at kv[1]
+ StatsTrack st = new StatsTrack("x");
+ Assert.assertEquals(-1, st.getCount());
+ Assert.assertEquals(-1, st.getBytes());
+ }
+
+ @Test
+ public void testMalformedNonNumericValueDoesNotThrow() {
+ // "count=abc" -> previously NumberFormatException
+ StatsTrack st = new StatsTrack("count=abc");
+ Assert.assertEquals(-1, st.getCount());
+ }
+
+ @Test
+ public void testMalformedFromBytesDoesNotThrow() {
+ StatsTrack st = new StatsTrack("x".getBytes());
+ Assert.assertEquals(-1, st.getCount());
+ Assert.assertEquals(-1, st.getBytes());
+ }
+
+ @Test
+ public void testNullBytesDoesNotThrow() {
+ // null payload (jute length -1) must not NPE new String(null, ...)
+ StatsTrack st = new StatsTrack((byte[]) null);
+ Assert.assertEquals(-1, st.getCount());
+ Assert.assertEquals(-1, st.getBytes());
+ }
+
+ @Test
+ public void testPartiallyMalformedKeepsGoodEntries() {
+ // good entries survive, only the bad pair is skipped
+ StatsTrack st = new StatsTrack("count=5,bytes=zzz");
+ Assert.assertEquals(5, st.getCount());
+ Assert.assertEquals(-1, st.getBytes());
+ }
+
+ @Test
+ public void testEmptyAndNullDoNotThrow() {
+ Assert.assertEquals(-1, new StatsTrack("").getCount());
+ Assert.assertEquals(-1, new StatsTrack((String) null).getCount());
+ }
+
+ @Test
+ public void testHardLimitRoundTripStillWorks() {
+ // Guards the compatibility sentinel trailing '=' produced by toString():
+ // parsing must recover every field, including the hard limits.
+ StatsTrack quota = new StatsTrack();
+ quota.setCount(4);
+ quota.setCountHardLimit(4);
+ quota.setBytes(9L);
+ quota.setByteHardLimit(15L);
+ String serialized = quota.toString();
+ Assert.assertEquals("count=4,bytes=9=;byteHardLimit=15;countHardLimit=4", serialized);
+
+ StatsTrack reparsed = new StatsTrack(serialized);
+ Assert.assertEquals(4, reparsed.getCount());
+ Assert.assertEquals(9L, reparsed.getBytes());
+ Assert.assertEquals(15L, reparsed.getByteHardLimit());
+ Assert.assertEquals(4, reparsed.getCountHardLimit());
+ }
+
+ @Test
+ public void testIsValidStatsData() {
+ // well-formed, including the compatibility sentinel format
+ Assert.assertTrue(StatsTrack.isValidStatsData(new byte[0]));
+ Assert.assertTrue(StatsTrack.isValidStatsData("count=5,bytes=10".getBytes()));
+ Assert.assertTrue(StatsTrack.isValidStatsData(
+ "count=4,bytes=9=;byteHardLimit=15;countHardLimit=4".getBytes()));
+
+ // malformed -> rejected at prep time
+ Assert.assertFalse(StatsTrack.isValidStatsData(null));
+ Assert.assertFalse(StatsTrack.isValidStatsData("x".getBytes()));
+ Assert.assertFalse(StatsTrack.isValidStatsData("count=abc".getBytes()));
+ Assert.assertFalse(StatsTrack.isValidStatsData("=5".getBytes()));
+ Assert.assertFalse(StatsTrack.isValidStatsData("count=5,x".getBytes()));
+ }
}