From a7fb7aeff4bd140374dbbe1a045080b038c51738 Mon Sep 17 00:00:00 2001 From: lxk955 <164191365+lxk955@users.noreply.github.com> Date: Sun, 18 Oct 2026 10:53:17 +0800 Subject: [PATCH] fix(gbase8s): strip NUL characters from column defaults and table DDL --- .../main/java/com/dbx/agent/DdlBuilder.java | 46 ++++++++-- .../agent/CommonJavaCompatibilityTest.java | 29 +++++++ .../com/dbx/agent/DdlBuilderDefaultTest.java | 27 ++++++ .../com/dbx/agent/gbase8s/Gbase8sAgent.java | 41 ++++++++- .../dbx/agent/gbase8s/Gbase8sAgentTest.java | 87 +++++++++++++++++++ .../metadata/__tests__/objectDdlCache.spec.ts | 19 ++++ .../src/lib/metadata/objectDdlCache.ts | 9 +- crates/dbx-core/src/schema/mod.rs | 76 ++++++++++------ 8 files changed, 297 insertions(+), 37 deletions(-) create mode 100644 agents/common/src/test/java/com/dbx/agent/DdlBuilderDefaultTest.java diff --git a/agents/common/src/main/java/com/dbx/agent/DdlBuilder.java b/agents/common/src/main/java/com/dbx/agent/DdlBuilder.java index 281c8c9fa3..34f0f000b7 100644 --- a/agents/common/src/main/java/com/dbx/agent/DdlBuilder.java +++ b/agents/common/src/main/java/com/dbx/agent/DdlBuilder.java @@ -100,9 +100,10 @@ public final class DdlBuilder { if (!column.getIs_nullable()) { line.append(" NOT NULL"); } - if (notBlank(column.getColumn_default())) { + String columnDefault = sanitizeDefault(column.getColumn_default()); + if (notBlank(columnDefault)) { line.append(" DEFAULT "); - line.append(column.getColumn_default()); + line.append(columnDefault); } columnLines.add(line.toString()); } @@ -387,9 +388,10 @@ public final class DdlBuilder { } private static String quoteIdent(String identifier, boolean useBacktick) { + String clean = sanitizeString(identifier); return useBacktick - ? JdbcIdentifiers.INSTANCE.backtick(identifier) - : JdbcIdentifiers.INSTANCE.doubleQuote(identifier); + ? JdbcIdentifiers.INSTANCE.backtick(clean) + : JdbcIdentifiers.INSTANCE.doubleQuote(clean); } private static String qualifiedName(String schema, String name, boolean useBacktick) { @@ -438,11 +440,45 @@ public final class DdlBuilder { return matcher.matches() ? "IDENTITY(" + matcher.group(1) + "," + matcher.group(2) + ")" : null; } + private static String sanitizeString(String value) { + if (value == null) { + return ""; + } + int nullIdx = value.indexOf('\0'); + return nullIdx >= 0 ? value.substring(0, nullIdx) : value; + } + + private static String sanitizeDefault(String defaultValue) { + if (defaultValue == null) { + return null; + } + int nullIdx = defaultValue.indexOf('\0'); + if (nullIdx < 0) { + return defaultValue; + } + String truncated = defaultValue.substring(0, nullIdx).trim(); + return truncated.isEmpty() ? null : truncated; + } + private static boolean notBlank(String value) { - return value != null && !value.trim().isEmpty(); + if (value == null) { + return false; + } + int nullIdx = value.indexOf('\0'); + if (nullIdx >= 0) { + value = value.substring(0, nullIdx); + } + return !value.trim().isEmpty(); } private static String sqlStringBody(String value) { + if (value == null) { + return ""; + } + int nullIdx = value.indexOf('\0'); + if (nullIdx >= 0) { + value = value.substring(0, nullIdx); + } return value.replace("'", "''"); } diff --git a/agents/common/src/test/java/com/dbx/agent/CommonJavaCompatibilityTest.java b/agents/common/src/test/java/com/dbx/agent/CommonJavaCompatibilityTest.java index be35c61b69..03f41bd371 100644 --- a/agents/common/src/test/java/com/dbx/agent/CommonJavaCompatibilityTest.java +++ b/agents/common/src/test/java/com/dbx/agent/CommonJavaCompatibilityTest.java @@ -970,6 +970,35 @@ class CommonJavaCompatibilityTest { assertEquals(ddl, DdlBuilder.appendTrailingSql(ddl, " ")); } + @Test + void ddlBuilderStripsNulCharactersFromDefaultsAndComments() { + ColumnInfo col1 = new ColumnInfo("status\0ignored", "int", false, "'2'\0\0\0", false); + col1.setComment("status comment\0ignored"); + ColumnInfo col2 = new ColumnInfo("name", "varchar", true, "''\0\0\0", false); + ColumnInfo col3 = new ColumnInfo("blank_default", "int", true, "\0\0\0", false); + + String ddl = DdlBuilder.buildTableDdl( + "app\0ignored", + "users\0ignored", + Arrays.asList(col1, col2, col3), + Collections.emptyList(), + Collections.emptyList(), + Collections.emptyList(), + false, + true, + "table comment\0ignored" + ); + + assertFalse(ddl.contains("\0"), "DDL must not contain NUL bytes"); + assertTrue(ddl.contains("CREATE TABLE \"app\".\"users\"")); + assertTrue(ddl.contains("\"status\" int NOT NULL DEFAULT '2'")); + assertTrue(ddl.contains("\"name\" varchar DEFAULT ''")); + assertTrue(ddl.contains("\"blank_default\" int")); + assertFalse(ddl.contains("\"blank_default\" int DEFAULT")); + assertTrue(ddl.contains("COMMENT ON TABLE \"app\".\"users\" IS 'table comment';")); + assertTrue(ddl.contains("COMMENT ON COLUMN \"app\".\"users\".\"status\" IS 'status comment';")); + } + private static class MinimalAgent implements DatabaseAgent { private ExecuteQueryOptions lastExecuteOptions; diff --git a/agents/common/src/test/java/com/dbx/agent/DdlBuilderDefaultTest.java b/agents/common/src/test/java/com/dbx/agent/DdlBuilderDefaultTest.java new file mode 100644 index 0000000000..4d53716ce8 --- /dev/null +++ b/agents/common/src/test/java/com/dbx/agent/DdlBuilderDefaultTest.java @@ -0,0 +1,27 @@ +package com.dbx.agent; + +import java.util.List; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +class DdlBuilderDefaultTest { + + @Test + void nulFreeDefaultsPassThroughUnchanged() { + ColumnInfo padded = new ColumnInfo("padded", "INTEGER", true, " 42 ", false); + String ddl = DdlBuilder.buildTableDdl("root", "test_table", List.of(padded), List.of(), List.of()); + Assertions.assertFalse(ddl.contains("\0"), "DDL must not contain NUL bytes"); + Assertions.assertTrue(ddl.contains("DEFAULT 42 "), "NUL-free defaults must not be trimmed"); + } + + @Test + void nulTruncatedDefaultsAreTrimmedAndDroppedWhenEmpty() { + ColumnInfo padded = new ColumnInfo("padded", "INTEGER", true, " 42 \0pad", false); + ColumnInfo empty = new ColumnInfo("empty_default", "INTEGER", true, "\0junk", false); + String ddl = DdlBuilder.buildTableDdl("root", "test_table", List.of(padded, empty), List.of(), List.of()); + Assertions.assertTrue(ddl.contains("DEFAULT 42")); + Assertions.assertFalse(ddl.contains("DEFAULT pad")); + Assertions.assertTrue(ddl.contains("\"empty_default\" INTEGER")); + Assertions.assertFalse(ddl.contains("\"empty_default\" INTEGER NOT NULL DEFAULT")); + } +} diff --git a/agents/drivers/gbase8s/src/main/java/com/dbx/agent/gbase8s/Gbase8sAgent.java b/agents/drivers/gbase8s/src/main/java/com/dbx/agent/gbase8s/Gbase8sAgent.java index 70734e5bf5..7fe210f4b0 100644 --- a/agents/drivers/gbase8s/src/main/java/com/dbx/agent/gbase8s/Gbase8sAgent.java +++ b/agents/drivers/gbase8s/src/main/java/com/dbx/agent/gbase8s/Gbase8sAgent.java @@ -613,7 +613,7 @@ public final class Gbase8sAgent extends ConfiguredJdbcAgent { bind(stmt, args); try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { - String value = rs.getString("column_default"); + String value = sanitizeColumnDefault(rs.getString("column_default")); if (value != null) { defaults.put(trim(rs.getString("colname")), value); } @@ -625,6 +625,18 @@ public final class Gbase8sAgent extends ConfiguredJdbcAgent { return defaults; } + public static String sanitizeColumnDefault(String value) { + if (value == null) { + return null; + } + int nullIndex = value.indexOf('\0'); + if (nullIndex >= 0) { + value = value.substring(0, nullIndex); + } + value = value.trim(); + return value.isEmpty() ? null : value; + } + private static boolean isMysqlCompatMode(ConnectParams params) { if (params == null) { return false; @@ -928,7 +940,14 @@ public final class Gbase8sAgent extends ConfiguredJdbcAgent { try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { String chunk = rs.getString("viewtext"); - source.append(chunk == null ? "" : chunk); + if (chunk != null) { + int nullIndex = chunk.indexOf('\0'); + if (nullIndex >= 0) { + source.append(chunk, 0, nullIndex); + } else { + source.append(chunk); + } + } } } } @@ -1199,7 +1218,14 @@ public final class Gbase8sAgent extends ConfiguredJdbcAgent { } private static String trim(String value) { - return value == null ? "" : value.trim(); + if (value == null) { + return ""; + } + int nullIndex = value.indexOf('\0'); + if (nullIndex >= 0) { + value = value.substring(0, nullIndex); + } + return value.trim(); } private static String emptyToNull(String value) { @@ -1207,7 +1233,14 @@ public final class Gbase8sAgent extends ConfiguredJdbcAgent { } private static String stripTrailing(String value) { - return value == null ? "" : value.stripTrailing(); + if (value == null) { + return ""; + } + int nullIndex = value.indexOf('\0'); + if (nullIndex >= 0) { + value = value.substring(0, nullIndex); + } + return value.stripTrailing(); } private String currentCatalog() { diff --git a/agents/drivers/gbase8s/src/test/java/com/dbx/agent/gbase8s/Gbase8sAgentTest.java b/agents/drivers/gbase8s/src/test/java/com/dbx/agent/gbase8s/Gbase8sAgentTest.java index d771c4434e..765cde6275 100644 --- a/agents/drivers/gbase8s/src/test/java/com/dbx/agent/gbase8s/Gbase8sAgentTest.java +++ b/agents/drivers/gbase8s/src/test/java/com/dbx/agent/gbase8s/Gbase8sAgentTest.java @@ -577,6 +577,66 @@ class Gbase8sAgentTest { Assertions.assertEquals("current_timestamp", columns.get(2).getColumn_default()); } + @Test + void sanitizeColumnDefaultStripsTrailingNulBytesAndEmptyExpressions() { + Assertions.assertEquals("'2'", Gbase8sAgent.sanitizeColumnDefault("'2'\0\0\0")); + Assertions.assertEquals("''", Gbase8sAgent.sanitizeColumnDefault("''\0\0\0")); + Assertions.assertEquals("'0'", Gbase8sAgent.sanitizeColumnDefault("'0' \0\0")); + Assertions.assertEquals("current_timestamp", Gbase8sAgent.sanitizeColumnDefault("current_timestamp\0")); + Assertions.assertNull(Gbase8sAgent.sanitizeColumnDefault("\0\0\0")); + Assertions.assertNull(Gbase8sAgent.sanitizeColumnDefault(" \0 ")); + Assertions.assertNull(Gbase8sAgent.sanitizeColumnDefault(null)); + Assertions.assertNull(Gbase8sAgent.sanitizeColumnDefault("")); + } + + @Test + void getColumnsAndDdlStripTrailingNulBytesFromDefaults() throws Exception { + List sql = new ArrayList<>(); + Gbase8sAgent agent = new Gbase8sAgent(); + Connection connection = preparedConnection( + sql, + resultSet( + new String[]{"part1", "part2", "part3", "part4", "part5", "part6", "part7", "part8", "part9", "part10", "part11", "part12", "part13", "part14", "part15", "part16"}, + new Object[][]{{1, 0, null, null, null, null, null, null, null, null, null, null, null, null, null, null}} + ), + resultSet( + new String[]{"colname", "column_default"}, + new Object[][]{ + {"show_status", "'2'\0\0\0\0\0"}, + {"name", "''\0\0\0\0\0"}, + {"empty_default", "\0\0\0"} + } + ), + resultSet( + new String[]{"colname", "coltype", "colno", "collength", "comments"}, + new Object[][]{ + {"id", 258, 1, 4, null}, + {"show_status", 257, 2, 2, null}, + {"name", 269, 3, 765, null}, + {"empty_default", 258, 4, 4, null} + } + ) + ); + TestSupport.setPrivateConnection(agent, connection); + agent.afterConnect(new ConnectParams( + "localhost", 9088, "appdb", "", "", "GBASEDBTSERVER=gbase8s;", "", false + ), connection); + + List columns = agent.getColumns("root", "test_table"); + Assertions.assertEquals(4, columns.size()); + Assertions.assertNull(columns.get(0).getColumn_default()); + Assertions.assertEquals("'2'", columns.get(1).getColumn_default()); + Assertions.assertEquals("''", columns.get(2).getColumn_default()); + Assertions.assertNull(columns.get(3).getColumn_default()); + + String ddl = DdlBuilder.buildTableDdl("root", "test_table", columns, List.of(), List.of()); + Assertions.assertFalse(ddl.contains("\0"), "DDL must not contain NUL bytes"); + Assertions.assertTrue(ddl.contains("\"show_status\" SMALLINT NOT NULL DEFAULT '2'")); + Assertions.assertTrue(ddl.contains("\"name\" VARCHAR(765) NOT NULL DEFAULT ''")); + Assertions.assertTrue(ddl.contains("\"empty_default\" INTEGER NOT NULL")); + Assertions.assertFalse(ddl.contains("\"empty_default\" INTEGER NOT NULL DEFAULT")); + } + @Test void getColumnsUsesMysqlModeFromCustomConnectionStringBeforeUrlParams() { String sql = defaultExpressionSql( @@ -764,6 +824,33 @@ class Gbase8sAgentTest { Assertions.assertTrue(sql.get(1).contains("FROM sysviews"), sql.get(1)); } + @Test + void getTableDdlStripsNulBytesFromSysviewsChunks() { + List sql = new ArrayList<>(); + Gbase8sAgent agent = new Gbase8sAgent(); + TestSupport.setPrivateConnection(agent, preparedConnection( + sql, + resultSet( + new String[]{"tabtype"}, + new Object[][]{ + {"V"} + } + ), + resultSet( + new String[]{"viewtext"}, + new Object[][]{ + {"create view v1 as select 1 as id; \0\0\0"}, + {"\0\0\0"} + } + ) + )); + + String ddl = agent.getTableDdl("gbasedbt", "v1"); + + Assertions.assertEquals("create view v1 as select 1 as id;", ddl); + Assertions.assertFalse(ddl.contains("\0")); + } + private static Connection preparedConnection(List sql, ResultSet... resultSets) { int[] resultIndex = {0}; PreparedStatement statement = proxy(PreparedStatement.class, (method, args) -> { diff --git a/apps/desktop/src/lib/metadata/__tests__/objectDdlCache.spec.ts b/apps/desktop/src/lib/metadata/__tests__/objectDdlCache.spec.ts index d179e1aa15..9c43d6765b 100644 --- a/apps/desktop/src/lib/metadata/__tests__/objectDdlCache.spec.ts +++ b/apps/desktop/src/lib/metadata/__tests__/objectDdlCache.spec.ts @@ -434,5 +434,24 @@ describe("objectDdlCache", () => { expect(mocks.deleteSchemaCachePrefix).toHaveBeenCalledWith("object-ddl:v1:a%3Ab%20c%25%E4%B8%AD:"); expect(mocks.deleteSchemaCachePrefix).toHaveBeenCalledWith("object-meta:v1:a%3Ab%20c%25%E4%B8%AD:"); }); + + it("strips NUL bytes from remote and cached DDL", async () => { + mocks.getTableDisplayDdl.mockResolvedValue("CREATE TABLE t (id int DEFAULT '2'\0\0);"); + + const remoteResult = await loadObjectDdl(request, { force: true }); + expect(remoteResult.ddl).toBe("CREATE TABLE t (id int DEFAULT '2');"); + expect(remoteResult.ddl).not.toContain("\0"); + + mocks.persisted.set(objectDdlCacheKey(request), { + version: 1, + cachedAt: new Date().toISOString(), + ddl: "CREATE TABLE t (name varchar DEFAULT ''\0\0);", + }); + clearMetadataRuntimeCache(); + + const diskResult = await loadObjectDdl(request); + expect(diskResult.ddl).toBe("CREATE TABLE t (name varchar DEFAULT '');"); + expect(diskResult.ddl).not.toContain("\0"); + }); }); }); diff --git a/apps/desktop/src/lib/metadata/objectDdlCache.ts b/apps/desktop/src/lib/metadata/objectDdlCache.ts index 686c828738..9ed528dc3e 100644 --- a/apps/desktop/src/lib/metadata/objectDdlCache.ts +++ b/apps/desktop/src/lib/metadata/objectDdlCache.ts @@ -81,12 +81,16 @@ function invalidationPrefix(match: MetadataCacheInvalidation): string { return `${parts.join(":")}:`; } +function sanitizeDdl(ddl: string): string { + return ddl.includes("\0") ? ddl.replaceAll("\0", "") : ddl; +} + function decodeCachedDdl(payload: unknown): string | null { if (!payload || typeof payload !== "object") return null; const envelope = payload as Partial; const cachedAt = typeof envelope.cachedAt === "string" ? Date.parse(envelope.cachedAt) : Number.NaN; if (envelope.version !== 1 || !Number.isFinite(cachedAt) || Date.now() - cachedAt > MAX_PERSISTED_DDL_AGE_MS || typeof envelope.ddl !== "string") return null; - return envelope.ddl; + return sanitizeDdl(envelope.ddl); } async function waitForPendingInvalidations(cacheKey: string): Promise { @@ -151,7 +155,8 @@ async function loadRemoteDdl(request: ObjectDdlRequest, cacheKey: string, force: signal.addEventListener("abort", cancel, { once: true }); recordMetadataCacheRemoteMiss(cacheKey); entry.promise = waitForMetadata( - api.getTableDisplayDdl(request.connectionId, request.database, request.schema, request.tableName, request.objectType, request.catalog).then((ddl) => { + api.getTableDisplayDdl(request.connectionId, request.database, request.schema, request.tableName, request.objectType, request.catalog).then((rawDdl) => { + const ddl = sanitizeDdl(rawDdl); const current = !entry.invalidated; if (current && ddl.length <= MAX_PERSISTED_DDL_CHARS) { const envelope: ObjectDdlCacheEnvelope = { version: 1, cachedAt: new Date().toISOString(), ddl }; diff --git a/crates/dbx-core/src/schema/mod.rs b/crates/dbx-core/src/schema/mod.rs index 669f3dff25..524c9bdeae 100644 --- a/crates/dbx-core/src/schema/mod.rs +++ b/crates/dbx-core/src/schema/mod.rs @@ -9770,19 +9770,21 @@ async fn get_table_ddl_core_with_options( .get_or_create_metadata_pool_for_session(connection_id, Some(database), client_session_id) .await .ok(); - return Ok(enrich_ddl_with_oracle_style_comments( - state, - connection_id, - database, - schema, - table, - &ddl, - client_session_id, - pool_key.as_deref(), - ) - .await); + return Ok(sanitize_ddl_string( + enrich_ddl_with_oracle_style_comments( + state, + connection_id, + database, + schema, + table, + &ddl, + client_session_id, + pool_key.as_deref(), + ) + .await, + )); } - return Ok(ddl); + return Ok(sanitize_ddl_string(ddl)); } if matches!(object_type, Some(db::ObjectSourceKind::MaterializedView)) { let source = get_object_source_core( @@ -9796,7 +9798,7 @@ async fn get_table_ddl_core_with_options( None, ) .await?; - return Ok(source.source); + return Ok(sanitize_ddl_string(source.source)); } if let Some(kind) = object_type.clone().filter(ddl_kind_uses_object_source) { // Routines, packages, triggers, types and the other schema objects have no @@ -9805,7 +9807,7 @@ async fn get_table_ddl_core_with_options( // instead, and keep the previous behaviour when the driver cannot produce // one (engine without a source query, empty definition, …). match get_object_source_core(state, connection_id, database, schema, table, kind, None, None).await { - Ok(source) if !source.source.trim().is_empty() => return Ok(source.source), + Ok(source) if !source.source.trim().is_empty() => return Ok(sanitize_ddl_string(source.source)), Ok(_) => {} Err(error) => { log::debug!( @@ -9824,22 +9826,31 @@ async fn get_table_ddl_core_with_options( if should_append_oracle_style_comment_ddl(db_config.as_ref()) { let pool_key = state.get_or_create_metadata_pool_for_session(connection_id, Some(database), client_session_id).await.ok(); - Ok(enrich_ddl_with_oracle_style_comments( - state, - connection_id, - database, - schema, - table, - &ddl, - client_session_id, - pool_key.as_deref(), - ) - .await) + Ok(sanitize_ddl_string( + enrich_ddl_with_oracle_style_comments( + state, + connection_id, + database, + schema, + table, + &ddl, + client_session_id, + pool_key.as_deref(), + ) + .await, + )) } else { - Ok(ddl) + Ok(sanitize_ddl_string(ddl)) } } +pub(crate) fn sanitize_ddl_string(mut ddl: String) -> String { + if ddl.contains('\0') { + ddl.retain(|c| c != '\0'); + } + ddl +} + /// Whether the DDL of this object kind is its own definition, rather than the /// table DDL built from columns and indexes. /// @@ -16497,3 +16508,16 @@ fn render_sqlserver_table_ddl_details( } ddl } + +#[test] +fn sanitize_ddl_string_removes_nul_characters() { + assert_eq!( + sanitize_ddl_string("CREATE TABLE t (id INT DEFAULT 0\0\0);".to_string()), + "CREATE TABLE t (id INT DEFAULT 0);" + ); + assert_eq!( + sanitize_ddl_string("CREATE TABLE t (name VARCHAR DEFAULT ''\0\0);".to_string()), + "CREATE TABLE t (name VARCHAR DEFAULT '');" + ); + assert_eq!(sanitize_ddl_string("no nul chars".to_string()), "no nul chars"); +}