diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yml index 4e4b196838618..ce8c03afec607 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yml @@ -83,6 +83,9 @@ setup: --- "date": + - skip: + features: "warnings" + - do: search: body: @@ -101,6 +104,28 @@ setup: source: "doc.date.value" - match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12.000Z' } + - do: + warnings: + - getDate is no longer necessary on date fields as the value is now a date. + search: + body: + script_fields: + field: + script: + source: "doc['date'].date" + - match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12.000Z' } + + - do: + warnings: + - getDates is no longer necessary on date fields as the values are now dates. + search: + body: + script_fields: + field: + script: + source: "doc['date'].dates.get(0)" + - match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12.000Z' } + --- "geo_point": - do: @@ -165,6 +190,9 @@ setup: --- "long": + - skip: + features: "warnings" + - do: search: body: @@ -183,6 +211,28 @@ setup: source: "doc['long'].value" - match: { hits.hits.0.fields.field.0: 12348732141234 } + - do: + warnings: + - getDate on numeric fields is deprecated. Use a date field to get dates. + search: + body: + script_fields: + field: + script: + source: "doc['long'].date" + - match: { hits.hits.0.fields.field.0: '2361-04-26T03:22:21.234Z' } + + - do: + warnings: + - getDates on numeric fields is deprecated. Use a date field to get dates. + search: + body: + script_fields: + field: + script: + source: "doc['long'].dates.get(0)" + - match: { hits.hits.0.fields.field.0: '2361-04-26T03:22:21.234Z' } + --- "integer": - do: diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java index 21894c3680965..552ddbf9d616d 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -35,10 +35,13 @@ import org.joda.time.ReadableDateTime; import java.io.IOException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.AbstractList; import java.util.Arrays; import java.util.Comparator; import java.util.List; +import java.util.function.Consumer; import java.util.function.UnaryOperator; @@ -46,7 +49,7 @@ * Script level doc values, the assumption is that any implementation will * implement a getValue and a getValues that return * the relevant type that then can be used in scripts. - * + * * Implementations should not internally re-use objects for the values that they * return as a single {@link ScriptDocValues} instance can be reused to return * values form multiple documents. @@ -94,14 +97,30 @@ public static final class Longs extends ScriptDocValues { protected static final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger(Longs.class)); private final SortedNumericDocValues in; + /** + * Callback for deprecated fields. In production this should always point to + * {@link #deprecationLogger} but tests will override it so they can test that + * we use the required permissions when calling it. + */ + private final Consumer deprecationCallback; private long[] values = new long[0]; private int count; private Dates dates; private int docId = -1; + /** + * Standard constructor. + */ public Longs(SortedNumericDocValues in) { - this.in = in; + this(in, deprecationLogger::deprecated); + } + /** + * Constructor for testing the deprecation callback. + */ + Longs(SortedNumericDocValues in, Consumer deprecationCallback) { + this.in = in; + this.deprecationCallback = deprecationCallback; } @Override @@ -142,7 +161,7 @@ public long getValue() { @Deprecated public ReadableDateTime getDate() throws IOException { - deprecationLogger.deprecated("getDate on numeric fields is deprecated. Use a date field to get dates."); + deprecated("getDate on numeric fields is deprecated. Use a date field to get dates."); if (dates == null) { dates = new Dates(in); dates.setNextDocId(docId); @@ -152,7 +171,7 @@ public ReadableDateTime getDate() throws IOException { @Deprecated public List getDates() throws IOException { - deprecationLogger.deprecated("getDates on numeric fields is deprecated. Use a date field to get dates."); + deprecated("getDates on numeric fields is deprecated. Use a date field to get dates."); if (dates == null) { dates = new Dates(in); dates.setNextDocId(docId); @@ -169,6 +188,22 @@ public Long get(int index) { public int size() { return count; } + + /** + * Log a deprecation log, with the server's permissions, not the permissions of the + * script calling this method. We need to do this to prevent errors when rolling + * the log file. + */ + private void deprecated(String message) { + // Intentionally not calling SpecialPermission.check because this is supposed to be called by scripts + AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Void run() { + deprecationCallback.accept(message); + return null; + } + }); + } } public static final class Dates extends ScriptDocValues { @@ -177,6 +212,12 @@ public static final class Dates extends ScriptDocValues { private static final ReadableDateTime EPOCH = new DateTime(0, DateTimeZone.UTC); private final SortedNumericDocValues in; + /** + * Callback for deprecated fields. In production this should always point to + * {@link #deprecationLogger} but tests will override it so they can test that + * we use the required permissions when calling it. + */ + private final Consumer deprecationCallback; /** * Values wrapped in {@link MutableDateTime}. Null by default an allocated on first usage so we allocate a reasonably size. We keep * this array so we don't have allocate new {@link MutableDateTime}s on every usage. Instead we reuse them for every document. @@ -184,8 +225,19 @@ public static final class Dates extends ScriptDocValues { private MutableDateTime[] dates; private int count; + /** + * Standard constructor. + */ public Dates(SortedNumericDocValues in) { + this(in, deprecationLogger::deprecated); + } + + /** + * Constructor for testing deprecation logging. + */ + Dates(SortedNumericDocValues in, Consumer deprecationCallback) { this.in = in; + this.deprecationCallback = deprecationCallback; } /** @@ -204,7 +256,7 @@ public ReadableDateTime getValue() { */ @Deprecated public ReadableDateTime getDate() { - deprecationLogger.deprecated("getDate is no longer necessary on date fields as the value is now a date."); + deprecated("getDate is no longer necessary on date fields as the value is now a date."); return getValue(); } @@ -213,7 +265,7 @@ public ReadableDateTime getDate() { */ @Deprecated public List getDates() { - deprecationLogger.deprecated("getDates is no longer necessary on date fields as the values are now dates."); + deprecated("getDates is no longer necessary on date fields as the values are now dates."); return this; } @@ -274,6 +326,22 @@ void refreshArray() throws IOException { dates[i] = new MutableDateTime(in.nextValue(), DateTimeZone.UTC); } } + + /** + * Log a deprecation log, with the server's permissions, not the permissions of the + * script calling this method. We need to do this to prevent errors when rolling + * the log file. + */ + private void deprecated(String message) { + // Intentionally not calling SpecialPermission.check because this is supposed to be called by scripts + AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Void run() { + deprecationCallback.accept(message); + return null; + } + }); + } } public static final class Doubles extends ScriptDocValues { diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java index 626327d454910..7a0a6816a66bf 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java @@ -26,6 +26,17 @@ import org.joda.time.ReadableDateTime; import java.io.IOException; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PermissionCollection; +import java.security.Permissions; +import java.security.PrivilegedAction; +import java.security.ProtectionDomain; +import java.util.HashSet; +import java.util.Set; +import java.util.function.Consumer; + +import static org.hamcrest.Matchers.containsInAnyOrder; public class ScriptDocValuesDatesTests extends ESTestCase { public void test() throws IOException { @@ -39,12 +50,19 @@ public void test() throws IOException { values[d][i] = expectedDates[d][i].getMillis(); } } - Dates dates = wrap(values); + Set warnings = new HashSet<>(); + Dates dates = wrap(values, deprecationMessage -> { + warnings.add(deprecationMessage); + /* Create a temporary directory to prove we are running with the + * server's permissions. */ + createTempDir(); + }); for (int round = 0; round < 10; round++) { int d = between(0, values.length - 1); dates.setNextDocId(d); assertEquals(expectedDates[d].length > 0 ? expectedDates[d][0] : new DateTime(0, DateTimeZone.UTC), dates.getValue()); + assertEquals(expectedDates[d].length > 0 ? expectedDates[d][0] : new DateTime(0, DateTimeZone.UTC), dates.getDate()); assertEquals(values[d].length, dates.size()); for (int i = 0; i < values[d].length; i++) { @@ -54,9 +72,33 @@ public void test() throws IOException { Exception e = expectThrows(UnsupportedOperationException.class, () -> dates.add(new DateTime())); assertEquals("doc values are unmodifiable", e.getMessage()); } + + /* + * Invoke getDates without any privileges to verify that + * it still works without any. In particularly, this + * verifies that the callback that we've configured + * above works. That callback creates a temporary + * directory which is not possible with "noPermissions". + */ + PermissionCollection noPermissions = new Permissions(); + AccessControlContext noPermissionsAcc = new AccessControlContext( + new ProtectionDomain[] { + new ProtectionDomain(null, noPermissions) + } + ); + AccessController.doPrivileged(new PrivilegedAction() { + public Void run() { + dates.getDates(); + return null; + } + }, noPermissionsAcc); + + assertThat(warnings, containsInAnyOrder( + "getDate is no longer necessary on date fields as the value is now a date.", + "getDates is no longer necessary on date fields as the values are now dates.")); } - private Dates wrap(long[][] values) { + private Dates wrap(long[][] values, Consumer deprecationHandler) { return new Dates(new AbstractSortedNumericDocValues() { long[] current; int i; @@ -75,6 +117,6 @@ public int docValueCount() { public long nextValue() { return current[i++]; } - }); + }, deprecationHandler); } } diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java index 1b3e8fa227462..8b20e9a9f3a71 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java @@ -26,6 +26,17 @@ import org.joda.time.ReadableDateTime; import java.io.IOException; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PermissionCollection; +import java.security.Permissions; +import java.security.PrivilegedAction; +import java.security.ProtectionDomain; +import java.util.HashSet; +import java.util.Set; +import java.util.function.Consumer; + +import static org.hamcrest.Matchers.containsInAnyOrder; public class ScriptDocValuesLongsTests extends ESTestCase { public void testLongs() throws IOException { @@ -36,7 +47,7 @@ public void testLongs() throws IOException { values[d][i] = randomLong(); } } - Longs longs = wrap(values); + Longs longs = wrap(values, deprecationMessage -> {fail("unexpected deprecation: " + deprecationMessage);}); for (int round = 0; round < 10; round++) { int d = between(0, values.length - 1); @@ -66,7 +77,13 @@ public void testDates() throws IOException { values[d][i] = dates[d][i].getMillis(); } } - Longs longs = wrap(values); + Set warnings = new HashSet<>(); + Longs longs = wrap(values, deprecationMessage -> { + warnings.add(deprecationMessage); + /* Create a temporary directory to prove we are running with the + * server's permissions. */ + createTempDir(); + }); for (int round = 0; round < 10; round++) { int d = between(0, values.length - 1); @@ -82,12 +99,36 @@ public void testDates() throws IOException { assertEquals("doc values are unmodifiable", e.getMessage()); } - assertWarnings( + /* + * Invoke getDates without any privileges to verify that + * it still works without any. In particularly, this + * verifies that the callback that we've configured + * above works. That callback creates a temporary + * directory which is not possible with "noPermissions". + */ + PermissionCollection noPermissions = new Permissions(); + AccessControlContext noPermissionsAcc = new AccessControlContext( + new ProtectionDomain[] { + new ProtectionDomain(null, noPermissions) + } + ); + AccessController.doPrivileged(new PrivilegedAction() { + public Void run() { + try { + longs.getDates(); + } catch (IOException e) { + throw new RuntimeException("unexpected", e); + } + return null; + } + }, noPermissionsAcc); + + assertThat(warnings, containsInAnyOrder( "getDate on numeric fields is deprecated. Use a date field to get dates.", - "getDates on numeric fields is deprecated. Use a date field to get dates."); + "getDates on numeric fields is deprecated. Use a date field to get dates.")); } - private Longs wrap(long[][] values) { + private Longs wrap(long[][] values, Consumer deprecationCallback) { return new Longs(new AbstractSortedNumericDocValues() { long[] current; int i; @@ -106,6 +147,6 @@ public int docValueCount() { public long nextValue() { return current[i++]; } - }); + }, deprecationCallback); } }