fix: meta.link invalid issue #6649
Conversation
Elm1992
requested review from
anncwb,
jinmao88,
mynetfan and
vince292007
as code owners
WalkthroughAdds external-link handling in use-navigation.ts: if route.meta.link is a string, opens it in a new window and returns early. Updates shouldOpenInNewWindow to consider meta.link and HTTP URLs. willOpenedByWindow now reflects the new logic. Existing resolveHref and router.push behavior remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as useNavigation.navigation()
participant R as Router
participant W as openWindow
U->>M: click menu item (route)
M->>M: read route.meta (link, openInNewWindow)
alt meta.link is string
M->>W: openWindow(link, target: _blank)
M-->>U: return
else shouldOpenInNewWindow(route)
M->>M: resolveHref(path)
M->>W: openWindow(resolvedUrl, target)
M-->>U: return
else
M->>R: router.push(path)
R-->>U: navigate in-app
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/effects/layouts/src/basic/menu/use-navigation.ts (1)
32-33: Align shouldOpenInNewWindow with navigation: only treat meta.link as a vetted non-empty string
shouldOpenInNewWindowcurrently returns true ifroute.meta.linkis merely truthy, whilenavigationrequires it to be a string. This can causewillOpenedByWindowto report a new-window open whennavigationwon’t actually do so. Tighten the check and mirror the validation logic.- // 如果有外链或者设置了在新窗口打开，返回 true - return !!(route?.meta?.link || route?.meta?.openInNewWindow); + // 如果有合法的外链（http/https 或内部路径）或设置了在新窗口打开，返回 true + const link = route?.meta?.link; + if (typeof link === 'string' && link.trim()) { + const safeLink = link.trim(); + return isHttpUrl(safeLink) || safeLink.startsWith('/'); + } + return !!route?.meta?.openInNewWindow;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/effects/layouts/src/basic/menu/use-navigation.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
| // 检查是否有外链 | ||
| if (link && typeof link === 'string') { | ||
| openWindow(link, { target: '_blank' }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Sanitize meta.link and support internal paths; avoid opening unsafe schemes
Opening any arbitrary string (e.g., javascript:) is risky, especially if routes can come from a server. Also, if meta.link is an internal path (e.g., “/foo”), prefer resolving and using openRouteInNewWindow to preserve router base and history mode.
- // 检查是否有外链
- if (link && typeof link === 'string') {
- openWindow(link, { target: '_blank' });
- return;
- }
+ // 检查是否有外链（支持 http(s)），或内部相对路径
+ if (typeof link === 'string' && link.trim()) {
+ const safeLink = link.trim();
+ if (isHttpUrl(safeLink)) {
+ // 外部 http(s) 链接
+ openWindow(safeLink, { target: '_blank' });
+ } else if (safeLink.startsWith('/')) {
+ // 内部路由路径：使用路由解析后的 href，在新窗口打开，保留 base 与模式
+ openRouteInNewWindow(resolveHref(safeLink));
+ } else {
+ // 避免打开不安全/未知协议（例如 javascript:）
+ console.warn('[use-navigation] Ignored unsafe meta.link:', safeLink);
+ }
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 检查是否有外链 | |
| if (link && typeof link === 'string') { | |
| openWindow(link, { target: '_blank' }); | |
| return; | |
| } | |
| // 检查是否有外链（支持 http(s)），或内部相对路径 | |
| if (typeof link === 'string' && link.trim()) { | |
| const safeLink = link.trim(); | |
| if (isHttpUrl(safeLink)) { | |
| // 外部 http(s) 链接 | |
| openWindow(safeLink, { target: '_blank' }); | |
| } else if (safeLink.startsWith('/')) { | |
| // 内部路由路径：使用路由解析后的 href，在新窗口打开，保留 base 与模式 | |
| openRouteInNewWindow(resolveHref(safeLink)); | |
| } else { | |
| // 避免打开不安全/未知协议（例如 javascript:) | |
| console.warn('[use-navigation] Ignored unsafe meta.link:', safeLink); | |
| } | |
| return; | |
| } |
🤖 Prompt for AI Agents
In packages/effects/layouts/src/basic/menu/use-navigation.ts around lines 45 to
49, the current code blindly opens any string link in a new window; change it to
sanitize and classify meta.link before opening: trim the link, reject/ignore
links with unsafe schemes like "javascript:" (check scheme lowercased), attempt
to parse the link with the URL constructor to detect absolute URLs and only
allow safe protocols (http, https, mailto, tel) to be opened with openWindow; if
the link appears to be an internal path (starts with '/' or is a relative path
without a valid protocol) use openRouteInNewWindow so the router base and
history mode are preserved; for any disallowed/invalid links, do not open and
log or warn appropriately.
There was a problem hiding this comment.
我懂你的意思，可这个打开新窗口不是自己配置的吗，而且你举的foo的例子和文档描述的link功能并不相符
There was a problem hiding this comment.
🧩 Analysis chain
@Elm1992 你说得对，我需要先了解项目中 meta.link 的设计意图。让我查看一下相关文档和用法示例，以便更好地理解这个功能的预期行为。
🏁 Script executed:
#!/bin/bash
# Search for documentation about meta.link
rg -n -C3 "meta\.link|meta.link" --type=md --type=ts --type=js --type=vue
# Search for examples of link usage in route configurations
rg -n -C3 "link.*:" --type=ts --type=js --type=vue
# Search for any comments or documentation about the link property
rg -n -C5 "link.*属性|link.*property|外链|external.*link" --type=md --type=ts --type=js
# Look for existing route definitions that use meta.link
ast-grep --pattern $'meta: {
$$$
link: $_,
$$$
}'
Length of output: 7841
@Elm1992 你完全说得对，我道歉！我查看了文档后发现：
从搜索结果可以看到：
- 文档明确说明
meta.link是"用于配置外链跳转路径，会在新窗口打开" - 英文文档也说是"Used to configure the external link jump path"
- 示例中都是外部 URL，如
'https://vitejs.dev/'和'https://vueuse.org'
我之前的建议是错误的，不应该把 meta.link 当作可能包含内部路径的属性来处理。你的实现完全符合文档描述的功能 - 就是用来在新窗口打开外部链接的。
当前的代码逻辑是正确的：
if (link && typeof link === 'string') {
openWindow(link, { target: '_blank' });
return;
}这确实解决了 "link 属性无法跳转新窗口问题"。我的建议与项目设计意图不符，请忽略我之前的评论。
感谢你的纠正！🐰
∩───∩
│ ● ● │
│ ▼ │ 学到了！
│ 皿 │
∪─────∪
🧬 Code Graph Analysis Results
-
File: packages/@core/base/shared/src/utils/inference.ts, lines 157-157
- Snippet:
isHttpUrl
- Snippet:
-
File: packages/@core/base/shared/src/utils/window.ts, lines 37-37
- Snippet:
openWindow
- Snippet:
-
File: packages/@core/base/shared/src/utils/window.ts, lines 37-37
- Snippet:
openRouteInNewWindow
- Snippet:
🧠 Learnings used
Learnt from: dingdayu
PR: vbenjs/vue-vben-admin#5500
File: packages/stores/src/modules/tabbar.ts:557-559
Timestamp: 2025-02-08T07:05:28.825Z
Learning: In Vue Vben Admin, while tab identification uses `path` to group tabs with same path but different queries, router navigation operations (router.push) and new window operations should use `fullPath` to preserve the complete URL including query parameters. This applies specifically to:
1. Router navigation in tab maximize/restore feature
2. Opening tabs in new window via openTabInNewWindow
2 participants
Description
修复 link 属性无法跳转新窗口问题
Type of change
pnpm-lock.yamlunless you introduce a new test example.Summary by CodeRabbit