This is an automated email from the ASF dual-hosted git repository.
cgivre pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push:
new fddd8be2c6 DRILL-8554: Update Jackcess and Fix Linked Table Issue
(#3080)
fddd8be2c6 is described below
commit fddd8be2c6efd87b47a188b1dcf182854ce7bef3
Author: Charles S. Givre <[email protected]>
AuthorDate: Thu Oct 8 17:22:51 2026 -0400
DRILL-8554: Update Jackcess and Fix Linked Table Issue (#3080)
---
contrib/format-access/README.md | 18 +++++
contrib/format-access/pom.xml | 2 +-
.../exec/store/msaccess/MSAccessBatchReader.java | 66 ++++++++++++++--
.../exec/store/msaccess/MSAccessFormatPlugin.java | 21 +++++-
.../src/main/resources/drill-module.conf | 8 ++
.../store/msaccess/TestMSAccessLinkedTables.java | 83 +++++++++++++++++++++
.../exec/store/msaccess/TestMSAccessReader.java | 87 ++++++++++++++++++++--
7 files changed, 268 insertions(+), 17 deletions(-)
diff --git a/contrib/format-access/README.md b/contrib/format-access/README.md
index 0c3035117a..230eda91df 100644
--- a/contrib/format-access/README.md
+++ b/contrib/format-access/README.md
@@ -11,6 +11,24 @@ Simply add the following to any Drill file system
configuration. Typically, MS
}
```
+### Linked Tables
+An Access file can define tables that live in another database file,
referenced by an absolute local path or a UNC
+share embedded in the file itself. Drill refuses to follow those references
by default: reading an untrusted file
+would otherwise make the Drillbit open a path of the file author's choosing,
as the Drillbit service account.
+
+**Note:** Before Drill 1.23, linked tables were followed automatically.
Queries against a linked table now fail with a
+permission error unless an administrator enables them. Listing the tables in
a file (a query without `tableName`)
+still works: linked tables are shown with null dates and counts.
+
+If you trust every file that can be queried and need linked tables to work, an
administrator can enable them by
+setting the following boot option in `drill-override.conf` on every Drillbit:
+
+```
+drill.exec.storage.msaccess.allow_linked_databases: true
+```
+
+This is deliberately not a format config or table-function option, so that a
query user cannot turn it on.
+
## Schemas
Drill will discover the schema automatically from the Access file. The plugin
does support schema provisioning for consistency, but is not recommended.
diff --git a/contrib/format-access/pom.xml b/contrib/format-access/pom.xml
index 288bc9d92d..8262146671 100644
--- a/contrib/format-access/pom.xml
+++ b/contrib/format-access/pom.xml
@@ -39,7 +39,7 @@
<dependency>
<groupId>com.healthmarketscience.jackcess</groupId>
<artifactId>jackcess</artifactId>
- <version>4.0.8</version>
+ <version>4.0.11</version>
<exclusions>
<exclusion>
<groupId>commons-logging</groupId>
diff --git
a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessBatchReader.java
b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessBatchReader.java
index 735c2cc71f..070f793185 100644
---
a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessBatchReader.java
+++
b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessBatchReader.java
@@ -24,6 +24,8 @@ import com.healthmarketscience.jackcess.Database;
import com.healthmarketscience.jackcess.DatabaseBuilder;
import com.healthmarketscience.jackcess.Row;
import com.healthmarketscience.jackcess.Table;
+import com.healthmarketscience.jackcess.TableMetaData;
+import com.healthmarketscience.jackcess.util.LinkResolver;
import org.apache.commons.lang3.StringUtils;
import org.apache.drill.common.AutoCloseables;
import org.apache.drill.common.exceptions.CustomErrorContext;
@@ -58,6 +60,16 @@ public class MSAccessBatchReader implements ManagedReader {
private static final Logger logger =
LoggerFactory.getLogger(MSAccessBatchReader.class);
+ /**
+ * An Access file can name another database (a local path or a UNC share) as
the source of a
+ * linked table. Jackcess's default resolver would open it as the Drillbit
service account, so
+ * refuse instead unless {@link MSAccessFormatPlugin#ALLOW_LINKED_DATABASES}
is set. Linked tables
+ * are normally caught earlier with a clearer error; this is the backstop.
+ */
+ private static final LinkResolver REJECT_LINKED_DATABASES = (linkerDb,
linkeeFileName) -> {
+ throw new IOException("Refusing to open linked database referenced by this
MS Access file: " + linkeeFileName);
+ };
+
private final FileDescrip file;
private final CustomErrorContext errorContext;
private final RowSetLoader rowWriter;
@@ -65,6 +77,7 @@ public class MSAccessBatchReader implements ManagedReader {
private final List<MSAccessColumn> columnList;
private final MSAccessFormatConfig config;
private final boolean metadataOnly;
+ private final boolean allowLinkedDatabases;
private File tempFile;
private Set<String> tableList;
private Iterator<Row> rowIterator;
@@ -73,8 +86,10 @@ public class MSAccessBatchReader implements ManagedReader {
private Table table;
private Database db;
- public MSAccessBatchReader(FileSchemaNegotiator negotiator, File tempDir,
MSAccessFormatConfig config) {
+ public MSAccessBatchReader(FileSchemaNegotiator negotiator, File tempDir,
MSAccessFormatConfig config,
+ boolean allowLinkedDatabases) {
this.tempDir = tempDir;
+ this.allowLinkedDatabases = allowLinkedDatabases;
this.columnList = new ArrayList<>();
this.config = config;
this.file = negotiator.file();
@@ -123,10 +138,11 @@ public class MSAccessBatchReader implements ManagedReader
{
private TupleMetadata buildMetadataSchema(SchemaBuilder builder) {
// Adds the table name
builder.add("table", MinorType.VARCHAR);
- builder.add("created_date", MinorType.TIMESTAMP);
- builder.add("updated_date", MinorType.TIMESTAMP);
- builder.add("row_count", MinorType.INT);
- builder.add("col_count", MinorType.INT);
+ // Nullable because linked tables are listed without being opened, so
these are unknown.
+ builder.addNullable("created_date", MinorType.TIMESTAMP);
+ builder.addNullable("updated_date", MinorType.TIMESTAMP);
+ builder.addNullable("row_count", MinorType.INT);
+ builder.addNullable("col_count", MinorType.INT);
builder.addArray("columns", MinorType.VARCHAR);
return builder.buildSchema();
@@ -134,11 +150,28 @@ public class MSAccessBatchReader implements ManagedReader
{
private TupleMetadata buildSchemaFromTable(SchemaBuilder builder, String
tableName) {
try {
+ TableMetaData metaData = db.getTableMetaData(tableName);
+ if (metaData == null) {
+ deleteTempFile();
+ throw UserException.validationError()
+ .message("Table " + tableName + " not found.")
+ .addContext(errorContext)
+ .build(logger);
+ }
+ if (isBlockedLink(metaData)) {
+ deleteTempFile();
+ throw UserException.permissionError()
+ .message("Table " + tableName + " is a linked table that refers to
another database (" +
+ metaData.getLinkedDbName() + "). Reading linked tables is
disabled. An administrator can " +
+ "enable it by setting " +
MSAccessFormatPlugin.ALLOW_LINKED_DATABASES + " to true in
drill-override.conf.")
+ .addContext(errorContext)
+ .build(logger);
+ }
table = db.getTable(tableName);
} catch (IOException e) {
deleteTempFile();
throw UserException.dataReadError(e)
- .message("Table " + config.getTableName() + " not found. " +
e.getMessage())
+ .message("Error opening table " + tableName + ": " + e.getMessage())
.addContext(errorContext)
.build(logger);
}
@@ -247,7 +280,12 @@ public class MSAccessBatchReader implements ManagedReader {
private void openFile() {
try {
fsStream =
file.fileSystem().openPossiblyCompressedStream(file.split().getPath());
- db = DatabaseBuilder.open(convertInputStreamToFile(fsStream));
+ db = new DatabaseBuilder(convertInputStreamToFile(fsStream))
+ .setReadOnly(true)
+ .open();
+ if (!allowLinkedDatabases) {
+ db.setLinkResolver(REJECT_LINKED_DATABASES);
+ }
tableList = db.getTableNames();
} catch (IOException e) {
deleteTempFile();
@@ -258,9 +296,21 @@ public class MSAccessBatchReader implements ManagedReader {
}
}
+ private boolean isBlockedLink(TableMetaData metaData) {
+ return metaData.isLinked() && !allowLinkedDatabases;
+ }
+
private void processMetadataRow(String tableName) throws IOException {
Table table;
try {
+ TableMetaData metaData = db.getTableMetaData(tableName);
+ if (metaData != null && isBlockedLink(metaData)) {
+ // List the linked table without opening it, so the rest of the file
can still be explored.
+ rowWriter.start();
+ rowWriter.scalar("table").setString(tableName);
+ rowWriter.save();
+ return;
+ }
table = db.getTable(tableName);
} catch (IOException e) {
deleteTempFile();
@@ -274,7 +324,7 @@ public class MSAccessBatchReader implements ManagedReader {
rowWriter.scalar("table").setString(tableName);
LocalDateTime createdDate = table.getCreatedDate();
rowWriter.scalar("created_date").setTimestamp(createdDate.toInstant(ZoneOffset.UTC));
- LocalDateTime updatedDate = table.getCreatedDate();
+ LocalDateTime updatedDate = table.getUpdatedDate();
rowWriter.scalar("updated_date").setTimestamp(updatedDate.toInstant(ZoneOffset.UTC));
rowWriter.scalar("row_count").setInt(table.getRowCount());
rowWriter.scalar("col_count").setInt(table.getColumnCount());
diff --git
a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatPlugin.java
b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatPlugin.java
index 16508acfa4..38d9f1e750 100644
---
a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatPlugin.java
+++
b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatPlugin.java
@@ -39,20 +39,30 @@ import java.io.File;
public class MSAccessFormatPlugin extends
EasyFormatPlugin<MSAccessFormatConfig> {
protected static final String DEFAULT_NAME = "msaccess";
+
+ /**
+ * Boot option that lets the reader follow linked tables to other database
files. It is
+ * deliberately not part of {@link MSAccessFormatConfig}: every format
config field can be
+ * overridden from a table function, which would let any query user turn the
protection off.
+ */
+ public static final String ALLOW_LINKED_DATABASES =
"drill.exec.storage.msaccess.allow_linked_databases";
private final DrillbitContext context;
private static class MSAccessReaderFactory extends FileReaderFactory {
private final File tempDir;
private final MSAccessFormatConfig config;
- public MSAccessReaderFactory(File tempDir, MSAccessFormatConfig config) {
+ private final boolean allowLinkedDatabases;
+
+ public MSAccessReaderFactory(File tempDir, MSAccessFormatConfig config,
boolean allowLinkedDatabases) {
this.tempDir = tempDir;
this.config = config;
+ this.allowLinkedDatabases = allowLinkedDatabases;
}
@Override
public ManagedReader newReader(FileSchemaNegotiator negotiator) {
- return new MSAccessBatchReader(negotiator, tempDir, config);
+ return new MSAccessBatchReader(negotiator, tempDir, config,
allowLinkedDatabases);
}
}
@@ -82,7 +92,12 @@ public class MSAccessFormatPlugin extends
EasyFormatPlugin<MSAccessFormatConfig>
@Override
protected void configureScan(FileScanLifecycleBuilder builder, EasySubScan
scan) {
builder.nullType(Types.optional(TypeProtos.MinorType.VARCHAR));
- builder.readerFactory(new MSAccessReaderFactory(getTmpDir(),
formatConfig));
+ builder.readerFactory(new MSAccessReaderFactory(getTmpDir(), formatConfig,
allowLinkedDatabases()));
+ }
+
+ private boolean allowLinkedDatabases() {
+ DrillConfig config = context.getConfig();
+ return config.hasPath(ALLOW_LINKED_DATABASES) &&
config.getBoolean(ALLOW_LINKED_DATABASES);
}
/**
diff --git a/contrib/format-access/src/main/resources/drill-module.conf
b/contrib/format-access/src/main/resources/drill-module.conf
index 7004e22a8b..d7216e7565 100644
--- a/contrib/format-access/src/main/resources/drill-module.conf
+++ b/contrib/format-access/src/main/resources/drill-module.conf
@@ -22,3 +22,11 @@
drill.classpath.scanning: {
packages += "org.apache.drill.exec.store.msaccess"
}
+
+drill.exec.storage.msaccess: {
+ # An Access file can define linked tables that point at another database
file by an absolute
+ # local path or a UNC share. Following them makes the Drillbit open a path
chosen by the file's
+ # author, as the Drillbit service account, so this is off by default. It is
a boot option so
+ # that it can only be enabled by an administrator, not by a query.
+ allow_linked_databases: false
+}
diff --git
a/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessLinkedTables.java
b/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessLinkedTables.java
new file mode 100644
index 0000000000..4d4ec581f8
--- /dev/null
+++
b/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessLinkedTables.java
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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.apache.drill.exec.store.msaccess;
+
+import com.healthmarketscience.jackcess.ColumnBuilder;
+import com.healthmarketscience.jackcess.DataType;
+import com.healthmarketscience.jackcess.Database;
+import com.healthmarketscience.jackcess.DatabaseBuilder;
+import com.healthmarketscience.jackcess.Table;
+import com.healthmarketscience.jackcess.TableBuilder;
+import org.apache.drill.categories.RowSetTest;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.rowSet.RowSet;
+import org.apache.drill.exec.physical.rowSet.RowSetBuilder;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.File;
+
+/**
+ * Linked tables with {@link MSAccessFormatPlugin#ALLOW_LINKED_DATABASES}
enabled by the administrator.
+ * The default (disabled) behaviour is covered in {@link TestMSAccessReader}.
+ */
+@Category(RowSetTest.class)
+public class TestMSAccessLinkedTables extends ClusterTest {
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ ClusterTest.startCluster(ClusterFixture.builder(dirTestWatcher)
+ .configProperty(MSAccessFormatPlugin.ALLOW_LINKED_DATABASES, true));
+ }
+
+ @Test
+ public void testLinkedTableResolvedWhenAllowed() throws Exception {
+ File target = new File(dirTestWatcher.getRootDir(), "linked_target.mdb");
+ try (Database db = DatabaseBuilder.create(Database.FileFormat.V2003,
target)) {
+ Table table = new TableBuilder("Table1")
+ .addColumn(new ColumnBuilder("name", DataType.TEXT))
+ .toTable(db);
+ table.addRow("from the linked database");
+ }
+
+ File linker = new File(dirTestWatcher.getRootDir(), "linker.mdb");
+ try (Database db = DatabaseBuilder.create(Database.FileFormat.V2003,
linker)) {
+ db.createLinkedTable("Linked", target.getAbsolutePath(), "Table1");
+ }
+
+ String sql = "SELECT * FROM table(dfs.`" + linker.getName() + "` (type=>
'msaccess', tableName => 'Linked'))";
+ RowSet results = client.queryBuilder().sql(sql).rowSet();
+
+ TupleMetadata expectedSchema = new SchemaBuilder()
+ .addNullable("name", MinorType.VARCHAR)
+ .buildSchema();
+
+ RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+ .addRow("from the linked database")
+ .build();
+
+ new RowSetComparison(expected).verifyAndClearAll(results);
+ }
+}
diff --git
a/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessReader.java
b/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessReader.java
index e05e57f814..c966d1f60e 100644
---
a/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessReader.java
+++
b/contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessReader.java
@@ -18,10 +18,14 @@
package org.apache.drill.exec.store.msaccess;
+import com.healthmarketscience.jackcess.Database;
+import com.healthmarketscience.jackcess.DatabaseBuilder;
import org.apache.drill.categories.RowSetTest;
+import org.apache.drill.common.exceptions.UserRemoteException;
import org.apache.drill.common.types.TypeProtos.MinorType;
import org.apache.drill.exec.physical.rowSet.RowSet;
import org.apache.drill.exec.physical.rowSet.RowSetBuilder;
+import org.apache.drill.exec.proto.UserBitShared.DrillPBError;
import org.apache.drill.exec.record.metadata.SchemaBuilder;
import org.apache.drill.exec.record.metadata.TupleMetadata;
import org.apache.drill.test.ClusterFixture;
@@ -33,8 +37,14 @@ import org.junit.Test;
import org.junit.experimental.categories.Category;
+import java.io.File;
+
import static org.apache.drill.test.rowSet.RowSetUtilities.strArray;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
@Category(RowSetTest.class)
public class TestMSAccessReader extends ClusterTest {
@@ -44,6 +54,73 @@ public class TestMSAccessReader extends ClusterTest {
ClusterTest.startCluster(ClusterFixture.builder(dirTestWatcher));
}
+ /**
+ * Writes an Access file whose only table is linked to a database that does
not exist. The target is
+ * a path under the test directory so that nothing outside the sandbox (e.g.
an SMB share) is touched
+ * if the link were ever followed.
+ */
+ static File writeLinkedTableFile(String name, String linkedDbPath) throws
Exception {
+ File mdb = new File(dirTestWatcher.getRootDir(), name);
+ try (Database db = DatabaseBuilder.create(Database.FileFormat.V2003, mdb))
{
+ db.createLinkedTable("Linked", linkedDbPath, "Table1");
+ }
+ return mdb;
+ }
+
+ private static File writeLinkedTableFile(String name) throws Exception {
+ return writeLinkedTableFile(name, new File(dirTestWatcher.getRootDir(),
"missing_target.mdb").getAbsolutePath());
+ }
+
+ @Test
+ public void testLinkedTableIsNotResolved() throws Exception {
+ // A malicious file can point a linked table at any local path or UNC
share; reading it must
+ // not make the Drillbit open that path.
+ File mdb = writeLinkedTableFile("linked.mdb");
+ String sql = "SELECT * FROM table(dfs.`" + mdb.getName() + "` (type=>
'msaccess', tableName => 'Linked'))";
+ try {
+ client.queryBuilder().sql(sql).run();
+ fail("Expected the linked database to be refused");
+ } catch (UserRemoteException e) {
+ assertEquals(DrillPBError.ErrorType.PERMISSION, e.getErrorType());
+ assertTrue(e.getMessage(), e.getMessage().contains("Table Linked is a
linked table"));
+ assertTrue(e.getMessage(),
e.getMessage().contains(MSAccessFormatPlugin.ALLOW_LINKED_DATABASES));
+ }
+ }
+
+ @Test
+ public void testLinkedTableCannotBeEnabledFromTableFunction() throws
Exception {
+ // Enabling linked tables is a boot option, so a query must not be able to
switch it on.
+ File mdb = writeLinkedTableFile("linked_table_function.mdb");
+ String sql = "SELECT * FROM table(dfs.`" + mdb.getName() +
+ "` (type=> 'msaccess', tableName => 'Linked', allowLinkedDatabases =>
true))";
+ try {
+ client.queryBuilder().sql(sql).run();
+ fail("Expected the query to fail");
+ } catch (UserRemoteException e) {
+ assertNotEquals(e.getMessage(), DrillPBError.ErrorType.DATA_READ,
e.getErrorType());
+ assertFalse(e.getMessage(), e.getMessage().contains("does not exist"));
+ }
+ }
+
+ @Test
+ public void testMetadataQueryListsLinkedTable() throws Exception {
+ File mdb = writeLinkedTableFile("linked_metadata.mdb");
+ String sql = "SELECT `table`, row_count, col_count FROM dfs.`" +
mdb.getName() + "`";
+ RowSet results = client.queryBuilder().sql(sql).rowSet();
+
+ TupleMetadata expectedSchema = new SchemaBuilder()
+ .add("table", MinorType.VARCHAR)
+ .addNullable("row_count", MinorType.INT)
+ .addNullable("col_count", MinorType.INT)
+ .buildSchema();
+
+ RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+ .addRow("Linked", null, null)
+ .build();
+
+ new RowSetComparison(expected).verifyAndClearAll(results);
+ }
+
@Test
public void testStarQuery() throws Exception {
String sql = "SELECT * FROM table(cp.`data/V2019/extDateTestV2019.accdb`
(type=> 'msaccess', tableName => 'Table1')) LIMIT 5";
@@ -128,16 +205,16 @@ public class TestMSAccessReader extends ClusterTest {
TupleMetadata expectedSchema = new SchemaBuilder()
.add("table", MinorType.VARCHAR)
- .add("created_date", MinorType.TIMESTAMP)
- .add("updated_date", MinorType.TIMESTAMP)
- .add("row_count", MinorType.INT)
- .add("col_count", MinorType.INT)
+ .addNullable("created_date", MinorType.TIMESTAMP)
+ .addNullable("updated_date", MinorType.TIMESTAMP)
+ .addNullable("row_count", MinorType.INT)
+ .addNullable("col_count", MinorType.INT)
.addArray("columns", MinorType.VARCHAR)
.buildSchema();
RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
.addRow("Table1",
QueryTestUtil.ConvertDateToLong("2021-06-03T20:09:56.993Z"),
- QueryTestUtil.ConvertDateToLong("2021-06-03T20:09:56.993Z"), 9, 6,
strArray("ID", "Field1", "DateExt", "DateNormal", "DateExtStr",
"DateNormalCalc"))
+ QueryTestUtil.ConvertDateToLong("2021-06-03T20:25:26.483Z"), 9, 6,
strArray("ID", "Field1", "DateExt", "DateNormal", "DateExtStr",
"DateNormalCalc"))
.build();
new RowSetComparison(expected).verifyAndClearAll(results);
