github-actions[bot] commented on code in PR #69462:
URL: https://github.com/apache/doris/pull/69462#discussion_r4357285059
##########
fe/fe-core/src/test/java/org/apache/doris/qe/StmtExecutorTest.java:
##########
@@ -1292,9 +1305,144 @@ public void
testEmptyOriginStmtSkipsAuditMaskingReparse() throws Exception {
getStmtForLogging.setAccessible(true);
Assertions.assertEquals("", getStmtForLogging.invoke(executor, ""));
- Method getStmtForLoggingBeforeParse =
StmtExecutor.class.getDeclaredMethod("getStmtForLoggingBeforeParse");
- getStmtForLoggingBeforeParse.setAccessible(true);
- Assertions.assertEquals("",
getStmtForLoggingBeforeParse.invoke(executor));
+ Method getCurrentStmtForLogging =
StmtExecutor.class.getDeclaredMethod("getStmtForLogging");
+ getCurrentStmtForLogging.setAccessible(true);
+ Assertions.assertEquals("", getCurrentStmtForLogging.invoke(executor));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testParsedRoutineLoadLogsMaskBothSqlModes(boolean
originalNoBackslashMode) throws Exception {
+ String sql = "ALTER ROUTINE LOAD FOR db.job FROM KAFKA "
+ + "('property.sasl.\\password' = 'synthetic-value')";
+ long originalMode = connectContext.getSessionVariable().getSqlMode();
+ boolean originalPrintRequest =
Config.enable_print_request_before_execution;
+ try {
+
connectContext.getSessionVariable().setSqlMode(originalNoBackslashMode
+ ? SqlModeHelper.MODE_NO_BACKSLASH_ESCAPES :
SqlModeHelper.MODE_DEFAULT);
+ AlterRoutineLoadCommand command = Mockito.spy(
+ (AlterRoutineLoadCommand) new
NereidsParser().parseSingle(sql));
+ // Keep real execution and its INFO/DEBUG logs; only skip the
Kafka job mutation.
+ Mockito.doNothing().when(command).run(Mockito.any(),
Mockito.any());
+ LogicalPlanAdapter statement = new LogicalPlanAdapter(command, new
StatementContext());
+ statement.setOrigStmt(new OriginStatement(sql, 0));
+ // An earlier SET in a pre-parsed batch changes the mode before
this command executes.
+
connectContext.getSessionVariable().setSqlMode(originalNoBackslashMode
+ ? SqlModeHelper.MODE_DEFAULT :
SqlModeHelper.MODE_NO_BACKSLASH_ESCAPES);
+ Config.enable_print_request_before_execution = true;
+ connectContext.getState().reset();
+ try (TestLogAppender appender =
TestLogAppender.attach(StmtExecutor.class)) {
+ StmtExecutor executor = new StmtExecutor(connectContext,
statement);
+ executor.execute();
+
+ String expected = sql.replace("'synthetic-value'", "'******'");
+
Assertions.assertTrue(appender.contains(org.apache.logging.log4j.Level.INFO,
expected));
+
Assertions.assertTrue(appender.contains(org.apache.logging.log4j.Level.DEBUG,
expected));
+
Assertions.assertFalse(appender.contains(org.apache.logging.log4j.Level.INFO,
"synthetic-value"));
+
Assertions.assertFalse(appender.contains(org.apache.logging.log4j.Level.DEBUG,
"synthetic-value"));
+ Mockito.verify(command).run(connectContext, executor);
+ }
+ } finally {
+ Config.enable_print_request_before_execution =
originalPrintRequest;
+ connectContext.getSessionVariable().setSqlMode(originalMode);
+ }
+ }
+
+ @Test
+ public void testSelectStartupLogsFailClosedOnBatchSplitMismatch() throws
Exception {
+ String sql = "SELECT 1; /* outer /* inner */ x; */ ALTER ROUTINE LOAD
FOR db.job FROM KAFKA "
+ + "('property.sasl.password' = 'synthetic-secret')";
+ List<StatementBase> statements = new NereidsParser().parseSQL(sql);
+ Assertions.assertEquals(2, statements.size());
+ Assertions.assertEquals(3, SqlUtils.splitMultiStmts(sql).size());
+ // ConnectProcessor retains the complete request when its splitter
disagrees with Nereids.
+ StatementBase select = statements.get(0);
+ select.setOrigStmt(new OriginStatement(sql, 0));
+ boolean originalPrintRequest =
Config.enable_print_request_before_execution;
+ Config.enable_print_request_before_execution = true;
+ connectContext.getState().reset();
+ try (TestLogAppender appender =
TestLogAppender.attach(StmtExecutor.class)) {
+ StmtExecutor executor = new StmtExecutor(connectContext, select);
+ executor.execute();
+
+
Assertions.assertTrue(appender.contains(org.apache.logging.log4j.Level.INFO,
MASKED_STMT_FALLBACK));
+
Assertions.assertTrue(appender.contains(org.apache.logging.log4j.Level.DEBUG,
MASKED_STMT_FALLBACK));
+
Assertions.assertFalse(appender.contains(org.apache.logging.log4j.Level.INFO,
"synthetic-secret"));
+
Assertions.assertFalse(appender.contains(org.apache.logging.log4j.Level.DEBUG,
"synthetic-secret"));
+ } finally {
+ Config.enable_print_request_before_execution =
originalPrintRequest;
+ }
+ }
+
+ @Test
+ public void testForwardedRoutineLoadAuditMasksMasterKeyInterpretation()
throws Exception {
+ String alter = "ALTER ROUTINE LOAD FOR testDb.job FROM KAFKA "
+ + "('property.sasl.\\password' = 'synthetic-secret',
'property.client.id' = 'client-value')";
+ String batch = "SET sql_mode=''; " + alter;
+ ConnectContext follower = Mockito.spy(new ConnectContext());
+ follower.setCurrentUserIdentity(UserIdentity.ROOT);
+ follower.setEnv(Env.getCurrentEnv());
+ follower.setRemoteIP("127.0.0.1");
+ follower.setStartTime();
+
follower.getSessionVariable().setSqlMode(SqlModeHelper.MODE_NO_BACKSLASH_ESCAPES);
+ follower.setThreadLocalInfo();
+ Mockito.doReturn(null).when(follower).getCurrentCatalog();
+ try {
+ // Parse the whole batch before running its SET, as
ConnectProcessor does.
+ List<StatementBase> statements = new
NereidsParser().parseSQL(batch);
+ LogicalPlanAdapter followerStatement = (LogicalPlanAdapter)
statements.get(1);
+ followerStatement.setOrigStmt(new OriginStatement(alter, 0));
+ AlterRoutineLoadCommand followerCommand =
(AlterRoutineLoadCommand) followerStatement.getLogicalPlan();
+ Assertions.assertEquals("synthetic-secret",
+
followerCommand.getDataSourceMapProperties().get("property.sasl.\\password"));
+ LogicalPlanAdapter setStatement = (LogicalPlanAdapter)
statements.get(0);
+ follower.setStatementContext(setStatement.getStatementContext());
+ ((SetOptionsCommand) setStatement.getLogicalPlan()).run(follower,
null);
+ Assertions.assertEquals(SqlModeHelper.MODE_DEFAULT,
follower.getSessionVariable().getSqlMode());
Review Comment:
[P2] Expect zero after clearing sql_mode in the forwarding test
The preceding `SET sql_mode=''` is executed by the real `SetOptionsCommand`,
whose `VariableMgr` converter calls `SqlModeHelper.encode("")` and stores `0L`.
`MODE_DEFAULT` is `1L`, and neither the field assignment nor callbacks
normalize zero to that value, so this assertion fails before the request
serialization and audit checks run. Expect `0L` (or assert that
`NO_BACKSLASH_ESCAPES` is disabled) so the new regression reaches the behavior
it is meant to cover.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
