Skip to content

fix: skip ODC query path in static file middleware (#815)#616

Closed
LordofAvernus wants to merge 1 commit into
mainfrom
dms/feat-815
Closed

fix: skip ODC query path in static file middleware (#815)#616
LordofAvernus wants to merge 1 commit into
mainfrom
dms/feat-815

Conversation

@LordofAvernus
Copy link
Copy Markdown
Collaborator

@LordofAvernus LordofAvernus commented May 13, 2026

User description

Skip ODC query path in static file middleware to prevent fallback interception, ensuring Provision TDSQL data source sync works correctly through the DMS proxy layer.

Related Issue: https://github.com/actiontech/dms-ee/issues/815


Description

  • 新增对 SqlWorkbenchService.GetRootUri() 判断

  • 修正静态文件中间件拦截 ODC 查询路径问题

  • 确保代理层正确转发请求


Diagram Walkthrough

flowchart LR
  A["检测 Cloudbeaver 请求"] -- "已有判断" --> B["跳过静态中间件"]
  A2["检测 SqlWorkbench 请求"] -- "新增加判断" --> B
  C["检测 provision/sqle 路径"] -- "已有判断" --> B
Loading

File Walkthrough

Relevant files
Bug fix
router.go
在静态中间件中添加 ODC 查询跳过逻辑                                                                         

internal/apiserver/service/router.go

  • 添加判断逻辑,跳过 SqlWorkbenchService.GetRootUri()
  • 修正静态中间件对 ODC 查询路径的拦截
+3/-0     

@actiontech-bot actiontech-bot requested a review from BugsGuru May 13, 2026 07:55
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

PR Reviewer Guide 🔍

(Review updated until commit 20c113e)

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 PR contains tests
🔒 Security concerns

⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

PR Code Suggestions ✨

Latest suggestions up to 20c113e
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
避免空字符串匹配

建议在调用 GetRootUri() 后先验证返回结果是否为空,以防因空字符串导致所有请求均匹配,从而跳过中间件。添加非空判断可以避免错误的静态文件路径跳过问题。

internal/apiserver/service/router.go [439-441]

-if strings.HasPrefix(c.Request().URL.Path, s.SqlWorkbenchController.SqlWorkbenchService.GetRootUri()) {
+rootUri := s.SqlWorkbenchController.SqlWorkbenchService.GetRootUri()
+if rootUri != "" && strings.HasPrefix(c.Request().URL.Path, rootUri) {
     return true
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion adds a non-empty check to the result of GetRootUri(), guarding against unintended matches from an empty string which could skip middleware processing. The change is contextually appropriate and enhances safety with moderate impact.

Medium

Previous suggestions

Suggestions up to commit 51861dc
CategorySuggestion                                                                                                                                    Impact
Possible issue
增加 nil 指针检查

建议在调用 GetRootUri() 方法之前,先检查 s.SqlWorkbenchController
s.SqlWorkbenchController.SqlWorkbenchService 是否为 nil,避免可能的 nil 指针调用导致
panic。确保对这些变量进行 nil 检查可以提高代码的健壮性和稳定性。

internal/apiserver/service/router.go [439-441]

-if strings.HasPrefix(c.Request().URL.Path, s.SqlWorkbenchController.SqlWorkbenchService.GetRootUri()) {
+if s.SqlWorkbenchController != nil && s.SqlWorkbenchController.SqlWorkbenchService != nil && strings.HasPrefix(c.Request().URL.Path, s.SqlWorkbenchController.SqlWorkbenchService.GetRootUri()) {
 	return true
 }
Suggestion importance[1-10]: 8

__

Why: The suggestion enhances code robustness by adding nil checks to avoid potential panic from nil pointer dereferences when calling GetRootUri(). It correctly identifies the issue in the diff and offers an appropriate improvement.

Medium

…k interception (#815)

The static file middleware was intercepting /odc_query/* requests and
serving the DMS index.html instead of letting the proxy middleware
forward them to the ODC backend. Added SqlWorkbenchService.GetRootUri()
to the static middleware skipper, matching the existing pattern for
CloudBeaver.
@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 20c113e

@BugsGuru BugsGuru closed this May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants