Skip to content

Commit

Permalink
Try to repro #547, no success yet
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Jul 28, 2019
1 parent c8fe42d commit c0ffdc2
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -481,9 +481,8 @@ private String _addSymbol(char[] buffer, int start, int len, int h, int index)
_hashShared = false;
} else if (_size >= _sizeThreshold) { // Need to expand?
rehash();
/* Need to recalc hash; rare occurence (index mask has been
* recalculated as part of rehash)
*/
// Need to recalc hash; rare occurence (index mask has been
// recalculated as part of rehash)
index = _hashToIndex(calcHash(buffer, start, len));
}

Expand Down Expand Up @@ -604,7 +603,7 @@ private void copyArrays() {
* entries.
*/
private void rehash() {
int size = _symbols.length;
final int size = _symbols.length;
int newSize = size + size;

/* 12-Mar-2010, tatu: Let's actually limit maximum size we are
Expand All @@ -624,8 +623,8 @@ private void rehash() {
return;
}

String[] oldSyms = _symbols;
Bucket[] oldBuckets = _buckets;
final String[] oldSyms = _symbols;
final Bucket[] oldBuckets = _buckets;
_symbols = new String[newSize];
_buckets = new Bucket[newSize >> 1];
// Let's update index mask, threshold, now (needed for rehashing)
Expand Down Expand Up @@ -653,8 +652,8 @@ private void rehash() {
}
}

size >>= 1;
for (int i = 0; i < size; ++i) {
final int bucketSize = (size >> 1);
for (int i = 0; i < bucketSize; ++i) {
Bucket b = oldBuckets[i];
while (b != null) {
++count;
Expand Down Expand Up @@ -689,6 +688,36 @@ protected void reportTooManyCollisions(int maxLen) {
+") now exceeds maximum, "+maxLen+" -- suspect a DoS attack based on hash collisions");
}

// since 2.10, for tests only
/**
* Diagnostics method that will verify that internal data structures are consistent;
* not meant as user-facing method but only for test suites and possible troubleshooting.
*
* @since 2.10
*/
protected void verifyInternalConsistency() {
int count = 0;
final int size = _symbols.length;

for (int i = 0; i < size; ++i) {
String symbol = _symbols[i];
if (symbol != null) {
++count;
}
}

final int bucketSize = (size >> 1);
for (int i = 0; i < bucketSize; ++i) {
for (Bucket b = _buckets[i]; b != null; b = b.next) {
++count;
}
}
if (count != _size) {
throw new IllegalStateException(String.format("Internal error: expected internal size %d vs calculated count %d",
_size, count));
}
}

// For debugging, comment out
/*
@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.fasterxml.jackson.failing;
package com.fasterxml.jackson.core.read;

import com.fasterxml.jackson.core.*;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class TestHashCollisionChars
"@~~", "A_~", "A`_", "Aa@", "Ab!", "B@~", // "BA_", "BB@", "BC!", "C!~"
};
*/

public void testReaderCollisions() throws Exception
{
StringBuilder sb = new StringBuilder();
Expand Down Expand Up @@ -113,7 +113,7 @@ static List<String> collisions() {
* Helper class to use for generating substrings to use for substring
* substitution collisions.
*/
public static class CollisionGenerator
static class CollisionGenerator
{
/* JDK uses 31, but Jackson `CharsToNameCanonicalizer.HASH_MULT`,
* which for 2.3 is 33.
Expand Down
15 changes: 12 additions & 3 deletions src/test/java/com/fasterxml/jackson/core/sym/TestSymbolTables.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public void testSyntheticWithChars()
for (int i = 0; i < COUNT; ++i) {
String id = fieldNameFor(i);
char[] ch = id.toCharArray();

symbols.findSymbol(ch, 0, ch.length, symbols.calcHash(id));
}

Expand All @@ -36,6 +37,9 @@ public void testSyntheticWithChars()
assertEquals(3431, symbols.collisionCount());

assertEquals(6, symbols.maxCollisionLength());

// and final validation
symbols.verifyInternalConsistency();
}

public void testSyntheticWithBytesNew() throws IOException
Expand Down Expand Up @@ -83,13 +87,18 @@ public void testThousandsOfSymbolsWithChars() throws IOException
symbolsC.calcHash(name));
assertNotNull(str);
}
// validate further, just to make sure
symbolsC.verifyInternalConsistency();

symbolsC.release();
exp += 250;
if (exp > CharsToNameCanonicalizer.MAX_ENTRIES_FOR_REUSE) {
exp = 0;
}
assertEquals(exp, symbolsCRoot.size());
}

// Note: can not validate root instance, is not set up same way
}

// Since 2.6
Expand Down Expand Up @@ -122,9 +131,9 @@ public void testThousandsOfSymbolsWithNew() throws IOException
}
assertEquals(exp, symbolsBRoot.size());
}
/* 05-Feb-2015, tatu: Fragile, but it is important to ensure that collision
* rates are not accidentally increased...
*/

// 05-Feb-2015, tatu: Fragile, but it is important to ensure that collision
// rates are not accidentally increased...
assertEquals(6250, symbolsB.size());
assertEquals(4761, symbolsB.primaryCount()); // 80% primary hit rate
assertEquals(1190, symbolsB.secondaryCount()); // 13% secondary
Expand Down

0 comments on commit c0ffdc2

Please sign in to comment.