Skip to content

Commit

Permalink
Refactor construction and use of CharsToNameCanonicalizer (#1088)
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder authored Aug 24, 2023
1 parent 2a87b6f commit 48c4783
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 49 deletions.
14 changes: 11 additions & 3 deletions src/main/java/com/fasterxml/jackson/core/JsonFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public static int collectDefaults() {
* It should not be linked back to the original blueprint, to
* avoid contents from leaking between factories.
*/
protected final transient CharsToNameCanonicalizer _rootCharSymbols = CharsToNameCanonicalizer.createRoot();
protected final transient CharsToNameCanonicalizer _rootCharSymbols;

/**
* Alternative to the basic symbol table, some stream-based
Expand Down Expand Up @@ -377,6 +377,8 @@ public JsonFactory(ObjectCodec oc) {
_streamWriteConstraints = StreamWriteConstraints.defaults();
_errorReportConfiguration = ErrorReportConfiguration.defaults();
_generatorDecorators = null;

_rootCharSymbols = CharsToNameCanonicalizer.createRoot(this);
}

/**
Expand Down Expand Up @@ -408,6 +410,8 @@ protected JsonFactory(JsonFactory src, ObjectCodec codec)
_rootValueSeparator = src._rootValueSeparator;
_maximumNonEscapedChar = src._maximumNonEscapedChar;
_quoteChar = src._quoteChar;

_rootCharSymbols = CharsToNameCanonicalizer.createRoot(this);
}

/**
Expand Down Expand Up @@ -437,6 +441,8 @@ public JsonFactory(JsonFactoryBuilder b) {
_rootValueSeparator = b._rootValueSeparator;
_maximumNonEscapedChar = b._maximumNonEscapedChar;
_quoteChar = b._quoteChar;

_rootCharSymbols = CharsToNameCanonicalizer.createRoot(this);
}

/**
Expand Down Expand Up @@ -466,6 +472,8 @@ protected JsonFactory(TSFBuilder<?,?> b, boolean bogus) {
_rootValueSeparator = null;
_maximumNonEscapedChar = 0;
_quoteChar = DEFAULT_QUOTE_CHAR;

_rootCharSymbols = CharsToNameCanonicalizer.createRoot(this);
}

/**
Expand Down Expand Up @@ -1890,7 +1898,7 @@ protected JsonParser _createParser(InputStream in, IOContext ctxt) throws IOExce
*/
protected JsonParser _createParser(Reader r, IOContext ctxt) throws IOException {
return new ReaderBasedJsonParser(ctxt, _parserFeatures, r, _objectCodec,
_rootCharSymbols.makeChild(_factoryFeatures));
_rootCharSymbols.makeChild());
}

/**
Expand All @@ -1912,7 +1920,7 @@ protected JsonParser _createParser(Reader r, IOContext ctxt) throws IOException
protected JsonParser _createParser(char[] data, int offset, int len, IOContext ctxt,
boolean recyclable) throws IOException {
return new ReaderBasedJsonParser(ctxt, _parserFeatures, null, _objectCodec,
_rootCharSymbols.makeChild(_factoryFeatures),
_rootCharSymbols.makeChild(),
data, offset, offset+len, recyclable);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ public JsonParser constructParser(int parserFeatures, ObjectCodec codec,
}
}
return new ReaderBasedJsonParser(_context, parserFeatures, constructReader(), codec,
rootCharSymbols.makeChild(factoryFeatures));
rootCharSymbols.makeChild());
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import java.util.concurrent.atomic.AtomicReference;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.core.TokenStreamFactory;
import com.fasterxml.jackson.core.exc.StreamConstraintsException;
import com.fasterxml.jackson.core.util.InternCache;

Expand Down Expand Up @@ -114,6 +116,14 @@ public final class CharsToNameCanonicalizer
*/
protected final AtomicReference<TableInfo> _tableInfo;

/**
* Constraints used by {@link TokenStreamFactory} that uses
* this canonicalizer.
*
* @since 2.16
*/
protected final StreamReadConstraints _streamReadConstraints;

/**
* Seed value we use as the base to make hash codes non-static between
* different runs, but still stable for lifetime of a single symbol table
Expand All @@ -125,7 +135,11 @@ public final class CharsToNameCanonicalizer
*/
protected final int _seed;

protected final int _flags;
/**
* Feature flags of {@link TokenStreamFactory} that uses
* this canonicalizer.
*/
protected final int _factoryFeatures;

/**
* Whether any canonicalization should be attempted (whether using
Expand Down Expand Up @@ -229,14 +243,16 @@ public final class CharsToNameCanonicalizer
/**
* Main method for constructing a root symbol table instance.
*/
private CharsToNameCanonicalizer(int seed)
private CharsToNameCanonicalizer(StreamReadConstraints src, int factoryFeatures,
int seed)
{
_parent = null;
_seed = seed;
_streamReadConstraints = src;

// these settings don't really matter for the bootstrap instance
_canonicalize = true;
_flags = -1;
_factoryFeatures = factoryFeatures;
// And we'll also set flags so no copying of buckets is needed:
_hashShared = false; // doesn't really matter for root instance
_longestCollisionList = 0;
Expand All @@ -249,14 +265,16 @@ private CharsToNameCanonicalizer(int seed)
/**
* Internal constructor used when creating child instances.
*/
private CharsToNameCanonicalizer(CharsToNameCanonicalizer parent, int flags, int seed,
private CharsToNameCanonicalizer(CharsToNameCanonicalizer parent,
StreamReadConstraints src, int factoryFeatures, int seed,
TableInfo parentState)
{
_parent = parent;
_streamReadConstraints = src;
_seed = seed;
_tableInfo = null; // not used by child tables
_flags = flags;
_canonicalize = JsonFactory.Feature.CANONICALIZE_FIELD_NAMES.enabledIn(flags);
_factoryFeatures = factoryFeatures;
_canonicalize = JsonFactory.Feature.CANONICALIZE_FIELD_NAMES.enabledIn(factoryFeatures);

// Then copy shared state
_symbols = parentState.symbols;
Expand All @@ -282,25 +300,61 @@ private CharsToNameCanonicalizer(CharsToNameCanonicalizer parent, int flags, int
/**********************************************************
*/

/**
* @deprecated Since 2.16 use {@link #createRoot(TokenStreamFactory)} instead
*
* @return Root instance to use for constructing new child instances
*/
@Deprecated
public static CharsToNameCanonicalizer createRoot() {
return createRoot(null);
}

/**
* @deprecated Since 2.16 use {@link #createRoot(TokenStreamFactory)} instead
*
* @return Root instance to use for constructing new child instances
*/
@Deprecated
public static CharsToNameCanonicalizer createRoot(int seed) {
return createRoot(null, seed);
}

/**
* Method called to create root canonicalizer for a {@link com.fasterxml.jackson.core.JsonFactory}
* instance. Root instance is never used directly; its main use is for
* storing and sharing underlying symbol arrays as needed.
*
* @param owner Factory that will use the root instance; used for accessing
* configuration
*
* @return Root instance to use for constructing new child instances
*/
public static CharsToNameCanonicalizer createRoot() {
public static CharsToNameCanonicalizer createRoot(TokenStreamFactory owner) {
return createRoot(owner, 0);
}

public static CharsToNameCanonicalizer createRoot(TokenStreamFactory owner, int seed) {
// Need to use a variable seed, to thwart hash-collision based attacks.
// 14-Feb-2017, tatu: not sure it actually helps, at all, since it won't
// change mixing or any of the steps. Should likely just remove in future.
long now = System.currentTimeMillis();
// ensure it's not 0; and might as well require to be odd so:
int seed = (((int) now) + ((int) (now >>> 32))) | 1;
return createRoot(seed);
}
if (seed == 0) {
long now = System.currentTimeMillis();
// ensure it's not 0; and might as well require to be odd so:
seed = (((int) now) + ((int) (now >>> 32))) | 1;
}

protected static CharsToNameCanonicalizer createRoot(int seed) {
return new CharsToNameCanonicalizer(seed);
StreamReadConstraints src;
int factoryFeatures;

if (owner == null) {
src = StreamReadConstraints.defaults();
factoryFeatures = 0;
} else {
src = owner.streamReadConstraints();
factoryFeatures = owner.getFactoryFeatures();
}
return new CharsToNameCanonicalizer(src, factoryFeatures, seed);
}

/**
Expand All @@ -315,14 +369,21 @@ protected static CharsToNameCanonicalizer createRoot(int seed) {
* on which only makeChild/mergeChild are called, but instance itself
* is not used as a symbol table.
*
* @param flags Bit flags of active {@link com.fasterxml.jackson.core.JsonFactory.Feature}s enabled.
*
* @return Actual canonicalizer instance that can be used by a parser
*/
public CharsToNameCanonicalizer makeChild(int flags) {
return new CharsToNameCanonicalizer(this, flags, _seed, _tableInfo.get());
public CharsToNameCanonicalizer makeChild() {
return new CharsToNameCanonicalizer(this, _streamReadConstraints,
_factoryFeatures, _seed, _tableInfo.get());
}

/**
* @deprecated Since 2.16 use {@link #makeChild()} instead.
*/
@Deprecated
public CharsToNameCanonicalizer makeChild(int flags) {
return makeChild();
}

/**
* Method called by the using code to indicate it is done with this instance.
* This lets instance merge accumulated changes into parent (if need be),
Expand Down Expand Up @@ -502,7 +563,7 @@ private String _addSymbol(char[] buffer, int start, int len, int h, int index) t
}

String newSymbol = new String(buffer, start, len);
if (JsonFactory.Feature.INTERN_FIELD_NAMES.enabledIn(_flags)) {
if (JsonFactory.Feature.INTERN_FIELD_NAMES.enabledIn(_factoryFeatures)) {
newSymbol = InternCache.instance.intern(newSymbol);
}
++_size;
Expand Down Expand Up @@ -540,7 +601,7 @@ private void _handleSpillOverflow(int bucketIndex, Bucket newBucket, int mainInd
} else {
if (_overflows.get(bucketIndex)) {
// Has happened once already for this bucket index, so probably not coincidental...
if (JsonFactory.Feature.FAIL_ON_SYMBOL_HASH_OVERFLOW.enabledIn(_flags)) {
if (JsonFactory.Feature.FAIL_ON_SYMBOL_HASH_OVERFLOW.enabledIn(_factoryFeatures)) {
_reportTooManyCollisions(MAX_COLL_CHAIN_LENGTH);
}
// but even if we don't fail, we will stop canonicalizing as safety measure
Expand Down
17 changes: 9 additions & 8 deletions src/test/java/com/fasterxml/jackson/core/TestVersions.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ public class TestVersions extends com.fasterxml.jackson.core.BaseTest
{
public void testCoreVersions() throws Exception
{
assertVersion(new JsonFactory().version());
ReaderBasedJsonParser jp = new ReaderBasedJsonParser(getIOContext(), 0, null, null,
CharsToNameCanonicalizer.createRoot());
assertVersion(jp.version());
jp.close();
JsonGenerator jgen = new WriterBasedJsonGenerator(getIOContext(), 0, null, null, '"');
assertVersion(jgen.version());
jgen.close();
final JsonFactory f = new JsonFactory();
assertVersion(f.version());
try (ReaderBasedJsonParser p = new ReaderBasedJsonParser(getIOContext(), 0, null, null,
CharsToNameCanonicalizer.createRoot(f))) {
assertVersion(p.version());
}
try (JsonGenerator g = new WriterBasedJsonGenerator(getIOContext(), 0, null, null, '"')) {
assertVersion(g.version());
}
}

public void testMisc() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void testByteSymbolsWithEOF() throws Exception

public void testHashCalc() throws Exception
{
CharsToNameCanonicalizer sym = CharsToNameCanonicalizer.createRoot(123);
CharsToNameCanonicalizer sym = CharsToNameCanonicalizer.createRoot(new JsonFactory());
char[] str1 = "foo".toCharArray();
char[] str2 = " foo ".toCharArray();

Expand Down
Loading

0 comments on commit 48c4783

Please sign in to comment.