Skip to content

Commit

Permalink
Decouple more classes from XContentBuilder and make builder strict (#…
Browse files Browse the repository at this point in the history
…29197)

This commit decouples `BytesRef`, `Releaseable`, and `TimeValue` from
XContentBuilder, and paves the way for doupling `ByteSizeValue` as well. It
moves much of the Lucene and Joda encoding into a new SPI extension that is
loaded by XContentBuilder to know how to encode these values.

Part of doing this also allows us to make JSON encoding strict, as we no longer
allow just any old object to be passed (in the past it was possible to get json
that was `"field": "java.lang.Object@d8355a8"` if no one was careful about what
was passed in).

Relates to #28504
  • Loading branch information
dakrone committed Mar 22, 2018
1 parent 683bb1c commit 4f84c02
Show file tree
Hide file tree
Showing 18 changed files with 171 additions and 65 deletions.
9 changes: 8 additions & 1 deletion server/src/main/java/org/elasticsearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.monitor.jvm.JvmInfo;

import java.io.IOException;
Expand All @@ -34,7 +36,7 @@
import java.util.Collections;
import java.util.List;

public class Version implements Comparable<Version> {
public class Version implements Comparable<Version>, ToXContentFragment {
/*
* The logic for ID is: XXYYZZAA, where XX is major version, YY is minor version, ZZ is revision, and AA is alpha/beta/rc indicator AA
* values below 25 are for alpha builder (since 5.0), and above 25 and below 50 are beta builds, and below 99 are RC builds, with 99
Expand Down Expand Up @@ -409,6 +411,11 @@ public int compareTo(Version other) {
return Integer.compare(this.id, other.id);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.value(toString());
}

/*
* We need the declared versions when computing the minimum compatibility version. As computing the declared versions uses reflection it
* is not cheap. Since computing the minimum compatibility version can occur often, we use this holder to compute the declared versions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.lucene.util.BytesRefIterator;
import org.elasticsearch.common.io.stream.BytesStream;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.ByteArrayOutputStream;
Expand All @@ -35,7 +36,7 @@
/**
* A reference to bytes.
*/
public abstract class BytesReference implements Accountable, Comparable<BytesReference> {
public abstract class BytesReference implements Accountable, Comparable<BytesReference>, ToXContentFragment {

private Integer hash = null; // we cache the hash of this reference since it can be quite costly to re-calculated it

Expand Down Expand Up @@ -301,4 +302,10 @@ public long skip(long n) throws IOException {
return input.skip(n);
}
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
BytesRef bytes = toBytesRef();
return builder.value(bytes.bytes, bytes.offset, bytes.length);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.common.document;

import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -127,11 +128,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
// Stored fields values are converted using MappedFieldType#valueForDisplay.
// As a result they can either be Strings, Numbers, or Booleans, that's
// all.
if (value instanceof BytesReference) {
builder.binaryValue(((BytesReference) value).toBytesRef());
} else {
builder.value(value);
}
builder.value(value);
}
builder.endArray();
return builder;
Expand Down
4 changes: 3 additions & 1 deletion server/src/main/java/org/elasticsearch/common/text/Text.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.common.text;

import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.xcontent.ToXContent;
Expand Down Expand Up @@ -125,7 +126,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
} else {
// TODO: TextBytesOptimization we can use a buffer here to convert it? maybe add a
// request to jackson to support InputStream as well?
return builder.utf8Value(this.bytes().toBytesRef());
BytesRef br = this.bytes().toBytesRef();
return builder.utf8Value(br.bytes, br.offset, br.length);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.network.NetworkAddress;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;
import java.net.InetAddress;
Expand All @@ -34,7 +37,7 @@
/**
* A transport address used for IP socket address (wraps {@link java.net.InetSocketAddress}).
*/
public final class TransportAddress implements Writeable {
public final class TransportAddress implements Writeable, ToXContentFragment {

/**
* A <a href="https://en.wikipedia.org/wiki/0.0.0.0">non-routeable v4 meta transport address</a> that can be used for
Expand Down Expand Up @@ -155,4 +158,9 @@ public int hashCode() {
public String toString() {
return NetworkAddress.format(address);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.value(toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;
import java.util.Locale;
import java.util.Objects;

public class ByteSizeValue implements Writeable, Comparable<ByteSizeValue> {
public class ByteSizeValue implements Writeable, Comparable<ByteSizeValue>, ToXContentFragment {
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ByteSizeValue.class));

private final long size;
Expand Down Expand Up @@ -271,4 +274,9 @@ public int compareTo(ByteSizeValue other) {
long otherValue = other.size * other.unit.toBytes(1);
return Long.compare(thisValue, otherValue);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.value(toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.joda.time.Period;
import org.joda.time.PeriodType;
import org.joda.time.format.PeriodFormat;
Expand All @@ -40,7 +43,7 @@
import java.util.Set;
import java.util.concurrent.TimeUnit;

public class TimeValue implements Writeable, Comparable<TimeValue> {
public class TimeValue implements Writeable, Comparable<TimeValue>, ToXContentFragment {

/** How many nano-seconds in one milli-second */
public static final long NSEC_PER_MSEC = TimeUnit.NANOSECONDS.convert(1, TimeUnit.MILLISECONDS);
Expand Down Expand Up @@ -398,4 +401,9 @@ public int compareTo(TimeValue timeValue) {
double otherValue = ((double) timeValue.duration) * timeValue.timeUnit.toNanos(1);
return Double.compare(thisValue, otherValue);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.value(toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,21 @@

package org.elasticsearch.common.xcontent;

import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.lease.Releasable;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.CollectionUtils;
import org.joda.time.DateTimeZone;
import org.joda.time.ReadableInstant;
import org.joda.time.format.DateTimeFormatter;
import org.joda.time.format.ISODateTimeFormat;

import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.Flushable;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Path;
import java.time.ZonedDateTime;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collections;
Expand All @@ -49,7 +48,7 @@
/**
* A utility to build XContent (ie json).
*/
public final class XContentBuilder implements Releasable, Flushable {
public final class XContentBuilder implements Closeable, Flushable {

/**
* Create a new {@link XContentBuilder} using the given {@link XContent} content.
Expand Down Expand Up @@ -91,7 +90,6 @@ public static XContentBuilder builder(XContent xContent, Set<String> includes, S
writers.put(Boolean.class, (b, v) -> b.value((Boolean) v));
writers.put(Byte.class, (b, v) -> b.value((Byte) v));
writers.put(byte[].class, (b, v) -> b.value((byte[]) v));
writers.put(BytesRef.class, (b, v) -> b.binaryValue((BytesRef) v));
writers.put(Date.class, (b, v) -> b.value((Date) v));
writers.put(Double.class, (b, v) -> b.value((Double) v));
writers.put(double[].class, (b, v) -> b.values((double[]) v));
Expand All @@ -105,12 +103,12 @@ public static XContentBuilder builder(XContent xContent, Set<String> includes, S
writers.put(short[].class, (b, v) -> b.values((short[]) v));
writers.put(String.class, (b, v) -> b.value((String) v));
writers.put(String[].class, (b, v) -> b.values((String[]) v));
writers.put(Locale.class, (b, v) -> b.value(v.toString()));
writers.put(Class.class, (b, v) -> b.value(v.toString()));
writers.put(ZonedDateTime.class, (b, v) -> b.value(v.toString()));


Map<Class<?>, HumanReadableTransformer> humanReadableTransformer = new HashMap<>();
// These will be moved to a different class at a later time to decouple them from XContentBuilder
humanReadableTransformer.put(TimeValue.class, v -> ((TimeValue) v).millis());
humanReadableTransformer.put(ByteSizeValue.class, v -> ((ByteSizeValue) v).getBytes());

// Load pluggable extensions
for (XContentBuilderExtension service : ServiceLoader.load(XContentBuilderExtension.class)) {
Expand Down Expand Up @@ -613,49 +611,25 @@ public XContentBuilder value(byte[] value, int offset, int length) throws IOExce
}

/**
* Writes the binary content of the given {@link BytesRef}.
*
* Use {@link org.elasticsearch.common.xcontent.XContentParser#binaryValue()} to read the value back
*/
public XContentBuilder field(String name, BytesRef value) throws IOException {
return field(name).binaryValue(value);
}

/**
* Writes the binary content of the given {@link BytesRef} as UTF-8 bytes.
* Writes the binary content of the given byte array as UTF-8 bytes.
*
* Use {@link XContentParser#charBuffer()} to read the value back
*/
public XContentBuilder utf8Field(String name, BytesRef value) throws IOException {
return field(name).utf8Value(value);
}

/**
* Writes the binary content of the given {@link BytesRef}.
*
* Use {@link org.elasticsearch.common.xcontent.XContentParser#binaryValue()} to read the value back
*/
public XContentBuilder binaryValue(BytesRef value) throws IOException {
if (value == null) {
return nullValue();
}
value(value.bytes, value.offset, value.length);
return this;
public XContentBuilder utf8Field(String name, byte[] bytes, int offset, int length) throws IOException {
return field(name).utf8Value(bytes, offset, length);
}

/**
* Writes the binary content of the given {@link BytesRef} as UTF-8 bytes.
* Writes the binary content of the given byte array as UTF-8 bytes.
*
* Use {@link XContentParser#charBuffer()} to read the value back
*/
public XContentBuilder utf8Value(BytesRef value) throws IOException {
if (value == null) {
return nullValue();
}
generator.writeUTF8String(value.bytes, value.offset, value.length);
public XContentBuilder utf8Value(byte[] bytes, int offset, int length) throws IOException {
generator.writeUTF8String(bytes, offset, length);
return this;
}


////////////////////////////////////////////////////////////////////////////
// Date
//////////////////////////////////
Expand Down Expand Up @@ -793,10 +767,11 @@ private void unknownValue(Object value, boolean ensureNoSelfReferences) throws I
value((ReadableInstant) value);
} else if (value instanceof ToXContent) {
value((ToXContent) value);
} else {
// This is a "value" object (like enum, DistanceUnit, etc) just toString() it
// (yes, it can be misleading when toString a Java class, but really, jackson should be used in that case)
} else if (value instanceof Enum<?>) {
// Write out the Enum toString
value(Objects.toString(value));
} else {
throw new IllegalArgumentException("cannot write xcontent for unknown value of type " + value.getClass());
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.common.xcontent;

import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.joda.time.DateTimeZone;
import org.joda.time.tz.CachedDateTimeZone;
import org.joda.time.tz.FixedDateTimeZone;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

/**
* SPI extensions for Elasticsearch-specific classes (like the Lucene or Joda
* dependency classes) that need to be encoded by {@link XContentBuilder} in a
* specific way.
*/
public class XContentElasticsearchExtension implements XContentBuilderExtension {

@Override
public Map<Class<?>, XContentBuilder.Writer> getXContentWriters() {
Map<Class<?>, XContentBuilder.Writer> writers = new HashMap<>();

// Fully-qualified here to reduce ambiguity around our (ES') Version class
writers.put(org.apache.lucene.util.Version.class, (b, v) -> b.value(Objects.toString(v)));
writers.put(DateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));
writers.put(CachedDateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));
writers.put(FixedDateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));

writers.put(BytesReference.class, (b, v) -> {
if (v == null) {
b.nullValue();
} else {
BytesRef bytes = ((BytesReference) v).toBytesRef();
b.value(bytes.bytes, bytes.offset, bytes.length);
}
});

writers.put(BytesRef.class, (b, v) -> {
if (v == null) {
b.nullValue();
} else {
BytesRef bytes = (BytesRef) v;
b.value(bytes.bytes, bytes.offset, bytes.length);
}
});
return writers;
}

@Override
public Map<Class<?>, XContentBuilder.HumanReadableTransformer> getXContentHumanReadableTransformers() {
Map<Class<?>, XContentBuilder.HumanReadableTransformer> transformers = new HashMap<>();
transformers.put(TimeValue.class, v -> ((TimeValue) v).millis());
transformers.put(ByteSizeValue.class, v -> ((ByteSizeValue) v).getBytes());
return transformers;
}
}
Loading

0 comments on commit 4f84c02

Please sign in to comment.