github-actions[bot] commented on code in PR #66690: URL: https://github.com/apache/doris/pull/66690#discussion_r3868398136
########## ui/src/main.tsx: ########## @@ -0,0 +1,55 @@ +// 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. + +import { QueryClientProvider } from '@tanstack/react-query'; +import { ConfigProvider } from 'antd'; +import React from 'react'; +import ReactDOM from 'react-dom/client'; +import { BrowserRouter } from 'react-router-dom'; + +import { AppRoutes } from './app/AppRoutes'; +import { queryClient } from './app/queryClient'; +import './styles/global.css'; + +const rootElement = document.getElementById('root'); + +if (!rootElement) { + throw new Error('The application root element is missing.'); +} + +ReactDOM.createRoot(rootElement).render( + <React.StrictMode> + <ConfigProvider + theme={{ + token: { + colorPrimary: '#0dbe85', + colorInfo: '#0dbe85', + colorText: '#1d2434', + borderRadius: 0, + controlHeight: 44, + fontFamily: '"IBM Plex Sans", "Noto Sans", Arial, sans-serif', + }, + }} + > + <QueryClientProvider client={queryClient}> + <BrowserRouter> Review Comment: Preserve the configured UI base path here. `http_api_extra_base_path` and `/api/basepath` are still supported, and the removed UI used the value for routing and requests; the replacement has no `BrowserRouter` basename, no Vite production `base`, and sends every `/rest`/`/api` request to the origin root. With FE mounted below a proxy prefix such as `/doris`, assets, navigation, login/bootstrap, and Web SQL all escape that prefix and hit the wrong upstream. Please restore one normalized base-path source across emitted assets, routing, redirects, and all clients, with a prefixed-production E2E case. ########## ui/package.json: ########## @@ -1,86 +1,52 @@ { - "name": "Doris", - "version": "1.0.0", - "description": "{{description}}", - "main": "index.js", - "scripts": { - "dev": "cross-env NODE_ENV=dev webpack-dev-server --progress --profile --process.env.PRODUCT_MODEL='DEVELOP' ", - "build": "cross-env NODE_ENV=prod webpack" + "name": "@apache-doris/ui", + "private": true, + "version": "0.1.0", + "type": "module", + "engines": { + "node": ">=22.12.0" Review Comment: Raising the engine floor here also requires updating the official ARM compilation image. `build.sh --fe` always runs `npm ci` and the Vite build, but `docker/compilation/arm/Dockerfile` still installs Node 16.3.0, below both this requirement and Vite 8's runtime floor. Consequently the supported aarch64 compilation path cannot build FE. Please upgrade that sibling image (the PR description currently says it was upgraded) and add an ARM FE-build smoke check. ########## fe/fe-core/src/main/java/org/apache/doris/httpv2/websql/WebSqlStatementExecutor.java: ########## @@ -0,0 +1,242 @@ +// 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.doris.httpv2.websql; + +import org.apache.doris.common.Config; + +import com.google.common.collect.Lists; + +import java.nio.charset.StandardCharsets; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; +import java.sql.SQLException; +import java.sql.SQLTimeoutException; +import java.sql.SQLWarning; +import java.sql.Statement; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.LongSupplier; + +/** Executes one validated statement on an existing Web SQL connection and builds a bounded JSON result. */ +public class WebSqlStatementExecutor { + private final LongSupplier maxResultBytesSupplier; + + public WebSqlStatementExecutor() { + this(() -> Config.web_sql_max_result_bytes); + } + + WebSqlStatementExecutor(LongSupplier maxResultBytesSupplier) { + this.maxResultBytesSupplier = maxResultBytesSupplier; + } + + public WebSqlExecutionResult execute(WebSqlSession session, String sql, WebSqlLimits limits) { + String validatedSql = SingleStatementValidator.requireSingleStatement(sql); + long maxResultBytes = currentMaxResultBytes(); + Connection connection = session.getConnection(); + long startTime = System.currentTimeMillis(); + QueryResult queryResult; + + try (Statement statement = connection.createStatement( + ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)) { + statement.setFetchSize(1000); + statement.setMaxRows(limits.maxResultRows + 1); + session.setActiveStatement(statement); + boolean hasResultSet = statement.execute(validatedSql); + if (hasResultSet) { + try (ResultSet resultSet = statement.getResultSet()) { + queryResult = readResultSet(resultSet, statement, connection, + limits.maxResultRows, maxResultBytes); + } + } else { + queryResult = new QueryResult(Collections.emptyList(), Collections.emptyList(), + Math.max(statement.getUpdateCount(), 0), false); + } + queryResult.warnings.addAll(readWarnings(statement)); + } catch (SQLTimeoutException exception) { + throw new WebSqlException(WebSqlError.QUERY_TIMEOUT, sqlDetails(exception), exception); + } catch (SQLException exception) { + throw new WebSqlException(WebSqlError.QUERY_ERROR, sqlDetails(exception), exception); + } finally { + session.setActiveStatement(null); + } + + SessionMetadata metadata = readSessionMetadata(connection); + return new WebSqlExecutionResult(queryResult.columns, queryResult.rows, queryResult.affectedRows, + System.currentTimeMillis() - startTime, metadata.queryId, queryResult.warnings, + metadata.catalog, metadata.database, queryResult.truncated); + } + + private QueryResult readResultSet(ResultSet resultSet, Statement statement, Connection connection, + int maxResultRows, long maxResultBytes) throws SQLException { + ResultSetMetaData metadata = resultSet.getMetaData(); + int columnCount = metadata.getColumnCount(); + List<WebSqlColumn> columns = Lists.newArrayListWithCapacity(columnCount); + for (int column = 1; column <= columnCount; column++) { + columns.add(new WebSqlColumn(metadata.getColumnName(column), metadata.getColumnTypeName(column))); + } + + List<List<Object>> rows = Lists.newArrayList(); + long resultBytes = 0; + boolean truncated = false; + while (resultSet.next()) { + if (rows.size() >= maxResultRows) { + truncated = true; + cancelAtResultLimit(statement, connection); + break; + } + List<Object> row = Lists.newArrayListWithCapacity(columnCount); + long rowBytes = 0; + for (int column = 1; column <= columnCount; column++) { + String type = metadata.getColumnTypeName(column); + Object value = isTextSerializedType(type) + ? resultSet.getString(column) : resultSet.getObject(column); + row.add(value); + rowBytes += valueSize(value); + } + if (resultBytes + rowBytes > maxResultBytes) { + truncated = true; + cancelAtResultLimit(statement, connection); + break; + } + rows.add(row); + resultBytes += rowBytes; + } + return new QueryResult(columns, rows, 0, truncated); + } + + private void cancelAtResultLimit(Statement statement, Connection connection) throws SQLException { + try { + statement.cancel(); + } catch (SQLException cancelException) { + try { + connection.close(); + } catch (SQLException closeException) { + cancelException.addSuppressed(closeException); + } + throw cancelException; + } + } + + long currentMaxResultBytes() { + long value = maxResultBytesSupplier.getAsLong(); + if (value <= 0 || value > Config.WEB_SQL_MAX_RESULT_BYTES_UPPER_BOUND) { + throw new IllegalStateException("Invalid web_sql_max_result_bytes: " + value); + } + return value; + } + + private boolean isDateType(String type) { + return "DATE".equalsIgnoreCase(type) || "DATETIME".equalsIgnoreCase(type) + || "DATEV2".equalsIgnoreCase(type) || "DATETIMEV2".equalsIgnoreCase(type); + } + + private boolean isTextSerializedType(String type) { + return isDateType(type) || "BIGINT".equalsIgnoreCase(type) || "LARGEINT".equalsIgnoreCase(type) + || type.regionMatches(true, 0, "DECIMAL", 0, "DECIMAL".length()); + } + + private long valueSize(Object value) { + return value == null ? 4 : String.valueOf(value).getBytes(StandardCharsets.UTF_8).length; Review Comment: This does not measure the value retained or serialized. MariaDB JDBC returns `byte[]` for binary values, for which `String.valueOf` is only a short `[B@...` identity while the full array stays in `rows` and Jackson emits it as Base64. A large VARBINARY cell therefore bypasses `web_sql_max_result_bytes` by orders of magnitude; JSON escaping also undercounts some strings. Count a normalized wire representation (or serialize through a bounded sink) and cover binary and escape-heavy results. ########## fe/fe-common/src/main/java/org/apache/doris/common/Config.java: ########## @@ -2785,6 +2790,49 @@ public class Config extends ConfigBase { + "SQL submitter.") public static int http_sql_submitter_max_worker_threads = 2; + @ConfField(mutable = true, masterOnly = false, + description = "Whether to enable stateful Web SQL HTTP sessions.") + public static boolean enable_web_sql_session = true; + + @ConfField(mutable = true, masterOnly = false, callback = PositiveWebSqlIntegerConfHandler.class, Review Comment: These callbacks run only for `setMutableConfig`; startup loading in `ConfigBase.setFields` calls `setConfigField` directly. Thus `fe.conf` can set non-positive session limits or an out-of-range result-byte limit even though dynamic updates reject them, leaving all creates expired/limited or all statements failing at runtime. Share the validation with startup loading and add initialization-boundary tests. ########## ui/src/pages/playground/useWebSqlSession.ts: ########## @@ -0,0 +1,236 @@ +// 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. + +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; + +import { + cancelWebSql, + closeWebSqlSession, + createWebSqlSession, + executeWebSql, + getWebSqlSession, + resetWebSqlSession, +} from '../../api/webSql'; +import { UiApiError } from '../../api/client'; +import type { WebSqlExecutionResult } from '../../api/types'; +import { + claimSessionForTab, + isClaimedByAnotherTab, + storedSessionId, + storeSessionId, +} from './sessionCoordinator'; + +export type WebSqlSessionStatus = 'connecting' | 'ready' | 'closed' | 'error'; + +const recoverableCodes = new Set([ + 'WEB_SQL_SESSION_NOT_FOUND', + 'WEB_SQL_SESSION_EXPIRED', + 'WEB_SQL_ACCESS_DENIED', +]); + +function isRecoverable(error: unknown): boolean { + return error instanceof UiApiError && recoverableCodes.has(error.code); +} + +export function useWebSqlSession() { + const [status, setStatus] = useState<WebSqlSessionStatus>('connecting'); + const [sessionId, setSessionId] = useState<string | null>(null); + const [error, setError] = useState<unknown>(null); + const sessionIdRef = useRef<string | null>(null); + const initializationRef = useRef<Promise<string> | null>(null); + const queueRef = useRef<Promise<unknown>>(Promise.resolve()); + const releaseClaimRef = useRef<() => void>(() => undefined); + const mountedRef = useRef(true); + const closingSessionIdsRef = useRef(new Set<string>()); + + const closeInBackground = useCallback((id: string, keepalive: boolean) => { + if (closingSessionIdsRef.current.has(id)) return; + closingSessionIdsRef.current.add(id); + void closeWebSqlSession(id, keepalive).catch(() => undefined); + }, []); + + const disposeOwnedSession = useCallback((keepalive: boolean) => { + const id = sessionIdRef.current; + sessionIdRef.current = null; + storeSessionId(null); + releaseClaimRef.current(); + releaseClaimRef.current = () => undefined; + if (id) closeInBackground(id, keepalive); + }, [closeInBackground]); + + const adoptSession = useCallback((id: string) => { + if (!mountedRef.current) { + closeInBackground(id, true); + return id; + } + releaseClaimRef.current(); + sessionIdRef.current = id; + storeSessionId(id); + releaseClaimRef.current = claimSessionForTab(id); + if (mountedRef.current) { + setSessionId(id); + setError(null); + setStatus('ready'); + } + return id; + }, [closeInBackground]); + + const createSession = useCallback(async () => { + if (initializationRef.current) return initializationRef.current; + const pending = createWebSqlSession() + .then((info) => adoptSession(info.sessionId)) + .finally(() => { + initializationRef.current = null; + }); + initializationRef.current = pending; + return pending; + }, [adoptSession]); + + useEffect(() => { + mountedRef.current = true; + const handlePageHide = (event: PageTransitionEvent) => { Review Comment: A normal reload dispatches `pagehide` with `persisted == false`, so this handler clears the stored ID and sends a keepalive DELETE. That makes the restore path below unreachable across reload and directly contradicts the new Playwright contract that expects SQL variables and the same session ID after `page.reload()`. Distinguish reload from permanent abandonment (or rely on bounded server expiry) so reload preserves the session, and test the real event sequence. ########## fe/fe-core/src/main/java/org/apache/doris/httpv2/websql/WebSqlSessionManager.java: ########## @@ -0,0 +1,424 @@ +// 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.doris.httpv2.websql; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.common.Config; +import org.apache.doris.common.ThreadPoolManager; + +import com.google.common.base.Strings; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.stereotype.Component; + +import java.security.SecureRandom; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Base64; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.function.LongSupplier; +import java.util.regex.Pattern; + +/** + * Manages bounded FE-local Web SQL sessions from creation through execution, reset, cancellation, and cleanup. + * Each registered session is owner-scoped and holds exactly one persistent JDBC connection. + */ +@Component +public class WebSqlSessionManager implements DisposableBean { + private static final Logger LOG = LogManager.getLogger(WebSqlSessionManager.class); + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + private static final Pattern SESSION_ID_PATTERN = Pattern.compile("[A-Za-z0-9_-]{8}\\.[A-Za-z0-9_-]{43}"); + + private final Map<String, WebSqlSession> sessions = new ConcurrentHashMap<>(); + private final Map<String, Integer> sessionsPerOwner = new HashMap<>(); + private final Cache<String, Boolean> expiredSessionIds = CacheBuilder.newBuilder() + .maximumSize(10000) + .expireAfterWrite(1, TimeUnit.HOURS) + .build(); + private final Object lifecycleLock = new Object(); + private int pendingSessions; + private volatile boolean destroyed; + private final WebSqlConnectionFactory connectionFactory; + private final WebSqlStatementExecutor statementExecutor; + private final WebSqlLimits limits; + private final LongSupplier clock; + private final boolean useRuntimeConfig; + private final String frontendHint; + private final ScheduledExecutorService cleaner; + + public WebSqlSessionManager() { + this(new JdbcWebSqlConnectionFactory(), new WebSqlStatementExecutor(), WebSqlLimits.fromConfig(), + System::currentTimeMillis, true, true); + } + + WebSqlSessionManager(WebSqlConnectionFactory connectionFactory, WebSqlStatementExecutor statementExecutor, + WebSqlLimits limits, LongSupplier clock, boolean startCleaner) { + this(connectionFactory, statementExecutor, limits, clock, startCleaner, false); + } + + private WebSqlSessionManager(WebSqlConnectionFactory connectionFactory, + WebSqlStatementExecutor statementExecutor, WebSqlLimits limits, LongSupplier clock, + boolean startCleaner, boolean useRuntimeConfig) { + this.connectionFactory = connectionFactory; + this.statementExecutor = statementExecutor; + this.limits = limits; + this.clock = clock; + this.useRuntimeConfig = useRuntimeConfig; + this.frontendHint = randomToken(6); + if (startCleaner && (useRuntimeConfig ? Config.enable_web_ui : limits.enabled)) { + cleaner = ThreadPoolManager.newDaemonScheduledThreadPool(1, "web-sql-session-cleaner", true); + cleaner.scheduleWithFixedDelay(this::cleanupExpiredSafely, limits.cleanupIntervalSeconds, + limits.cleanupIntervalSeconds, TimeUnit.SECONDS); + } else { + cleaner = null; + } + } + + public WebSqlSession createSession(String owner, String password) { + return createSession(owner, password, null, null); + } + + public WebSqlSession createSession(UserIdentity userIdentity, String password, String httpSessionId) { + return createSession(userIdentity.getQualifiedUser(), password, userIdentity, httpSessionId); + } + + private WebSqlSession createSession(String owner, String password, UserIdentity userIdentity, + String httpSessionId) { + requireEnabled(); + reserveSession(owner); + Connection connection; + try { + connection = userIdentity == null + ? connectionFactory.open(owner, password) + : connectionFactory.open(userIdentity, password); + } catch (SQLException exception) { + releaseReservation(owner); + throw connectionException(exception); + } + + String id = frontendHint + "." + randomToken(32); + WebSqlSession session = new WebSqlSession(id, owner, httpSessionId, connection, clock.getAsLong()); + boolean accepted; + synchronized (lifecycleLock) { + pendingSessions--; + accepted = !destroyed; Review Comment: Publication here only checks manager destruction, not whether this browser session logged out while `connectionFactory.open` ran outside the lock. `closeSessionsForHttpSession` snapshots only already-published entries, so logout can sweep nothing, invalidate the cookie session, and then this create publishes a live connection; the UI's later background DELETE is unauthenticated and swallowed, leaving the connection/quota until idle cleanup. Fence pending creates by HTTP session under `lifecycleLock` (or track retired sessions), reject/close when logout won, and add a latch-based race test. ########## ui/src/app/AuthGate.tsx: ########## @@ -0,0 +1,56 @@ +// 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. + +import { Alert, Spin } from 'antd'; +import { Navigate, useLocation } from 'react-router-dom'; + +import { UiApiError } from '../api/client'; +import { useMe } from '../api/me'; +import { AppShell } from './AppShell'; + +export function AuthGate() { + const location = useLocation(); + const me = useMe(); + + if (me.isPending) { + return ( + <main className="full-page-state" aria-label="Checking session"> + <Spin size="large" /> + <p>Checking your Doris session…</p> + </main> + ); + } + + if (me.error instanceof UiApiError && me.error.status === 401) { + return <Navigate to="/login" replace state={{ reason: 'expired', from: location.pathname }} />; Review Comment: Store the complete destination, not only `pathname`. `/system?path=%2Fbackends` uses the query as its selected proc directory, so an unauthenticated visit returns to `/system` and silently opens `/`; the AppShell 401 handler loses the entire current route. Preserve `pathname + search + hash` in both initial and expiry redirects and cover a query-bearing deep link. ########## fe/fe-core/src/main/java/org/apache/doris/httpv2/websql/JdbcWebSqlConnectionFactory.java: ########## @@ -0,0 +1,81 @@ +// 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.doris.httpv2.websql; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.common.Config; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; + +/** Opens Web SQL connections to the current FE's MySQL query port with the authenticated Doris user. */ +public class JdbcWebSqlConnectionFactory implements WebSqlConnectionFactory { + private static final String JDBC_DRIVER = "org.mariadb.jdbc.Driver"; + private static final int CONNECT_TIMEOUT_MILLIS = 10000; + private static final int SOCKET_TIMEOUT_MILLIS = 30 * 60 * 1000; + private static final String CURRENT_USER_SQL = "SELECT CURRENT_USER()"; + private static final String DB_URL_PATTERN = "jdbc:mariadb://127.0.0.1:%d/" + + "?connectTimeout=" + CONNECT_TIMEOUT_MILLIS + "&socketTimeout=" + SOCKET_TIMEOUT_MILLIS; + + @Override + public Connection open(String user, String password) throws SQLException { + try { + Class.forName(JDBC_DRIVER); + } catch (ClassNotFoundException exception) { + throw new SQLException("MariaDB JDBC driver is unavailable", exception); + } + return DriverManager.getConnection(connectionUrl(Config.query_port), user, password); + } + + @Override + public Connection open(UserIdentity userIdentity, String password) throws SQLException { + Connection connection = open(userIdentity.getQualifiedUser(), password); Review Comment: The HTTP path has already authenticated the exact `UserIdentity` using the browser's remote IP, but this call re-authenticates only the qualified username from `127.0.0.1`. A valid ADMIN such as `'alice'@'10.%'` therefore cannot open Web SQL: loopback either has no matching account or selects another host entry, which the following exact `CURRENT_USER()` check rejects. Preserve the authenticated identity through a trusted/in-process handoff instead of re-resolving it from loopback, and test a real host-scoped account. ########## ui/tests/e2e/scaffold.spec.ts: ########## @@ -0,0 +1,67 @@ +// 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. + +import { expect, test } from '@playwright/test'; + +test('guards protected routes and reports invalid credentials', async ({ page }) => { + await page.goto('/'); + await expect(page.getByRole('heading', { name: 'Sign in' })).toBeVisible(); + + await page.goto('/unknown-route'); + await expect(page.getByRole('heading', { name: 'Sign in' })).toBeVisible(); + + await page.getByLabel('Username').fill('root'); + await page.getByLabel('Password').fill('not-the-root-password'); + await page.getByRole('button', { name: 'Sign in' }).click(); + await expect(page.getByText('Sign-in failed. Check the username and password.')).toBeVisible(); +}); + +test('signs in, refreshes Home, inspects nodes, and signs out', async ({ page }) => { + await page.goto('/login'); + await page.getByLabel('Username').fill('root'); + await page.getByRole('button', { name: 'Sign in' }).click(); + + await expect(page).toHaveURL(/\/home$/); + await expect(page.getByRole('heading', { name: 'Home' })).toBeVisible(); Review Comment: This smoke test cannot pass against the UI in this same diff: HomePage renders the `Cluster Overview` heading, not `Home`, and node records use `.node-record-list`/`.node-record`, not `.node-table tr.node-row`. The other new E2E cleanups also parse `/rest/v1/ui/me` as an envelope and call nonexistent `/rest/v1/ui/logout` (and `/ui/log/verbose`) instead of the production endpoints. Align selectors and cleanup with the shipped UI and run the packaged-FE Playwright suite. ########## fe/fe-core/src/main/java/org/apache/doris/httpv2/websql/SingleStatementValidator.java: ########## @@ -0,0 +1,64 @@ +// 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.doris.httpv2.websql; + +import org.apache.doris.nereids.DorisLexer; +import org.apache.doris.nereids.DorisParser; +import org.apache.doris.nereids.DorisParser.MultiStatementsContext; +import org.apache.doris.nereids.parser.CaseInsensitiveStream; +import org.apache.doris.nereids.parser.NereidsParser; + +import org.antlr.v4.runtime.CharStreams; +import org.antlr.v4.runtime.CommonTokenStream; + +/** + * Uses Doris's own lexer and parser to require exactly one SQL statement per HTTP request. + * Parsing under both string-escape modes keeps the boundary safe even when the persistent + * JDBC session changed sql_mode in an earlier request. + */ +public final class SingleStatementValidator { + private SingleStatementValidator() { + } + + public static String requireSingleStatement(String sql) { + if (sql == null || sql.trim().isEmpty()) { + throw new WebSqlException(WebSqlError.INVALID_STATEMENT); + } + + requireSingleStatement(sql, false); + requireSingleStatement(sql, true); Review Comment: This rejects valid SQL by requiring it to parse under both mutually exclusive string-escape modes. Doris's own `NereidsParserTest.testNoBackSlashEscapes` proves literals that are valid in exactly one mode: a default session can submit a valid backslash-escaped apostrophe, yet the inactive `NO_BACKSLASH_ESCAPES` parse here rejects it before JDBC; after `SET sql_mode='NO_BACKSLASH_ESCAPES'`, the inverse class is rejected by the first parse. Validate against the persistent connection's actual SQL mode (and keep it synchronized after `SET`), or enforce the boundary at a mode-aware server/session layer, with positive tests in both directions. -- 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]
