Merge branch 'main' into non-issue/add-basic-kmeans

Author: benwtrent

modules/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/TikaImpl.java

@@ -16,33 +16,11 @@
 import org.apache.tika.parser.AutoDetectParser;
 import org.apache.tika.parser.Parser;
 import org.apache.tika.parser.ParserDecorator;
-import org.elasticsearch.SpecialPermission;
-import org.elasticsearch.bootstrap.FilePermissionUtils;
-import org.elasticsearch.core.PathUtils;
-import org.elasticsearch.core.SuppressForbidden;
-import org.elasticsearch.jdk.JarHell;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
-import java.io.UncheckedIOException;
-import java.lang.reflect.ReflectPermission;
-import java.net.URISyntaxException;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.security.AccessControlContext;
-import java.security.AccessController;
-import java.security.PermissionCollection;
-import java.security.Permissions;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
-import java.security.ProtectionDomain;
-import java.security.SecurityPermission;
 import java.util.Arrays;
 import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.PropertyPermission;
 import java.util.Set;
 
 /**
@@ -90,24 +68,8 @@ final class TikaImpl {
      * parses with tika, throwing any exception hit while parsing the document
      */
     static String parse(final byte content[], final Metadata metadata, final int limit) throws TikaException, IOException {
-        // check that its not unprivileged code like a script
-        SpecialPermission.check();
-
         try {
-            return AccessController.doPrivileged(
-                (PrivilegedExceptionAction<String>) () -> TIKA_INSTANCE.parseToString(new ByteArrayInputStream(content), metadata, limit),
-                RESTRICTED_CONTEXT
-            );
-        } catch (PrivilegedActionException e) {
-            // checked exception from tika: unbox it
-            Throwable cause = e.getCause();
-            if (cause instanceof TikaException tikaException) {
-                throw tikaException;
-            } else if (cause instanceof IOException ioException) {
-                throw ioException;
-            } else {
-                throw new AssertionError(cause);
-            }
+            return TIKA_INSTANCE.parseToString(new ByteArrayInputStream(content), metadata, limit);
         } catch (LinkageError e) {
             if (e.getMessage().contains("bouncycastle")) {
                 /*
@@ -119,76 +81,4 @@ static String parse(final byte content[], final Metadata metadata, final int lim
             throw new RuntimeException(e);
         }
     }
-
-    // apply additional containment for parsers, this is intersected with the current permissions
-    // its hairy, but worth it so we don't have some XML flaw reading random crap from the FS
-    private static final AccessControlContext RESTRICTED_CONTEXT = isUsingSecurityManager()
-        ? new AccessControlContext(new ProtectionDomain[] { new ProtectionDomain(null, getRestrictedPermissions()) })
-        : null;
-
-    private static boolean isUsingSecurityManager() {
-        return false;
-    }
-
-    // compute some minimal permissions for parsers. they only get r/w access to the java temp directory,
-    // the ability to load some resources from JARs, and read sysprops
-    @SuppressForbidden(reason = "adds access to tmp directory")
-    static PermissionCollection getRestrictedPermissions() {
-        Permissions perms = new Permissions();
-
-        // property/env access needed for parsing
-        perms.add(new PropertyPermission("*", "read"));
-        perms.add(new RuntimePermission("getenv.TIKA_CONFIG"));
-
-        try {
-            // add permissions for resource access:
-            // classpath
-            addReadPermissions(perms, JarHell.parseClassPath());
-            // plugin jars
-            if (TikaImpl.class.getClassLoader() instanceof URLClassLoader urlClassLoader) {
-                URL[] urls = urlClassLoader.getURLs();
-                Set<URL> set = new LinkedHashSet<>(Arrays.asList(urls));
-                if (set.size() != urls.length) {
-                    throw new AssertionError("duplicate jars: " + Arrays.toString(urls));
-                }
-                addReadPermissions(perms, set);
-            }
-            // jvm's java.io.tmpdir (needs read/write)
-            FilePermissionUtils.addDirectoryPath(
-                perms,
-                "java.io.tmpdir",
-                PathUtils.get(System.getProperty("java.io.tmpdir")),
-                "read,readlink,write,delete",
-                false
-            );
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
-        // current hacks needed for POI/PDFbox issues:
-        perms.add(new SecurityPermission("putProviderProperty.BC"));
-        perms.add(new SecurityPermission("insertProvider"));
-        perms.add(new ReflectPermission("suppressAccessChecks"));
-        perms.add(new RuntimePermission("accessClassInPackage.sun.java2d.cmm.kcms"));
-        // xmlbeans, use by POI, needs to get the context classloader
-        perms.add(new RuntimePermission("getClassLoader"));
-        perms.setReadOnly();
-        return perms;
-    }
-
-    // add resources to (what is typically) a jar, but might not be (e.g. in tests/IDE)
-    @SuppressForbidden(reason = "adds access to jar resources")
-    static void addReadPermissions(Permissions perms, Set<URL> resources) throws IOException {
-        try {
-            for (URL url : resources) {
-                Path path = PathUtils.get(url.toURI());
-                if (Files.isDirectory(path)) {
-                    FilePermissionUtils.addDirectoryPath(perms, "class.path", path, "read,readlink", false);
-                } else {
-                    FilePermissionUtils.addSingleFilePath(perms, path, "read,readlink");
-                }
-            }
-        } catch (URISyntaxException bogus) {
-            throw new RuntimeException(bogus);
-        }
-    }
 }

modules/ingest-attachment/src/main/plugin-metadata/plugin-security.policy

@@ -432,6 +432,15 @@ tests:
 - class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeIT
   method: test
   issue: https://github.com/elastic/elasticsearch/issues/127157
+- class: org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT
+  method: test {fork.ForkWithWhereSortDescAndLimit SYNC}
+  issue: https://github.com/elastic/elasticsearch/issues/127326
+- class: org.elasticsearch.geometry.utils.SpatialEnvelopeVisitorTests
+  method: testVisitGeoPointsWrapping
+  issue: https://github.com/elastic/elasticsearch/issues/123425
+- class: org.elasticsearch.xpack.esql.action.CrossClusterQueryWithFiltersIT
+  method: testTimestampFilterFromQuery
+  issue: https://github.com/elastic/elasticsearch/issues/127332
 
 # Examples:
 #

muted-tests.yml

@@ -227,7 +227,7 @@ static TransportVersion def(int id) {
     public static final TransportVersion DOC_FIELDS_AS_LIST = def(9_061_0_00);
     public static final TransportVersion DENSE_VECTOR_OFF_HEAP_STATS = def(9_062_00_0);
     public static final TransportVersion RANDOM_SAMPLER_QUERY_BUILDER = def(9_063_0_00);
-    public static final TransportVersion SETTINGS_IN_DATA_STREAMS = def(9_064_00_0);
+    public static final TransportVersion SETTINGS_IN_DATA_STREAMS = def(9_064_0_00);
 
     /*
      * STOP! READ THIS FIRST! No, really,

server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -23,7 +23,9 @@
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.common.bytes.ReleasableBytesReference;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.RecyclerBytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -50,6 +52,7 @@
 import org.elasticsearch.tasks.TaskId;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.AbstractTransportRequest;
+import org.elasticsearch.transport.BytesTransportResponse;
 import org.elasticsearch.transport.LeakTracker;
 import org.elasticsearch.transport.SendRequestTransportException;
 import org.elasticsearch.transport.Transport;
@@ -58,6 +61,7 @@
 import org.elasticsearch.transport.TransportException;
 import org.elasticsearch.transport.TransportResponse;
 import org.elasticsearch.transport.TransportResponseHandler;
+import org.elasticsearch.transport.TransportService;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -215,41 +219,22 @@ public static final class NodeQueryResponse extends TransportResponse {
             this.topDocsStats = SearchPhaseController.TopDocsStats.readFrom(in);
         }
 
-        NodeQueryResponse(
-            QueryPhaseResultConsumer.MergeResult mergeResult,
-            Object[] results,
-            SearchPhaseController.TopDocsStats topDocsStats
-        ) {
-            this.results = results;
-            for (Object result : results) {
-                if (result instanceof QuerySearchResult r) {
-                    r.incRef();
-                }
-            }
-            this.mergeResult = mergeResult;
-            this.topDocsStats = topDocsStats;
-            assert Arrays.stream(results).noneMatch(Objects::isNull) : Arrays.toString(results);
-        }
-
         // public for tests
         public Object[] getResults() {
             return results;
         }
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
-            out.writeArray((o, v) -> {
-                if (v instanceof Exception e) {
-                    o.writeBoolean(false);
-                    o.writeException(e);
+            out.writeVInt(results.length);
+            for (Object result : results) {
+                if (result instanceof Exception e) {
+                    writePerShardException(out, e);
                 } else {
-                    o.writeBoolean(true);
-                    assert v instanceof QuerySearchResult : v;
-                    ((QuerySearchResult) v).writeTo(o);
+                    writePerShardResult(out, (QuerySearchResult) result);
                 }
-            }, results);
-            mergeResult.writeTo(out);
-            topDocsStats.writeTo(out);
+            }
+            writeMergeResult(out, mergeResult, topDocsStats);
         }
 
         @Override
@@ -280,6 +265,25 @@ public boolean decRef() {
             }
             return false;
         }
+
+        private static void writeMergeResult(
+            StreamOutput out,
+            QueryPhaseResultConsumer.MergeResult mergeResult,
+            SearchPhaseController.TopDocsStats topDocsStats
+        ) throws IOException {
+            mergeResult.writeTo(out);
+            topDocsStats.writeTo(out);
+        }
+
+        private static void writePerShardException(StreamOutput o, Exception e) throws IOException {
+            o.writeBoolean(false);
+            o.writeException(e);
+        }
+
+        private static void writePerShardResult(StreamOutput out, SearchPhaseResult result) throws IOException {
+            out.writeBoolean(true);
+            result.writeTo(out);
+        }
     }
 
     /**
@@ -552,7 +556,7 @@ static void registerNodeSearchAction(
     ) {
         var transportService = searchTransportService.transportService();
         var threadPool = transportService.getThreadPool();
-        final Dependencies dependencies = new Dependencies(searchService, threadPool.executor(ThreadPool.Names.SEARCH));
+        final Dependencies dependencies = new Dependencies(searchService, transportService, threadPool.executor(ThreadPool.Names.SEARCH));
         // Even though not all searches run on the search pool, we use the search pool size as the upper limit of shards to execute in
         // parallel to keep the implementation simple instead of working out the exact pool(s) a query will use up-front.
         final int searchPoolMax = threadPool.info(ThreadPool.Names.SEARCH).getMax();
@@ -715,7 +719,7 @@ public void onFailure(Exception e) {
         }
     }
 
-    private record Dependencies(SearchService searchService, Executor executor) {}
+    private record Dependencies(SearchService searchService, TransportService transportService, Executor executor) {}
 
     private static final class QueryPerNodeState {
 
@@ -760,6 +764,8 @@ void onShardDone() {
             if (countDown.countDown() == false) {
                 return;
             }
+            RecyclerBytesStreamOutput out = null;
+            boolean success = false;
             var channelListener = new ChannelActionListener<>(channel);
             try (queryPhaseResultConsumer) {
                 var failure = queryPhaseResultConsumer.failure.get();
@@ -788,33 +794,46 @@ void onShardDone() {
                         relevantShardIndices.set(localIndex);
                     }
                 }
-                final Object[] results = new Object[queryPhaseResultConsumer.getNumShards()];
-                for (int i = 0; i < results.length; i++) {
-                    var result = queryPhaseResultConsumer.results.get(i);
-                    if (result == null) {
-                        results[i] = failures.get(i);
-                    } else {
-                        // free context id and remove it from the result right away in case we don't need it anymore
-                        if (result instanceof QuerySearchResult q
-                            && q.getContextId() != null
-                            && relevantShardIndices.get(q.getShardIndex()) == false
-                            && q.hasSuggestHits() == false
-                            && q.getRankShardResult() == null
-                            && searchRequest.searchRequest.scroll() == null
-                            && isPartOfPIT(searchRequest.searchRequest, q.getContextId()) == false) {
-                            if (dependencies.searchService.freeReaderContext(q.getContextId())) {
-                                q.clearContextId();
-                            }
+                final int resultCount = queryPhaseResultConsumer.getNumShards();
+                out = dependencies.transportService.newNetworkBytesStream();
+                out.setTransportVersion(channel.getVersion());
+                try {
+                    out.writeVInt(resultCount);
+                    for (int i = 0; i < resultCount; i++) {
+                        var result = queryPhaseResultConsumer.results.get(i);
+                        if (result == null) {
+                            NodeQueryResponse.writePerShardException(out, failures.remove(i));
+                        } else {
+                            // free context id and remove it from the result right away in case we don't need it anymore
+                            maybeFreeContext(result, relevantShardIndices);
+                            NodeQueryResponse.writePerShardResult(out, result);
                         }
-                        results[i] = result;
                     }
-                    assert results[i] != null;
+                    NodeQueryResponse.writeMergeResult(out, mergeResult, queryPhaseResultConsumer.topDocsStats);
+                    success = true;
+                } catch (IOException e) {
+                    handleMergeFailure(e, channelListener);
+                    return;
                 }
+            } finally {
+                if (success == false && out != null) {
+                    out.close();
+                }
+            }
+            ActionListener.respondAndRelease(channelListener, new BytesTransportResponse(new ReleasableBytesReference(out.bytes(), out)));
+        }
 
-                ActionListener.respondAndRelease(
-                    channelListener,
-                    new NodeQueryResponse(mergeResult, results, queryPhaseResultConsumer.topDocsStats)
-                );
+        private void maybeFreeContext(SearchPhaseResult result, BitSet relevantShardIndices) {
+            if (result instanceof QuerySearchResult q
+                && q.getContextId() != null
+                && relevantShardIndices.get(q.getShardIndex()) == false
+                && q.hasSuggestHits() == false
+                && q.getRankShardResult() == null
+                && searchRequest.searchRequest.scroll() == null
+                && isPartOfPIT(searchRequest.searchRequest, q.getContextId()) == false) {
+                if (dependencies.searchService.freeReaderContext(q.getContextId())) {
+                    q.clearContextId();
+                }
             }
         }