Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor construction and use of CharsToNameCanonicalizer #1088

Merged
merged 4 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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