From 9aeb5052c62af64ae08a87b8b45e407aba585d81 Mon Sep 17 00:00:00 2001 From: cgivre Date: Wed, 9 Sep 2026 09:58:31 -0400 Subject: [PATCH 1/2] DRILL-8554: Update Jackcess and Fix Linked Table Issue --- contrib/format-access/README.md | 22 ++++++++++ contrib/format-access/pom.xml | 2 +- .../store/msaccess/MSAccessBatchReader.java | 17 +++++++- .../store/msaccess/MSAccessFormatConfig.java | 16 ++++++-- .../store/msaccess/TestMSAccessReader.java | 41 +++++++++++++++++++ 5 files changed, 93 insertions(+), 5 deletions(-) diff --git a/contrib/format-access/README.md b/contrib/format-access/README.md index 0c3035117aa..ab210a0e727 100644 --- a/contrib/format-access/README.md +++ b/contrib/format-access/README.md @@ -11,6 +11,28 @@ 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. + +If you trust the files being queried and need linked tables to work, set `allowLinkedDatabases` to `true`: + +```json +"msaccess": { + "type": "msaccess", + "extensions": ["mdb", "accdb"], + "allowLinkedDatabases": true +} +``` + +It can also be enabled per query: + +```sql +SELECT * +FROM table(dfs.`file_name.accdb` (type=> 'msaccess', tableName => 'Linked', allowLinkedDatabases => true)) +``` + ## 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 288bc9d92d9..82621466717 100644 --- a/contrib/format-access/pom.xml +++ b/contrib/format-access/pom.xml @@ -39,7 +39,7 @@ com.healthmarketscience.jackcess jackcess - 4.0.8 + 4.0.11 commons-logging 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 735c2cc71fc..9989e95b53d 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 @@ -22,6 +22,7 @@ import com.healthmarketscience.jackcess.DataType; import com.healthmarketscience.jackcess.Database; import com.healthmarketscience.jackcess.DatabaseBuilder; +import com.healthmarketscience.jackcess.util.LinkResolver; import com.healthmarketscience.jackcess.Row; import com.healthmarketscience.jackcess.Table; import org.apache.commons.lang3.StringUtils; @@ -58,6 +59,15 @@ 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 the plugin config explicitly opts in. + */ + 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; @@ -247,7 +257,12 @@ public boolean next() { 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 (!config.getAllowLinkedDatabases()) { + db.setLinkResolver(REJECT_LINKED_DATABASES); + } tableList = db.getTableNames(); } catch (IOException e) { deleteTempFile(); diff --git a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java index 3a498f7b222..74bf434ba65 100644 --- a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java +++ b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java @@ -36,13 +36,16 @@ public class MSAccessFormatConfig implements FormatPluginConfig { private final List extensions; private final String tableName; + private final boolean allowLinkedDatabases; // Omitted properties take reasonable defaults @JsonCreator public MSAccessFormatConfig(@JsonProperty("extensions") List extensions, - @JsonProperty("tableName") String tableName) { + @JsonProperty("tableName") String tableName, + @JsonProperty("allowLinkedDatabases") Boolean allowLinkedDatabases) { this.extensions = extensions == null ? Arrays.asList("accdb", "mdb") : ImmutableList.copyOf(extensions); this.tableName = tableName; + this.allowLinkedDatabases = allowLinkedDatabases != null && allowLinkedDatabases; } @JsonInclude(Include.NON_DEFAULT) @@ -55,6 +58,11 @@ public String getTableName() { return tableName; } + @JsonInclude(Include.NON_DEFAULT) + public boolean getAllowLinkedDatabases() { + return allowLinkedDatabases; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -65,12 +73,13 @@ public boolean equals(Object o) { } MSAccessFormatConfig that = (MSAccessFormatConfig) o; return Objects.equals(extensions, that.extensions) && - Objects.equals(tableName, that.tableName); + Objects.equals(tableName, that.tableName) && + allowLinkedDatabases == that.allowLinkedDatabases; } @Override public int hashCode() { - return Objects.hash(extensions, tableName); + return Objects.hash(extensions, tableName, allowLinkedDatabases); } @Override @@ -78,6 +87,7 @@ public String toString() { return new PlanStringBuilder(this) .field("extensions", extensions) .field("tableName", tableName) + .field("allowLinkedDatabases", allowLinkedDatabases) .toString(); } } 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 e05e57f814f..085206597f6 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,6 +18,8 @@ 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.types.TypeProtos.MinorType; import org.apache.drill.exec.physical.rowSet.RowSet; @@ -33,8 +35,12 @@ 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.assertTrue; +import static org.junit.Assert.fail; @Category(RowSetTest.class) public class TestMSAccessReader extends ClusterTest { @@ -44,6 +50,41 @@ public static void setup() throws Exception { ClusterTest.startCluster(ClusterFixture.builder(dirTestWatcher)); } + @Test + public void testLinkedTableResolvedWhenAllowed() throws Exception { + String sql = "SELECT * FROM table(dfs.`" + writeLinkedTableFile("linked_allowed.mdb").getName() + + "` (type=> 'msaccess', tableName => 'Linked', allowLinkedDatabases => true))"; + try { + client.queryBuilder().sql(sql).run(); + fail("Expected the (nonexistent) linked database to be opened"); + } catch (Exception e) { + // Jackcess got as far as trying to open the linked path, which is what the option enables. + assertTrue(e.getMessage(), e.getMessage().contains("given file does not exist")); + } + } + + private static File writeLinkedTableFile(String name) throws Exception { + File mdb = new File(dirTestWatcher.getRootDir(), name); + try (Database db = DatabaseBuilder.create(Database.FileFormat.V2003, mdb)) { + db.createLinkedTable("Linked", "//evil.example.com/share/secret.mdb", "Table1"); + } + return mdb; + } + + @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 (Exception e) { + assertTrue(e.getMessage(), e.getMessage().contains("Refusing to open linked database")); + } + } + @Test public void testStarQuery() throws Exception { String sql = "SELECT * FROM table(cp.`data/V2019/extDateTestV2019.accdb` (type=> 'msaccess', tableName => 'Table1')) LIMIT 5"; From c5ac8ee7296f2f3d3f49656e72679c72f1b4ae37 Mon Sep 17 00:00:00 2001 From: Charles Givre Date: Thu, 8 Oct 2026 08:53:45 -0400 Subject: [PATCH 2/2] DRILL-8554: Make linked tables an admin-only boot option Address review feedback on the MS Access linked table fix: - Move allowLinkedDatabases out of the format config, where any query user could enable it through a table function, into the boot option drill.exec.storage.msaccess.allow_linked_databases. - List linked tables in metadata queries without opening them, instead of failing the whole query. Metadata dates and counts are now nullable. - Report a clear permission error naming the boot option when a query reads a linked table, and a proper error when a table does not exist. - Fix updated_date, which was reading the created date. - Test against a real linked database under the test directory rather than a UNC path, and test that a table function cannot enable links. - Document the behaviour change in the README. --- contrib/format-access/README.md | 22 ++--- .../store/msaccess/MSAccessBatchReader.java | 55 +++++++++--- .../store/msaccess/MSAccessFormatConfig.java | 16 +--- .../store/msaccess/MSAccessFormatPlugin.java | 21 ++++- .../src/main/resources/drill-module.conf | 8 ++ .../msaccess/TestMSAccessLinkedTables.java | 83 +++++++++++++++++++ .../store/msaccess/TestMSAccessReader.java | 80 +++++++++++++----- 7 files changed, 224 insertions(+), 61 deletions(-) create mode 100644 contrib/format-access/src/test/java/org/apache/drill/exec/store/msaccess/TestMSAccessLinkedTables.java diff --git a/contrib/format-access/README.md b/contrib/format-access/README.md index ab210a0e727..230eda91dff 100644 --- a/contrib/format-access/README.md +++ b/contrib/format-access/README.md @@ -16,22 +16,18 @@ An Access file can define tables that live in another database file, referenced 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. -If you trust the files being queried and need linked tables to work, set `allowLinkedDatabases` to `true`: +**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. -```json -"msaccess": { - "type": "msaccess", - "extensions": ["mdb", "accdb"], - "allowLinkedDatabases": true -} -``` +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: -It can also be enabled per query: - -```sql -SELECT * -FROM table(dfs.`file_name.accdb` (type=> 'msaccess', tableName => 'Linked', allowLinkedDatabases => true)) ``` +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/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 9989e95b53d..070f7931852 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 @@ -22,9 +22,10 @@ import com.healthmarketscience.jackcess.DataType; import com.healthmarketscience.jackcess.Database; import com.healthmarketscience.jackcess.DatabaseBuilder; -import com.healthmarketscience.jackcess.util.LinkResolver; 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; @@ -62,7 +63,8 @@ public class MSAccessBatchReader implements ManagedReader { /** * 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 the plugin config explicitly opts in. + * 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); @@ -75,6 +77,7 @@ public class MSAccessBatchReader implements ManagedReader { private final List columnList; private final MSAccessFormatConfig config; private final boolean metadataOnly; + private final boolean allowLinkedDatabases; private File tempFile; private Set tableList; private Iterator rowIterator; @@ -83,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(); @@ -133,10 +138,11 @@ private void buildSchema(FileSchemaNegotiator negotiator) { 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(); @@ -144,11 +150,28 @@ private TupleMetadata buildMetadataSchema(SchemaBuilder builder) { 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); } @@ -260,7 +283,7 @@ private void openFile() { db = new DatabaseBuilder(convertInputStreamToFile(fsStream)) .setReadOnly(true) .open(); - if (!config.getAllowLinkedDatabases()) { + if (!allowLinkedDatabases) { db.setLinkResolver(REJECT_LINKED_DATABASES); } tableList = db.getTableNames(); @@ -273,9 +296,21 @@ private void openFile() { } } + 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(); @@ -289,7 +324,7 @@ private void processMetadataRow(String tableName) throws IOException { 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/MSAccessFormatConfig.java b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java index 74bf434ba65..3a498f7b222 100644 --- a/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java +++ b/contrib/format-access/src/main/java/org/apache/drill/exec/store/msaccess/MSAccessFormatConfig.java @@ -36,16 +36,13 @@ public class MSAccessFormatConfig implements FormatPluginConfig { private final List extensions; private final String tableName; - private final boolean allowLinkedDatabases; // Omitted properties take reasonable defaults @JsonCreator public MSAccessFormatConfig(@JsonProperty("extensions") List extensions, - @JsonProperty("tableName") String tableName, - @JsonProperty("allowLinkedDatabases") Boolean allowLinkedDatabases) { + @JsonProperty("tableName") String tableName) { this.extensions = extensions == null ? Arrays.asList("accdb", "mdb") : ImmutableList.copyOf(extensions); this.tableName = tableName; - this.allowLinkedDatabases = allowLinkedDatabases != null && allowLinkedDatabases; } @JsonInclude(Include.NON_DEFAULT) @@ -58,11 +55,6 @@ public String getTableName() { return tableName; } - @JsonInclude(Include.NON_DEFAULT) - public boolean getAllowLinkedDatabases() { - return allowLinkedDatabases; - } - @Override public boolean equals(Object o) { if (this == o) { @@ -73,13 +65,12 @@ public boolean equals(Object o) { } MSAccessFormatConfig that = (MSAccessFormatConfig) o; return Objects.equals(extensions, that.extensions) && - Objects.equals(tableName, that.tableName) && - allowLinkedDatabases == that.allowLinkedDatabases; + Objects.equals(tableName, that.tableName); } @Override public int hashCode() { - return Objects.hash(extensions, tableName, allowLinkedDatabases); + return Objects.hash(extensions, tableName); } @Override @@ -87,7 +78,6 @@ public String toString() { return new PlanStringBuilder(this) .field("extensions", extensions) .field("tableName", tableName) - .field("allowLinkedDatabases", allowLinkedDatabases) .toString(); } } 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 16508acfa42..38d9f1e7502 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 @@ public class MSAccessFormatPlugin extends EasyFormatPlugin { 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 @@ private static EasyFormatConfig easyConfig(Configuration fsConf, MSAccessFormatC @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 7004e22a8bb..d7216e75654 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 00000000000..4d4ec581f8d --- /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 085206597f6..c966d1f60eb 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 @@ -21,9 +21,11 @@ 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; @@ -39,6 +41,8 @@ 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; @@ -50,27 +54,23 @@ public static void setup() throws Exception { ClusterTest.startCluster(ClusterFixture.builder(dirTestWatcher)); } - @Test - public void testLinkedTableResolvedWhenAllowed() throws Exception { - String sql = "SELECT * FROM table(dfs.`" + writeLinkedTableFile("linked_allowed.mdb").getName() + - "` (type=> 'msaccess', tableName => 'Linked', allowLinkedDatabases => true))"; - try { - client.queryBuilder().sql(sql).run(); - fail("Expected the (nonexistent) linked database to be opened"); - } catch (Exception e) { - // Jackcess got as far as trying to open the linked path, which is what the option enables. - assertTrue(e.getMessage(), e.getMessage().contains("given file does not exist")); - } - } - - private static File writeLinkedTableFile(String name) throws Exception { + /** + * 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", "//evil.example.com/share/secret.mdb", "Table1"); + 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 @@ -80,11 +80,47 @@ public void testLinkedTableIsNotResolved() throws Exception { try { client.queryBuilder().sql(sql).run(); fail("Expected the linked database to be refused"); - } catch (Exception e) { - assertTrue(e.getMessage(), e.getMessage().contains("Refusing to open linked database")); + } 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"; @@ -169,16 +205,16 @@ public void testMetadataStarQuery() throws Exception { 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);