feat:add analyze platform return #2229
Conversation
Lotterng
commented
Sep 18, 2025
Lotterng
commented
- analyze.ts 仅为通过代码检查的实例文件，并非真的实现
- platform-share
- 新增url拼接函数，以初始化不同平台的分享url
- 新增url解析函数，放到了initShareInfo中，因为需要使用解释函数的网页，通过会调用initShareInfo的，所以将两个操作统一在一个方法中
Code Review CompleteI've analyzed PR #2229 which adds analyze.ts functionality and enhances platform sharing. Here's my comprehensive review: Todo List:
Review Findings🔍 High Priority Issues1. Security Vulnerability - Unvalidated URL Parameters
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Lotterng, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
此拉取请求引入了跟踪和分析项目分享来源平台的功能。它通过修改分享机制，在分享 URL 中嵌入平台信息，并在页面加载时解析此信息，然后向后端发送分析请求，从而实现对分享回流的分析。
Highlights
- 新增分析API: 新增
analyze.ts文件，其中包含analyzePlatformShare函数，用于向后端发送平台分享分析请求。 - 分享URL拼接: 在
platform-share.ts中新增initShareURL函数，用于在分享 URL 中拼接平台信息，以便后端进行回流分析。 - 分享信息初始化逻辑更新: 修改
platform-share.ts中的initShareInfo函数，使其在初始化分享信息时，首先解析 URL 中的平台信息（如果存在），然后调用analyzePlatformShare发送分析请求，并更新 QQ 和微信平台的分享链接以使用新的initShareURL。 - URL平台解析: 在
platform-share.ts中新增analyzeProjectShareUrl函数，用于从当前 URL 中解析出分享平台信息。
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
spx-gui/src/apis/analyze.ts
Outdated
| export function analyzePlatformShare(platform: string) { | ||
| return fetch(`/api/analyze/platform-share?platform=${platform}`) | ||
| } |
There was a problem hiding this comment.
这个函数返回的 fetch Promise 没有处理潜在的错误（如网络问题）。调用方 initShareInfo 中使用了 await，如果此 Promise 被拒绝，将导致未处理的 Promise 拒绝，这可能会影响应用的正常运行。对于分析类型的请求，通常建议在内部处理错误，避免将错误抛给上层业务逻辑。
| export function analyzePlatformShare(platform: string) { | |
| return fetch(`/api/analyze/platform-share?platform=${platform}`) | |
| } | |
| export function analyzePlatformShare(platform: string) { | |
| return fetch(`/api/analyze/platform-share?platform=${platform}`).catch((error) => { | |
| // 仅在控制台记录错误，不影响主流程 | |
| console.error('Failed to analyze platform share:', error) | |
| }) | |
| } |
![niupilot[bot]](https://avatars.githubusercontent.com/in/1894735?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Found several critical security and code quality issues that should be addressed:
• Security vulnerabilities: Unvalidated URL parameters and potential injection risks
• Missing error handling: API calls lack proper error handling
• Code quality issues: Hardcoded values, typos, and inconsistent async handling
The overall architecture is well-designed, but security and error handling need immediate attention.
spx-gui/src/apis/analyze.ts
Outdated
| @@ -0,0 +1,3 @@ | |||
| export function analyzePlatformShare(platform: string) { | |||
| return fetch(`/api/analyze/platform-share?platform=${platform}`) | |||
There was a problem hiding this comment.
Add proper error handling and URL encoding for security. The original code lacks error handling for network failures and doesn't encode the platform parameter, which could lead to malformed URLs.
| return fetch(`/api/analyze/platform-share?platform=${platform}`) | |
| return fetch(`/api/analyze/platform-share?platform=${encodeURIComponent(platform)}`) | |
| .then(response => { | |
| if (!response.ok) { | |
| throw new Error(`HTTP error! status: ${response.status}`); | |
| } | |
| return response; | |
| }) | |
| .catch(error => { | |
| console.error('Failed to analyze platform share:', error); | |
| throw error; | |
| }); |
| export function analyzeProjectShareUrl(): string | null { | ||
| const currentUrl = location.href | ||
| if (currentUrl.includes('#/share-to-platform=')) { | ||
| return currentUrl.split('#/share-to-platform=')[1] |
There was a problem hiding this comment.
Add input validation using regex to prevent injection attacks. The original code directly splits the URL without validation, making it vulnerable to malicious input.
| return currentUrl.split('#/share-to-platform=')[1] | |
| const match = currentUrl.match(/#\/share-to-platform=([a-zA-Z0-9_-]+)$/); | |
| return match ? match[1] : null; |
|
|
||
| // 初始化分享URL，拼接对应平台，以便后端进行回流分析 | ||
| function initShareURL(platform: string) { | ||
| return location.href + '#/share-to-platform=' + platform |
There was a problem hiding this comment.
Add platform validation and URL encoding for security. The original function doesn't validate the platform parameter, allowing arbitrary values to be injected into URLs.
| return location.href + '#/share-to-platform=' + platform | |
| function initShareURL(platform: string) { | |
| // Validate platform parameter | |
| const validPlatforms = ['qq', 'wechat', 'douyin', 'xiaohongshu', 'bilibili']; | |
| if (!validPlatforms.includes(platform)) { | |
| throw new Error(`Invalid platform: ${platform}`); | |
| } | |
| return location.href + '#/share-to-platform=' + encodeURIComponent(platform); | |
| } |
| await analyzePlatformShare(platform) | ||
| } | ||
|
|
||
| const defaultShareInfo = shareInfo || { title: 'XBulider', desc: 'XBuilder分享你的创意作品' } |
There was a problem hiding this comment.
Fix typo in application name. 'XBulider' should be 'XBuilder' for consistency.
| const defaultShareInfo = shareInfo || { title: 'XBulider', desc: 'XBuilder分享你的创意作品' } | |
| const defaultShareInfo = shareInfo || { title: 'XBuilder', desc: 'XBuilder分享你的创意作品' }; |
| const platform = analyzeProjectShareUrl() | ||
| if (platform) { | ||
| await analyzePlatformShare(platform) | ||
| } |
There was a problem hiding this comment.
Add error handling for analytics API call. The original code doesn't handle potential failures from analyzePlatformShare, which could break the entire sharing flow.
| const platform = analyzeProjectShareUrl() | |
| if (platform) { | |
| await analyzePlatformShare(platform) | |
| } | |
| // 判断是否从某一平台分享的回流 | |
| const platform = analyzeProjectShareUrl(); | |
| if (platform) { | |
| try { | |
| await analyzePlatformShare(platform); | |
| } catch (error) { | |
| console.error('Failed to analyze platform share:', error); | |
| // Continue execution even if analytics fail | |
| } | |
| } |
spx-gui/src/apis/analyze.ts
Outdated
| @@ -0,0 +1,3 @@ | |||
| export function analyzePlatformShare(platform: string) { | |||
| return fetch(`/api/analyze/platform-share?platform=${platform}`) | |||
There was a problem hiding this comment.
这个是真实实现还是只是占位用的？如果是占位用的，不用真的 fetch，留一行注释 TODO 就好；如果是真实实现，我们发请求都是走公共的 client 的而不是直接调用 fetch
| /** | ||
| * 平台跳转链接的前缀 | ||
| */ | ||
| const hashPrefix = '#/share-to-platform=' |
There was a problem hiding this comment.
为什么用 hash 而不是 query（search）？
There was a problem hiding this comment.
考虑query（search）可能会导致页面刷新或者对路由有影响，这里因为功能比较单一hash不会影响页面路由
There was a problem hiding this comment.
考虑query（search）可能会导致页面刷新或者对路由有影响
这个没太理解，你应该不会直接把当前页面重定向到拼接后的 URL，那为啥会导致页面刷新呢
There was a problem hiding this comment.
这里确实不会存在刷新这个问题，只是觉得这个功能比较单一，hash实现起来比较简单
There was a problem hiding this comment.
hash 没有更简单，如果我们还用 hash 来做其他的功能（比如常见的用于在页面中标记锚点），那么这里跟你的 share-to-platform 就冲突了；而如果用 query search 的话，把 share-to-platform 跟其他 query param 视作不同的 param 就好了，它们是可以共存的，类似 ?xxx=1&share-to-platform
| } | ||
|
|
||
| // 初始化分享URL，拼接对应平台，以便后端进行回流分析 | ||
| function initShareURL(platform: string) { |
There was a problem hiding this comment.
一般这种比较基础的功能实现不应该去假设拿来拼接的 url 一定是当前页面 URL（location.href），在调用的地方（上下文、需求足够明确后）再去决定是不是用 location.href
举个例子，如果我们在 project 页面上分享 recording，这时会需要拿来拼接（用于分享）的 url 应该是 recording 页面的 url 而不是 project 页面的
下边的 analyzeProjectShareUrl 同理
There was a problem hiding this comment.
老师，我仔细思考发现这里其实不用传url，即使在project页面分享的是recording的页面，扫瞄到手机上就是recording页面，因为recordind页面需要做分享，所以是会进行initShareInfo的使用的，这里就会重新根据当前recording页面的路由进行配置了，而不是在project页面去配置recording页面的initshareurl
There was a problem hiding this comment.
即使在project页面分享的是recording的页面，扫瞄到手机上就是recording页面
这里手机扫描的二维码对应的是 recording 页面的 URL，你预期这个 URL 上会带分享参数吗？
There was a problem hiding this comment.
携带参数这个动作，发生在用户扫描完二维码，再qq和微信等平台内置浏览器打开recording页面时生效，并不再project的逻辑中，我将在recording添加相关配置代码
There was a problem hiding this comment.
不用追求构造跟解析的执行发生在一同一个函数中，只要它们的逻辑维护在靠近的地方就好了；相关的逻辑维护在靠近的地方可以降低维护成本，而执行在同一个函数中（同一个时间）并没有什么意义
There was a problem hiding this comment.
明白，那很明确的需求是在分享recording页面的时候，会需要在sharevideo里将recording的url携带上参数，然后手机扫描这个携带参数的url转换成的二维码，进入到对应页面，直接使用这个url进行配置，就不用再重复携带一遍参数了，那这里需要判定一下url是否携带参数。这里会引入两个新的问题就是：1.之前我是通过别的用户分享点进来的是有参数的，我再分享肯定要删除就参数加上新参数，怎么区分这个逻辑和recording参数的逻辑，能否再加一个firstshare=ture来区分2.用户进入recording页面的时候，由于已经携带了参数，会被解析函数给分析成回流的数据，但这个分享其实还没有发生，是否还要在url再加一个参数做判断，或者问题一和二的参数统一成一个
There was a problem hiding this comment.
那很明确的需求是在分享recording页面的时候，会需要在sharevideo里将recording的url携带上参数，然后手机扫描这个携带参数的url转换成的二维码，进入到对应页面，直接使用这个url进行配置，就不用再重复携带一遍参数了
是的
1.之前我是通过别的用户分享点进来的是有参数的，我再分享肯定要删除就参数加上新参数
不用，如果我们定义“ProjectDirectSharing、ProjectScreenshotSharing、ProjectRecordingSharing 这样的地方才是不同平台的分享动作公共的起始点，也是我们这次做的“分享功能”的起点”，那么某个人在手机上再分享给其他人的时候就不是在发起一次新的分享，而是延续原来的那次分享；因此用同一个参数（traffic-source ID）即可
这个可以看成一次（我们定义的）分享行为包含了多次事实上的传播，这个是正常的；比如你分享到其他平台（如抖音），也完全可能只在我们平台上操作一次（对应发布一次视频），然后那个视频会被多次转发、传播（可能是在抖音 APP 内）；这种情况显然我们也做不到要求后续抖音 APP 内的每次转发都对应一个新的 traffic source id。
2.用户进入recording页面的时候，由于已经携带了参数，会被解析函数给分析成回流的数据，但这个分享其实还没有发生，是否还要在url再加一个参数做判断
嗯这个可以加个参数来做区分，来排除这种明显不合理的数据
spx-gui/src/apis/analyze.ts
Outdated
| @@ -0,0 +1,3 @@ | |||
| export function analyzePlatformShare(platform: string) { | |||
There was a problem hiding this comment.
这个函数名不太对吧，这里应该只是上报（收集）数据，分析是更晚才会做的动作
There was a problem hiding this comment.
不过老师我这里也可以明确成 uploadPlatformData 如何？然后同步给聪哥
| const url = new URL(location.href) | ||
| url.searchParams.set(TRAFFIC_SOURCE_QUERY_KEY, String(trafficSource.id)) | ||
| return url.toString() | ||
| // 如果传入了 url，额外标记当前为“分享初始化”而非“分享完成”，避免被解析为回流 |
There was a problem hiding this comment.
这里“是否传入了 url”直觉上的语义应该是：是否使用一个跟当前页面 URL 不同的 URL 用于分享
为什么可以通过这个来判断是否需要标记“分享初始化”？如果是拿当前页面 URL 用于分享（比如 project 页面直接分享），就没有忽略手机端第一次打开的诉求了吗？
另外按理说手机端第一次打开后，后续分享时应该要把 URL 中的这个“初次打开”的标记去掉（否则后续真正的被分享然后打开的行为也会被忽略了？），为啥没看到去掉标记的逻辑
There was a problem hiding this comment.
如果是拿当前页面 URL 用于分享
这里更多的还是考虑以pc端的三种分享方式为分享起始点，而不是用户在微信和qq中的内置浏览器打开为起始点是吗？那如果在电脑端的qq或者复制别人分享的链接到来pc端，是不是应该算是一轮新的分享，所以只有点击直接分享和录屏分享我们才会携带参数，如果这样的话，携带参数的工作就可以不用放在project中，而是放在directShare和recordingShare里了
后续分享时应该要把 URL 中的这个“初次打开”的标记去掉
这个我后续加上
There was a problem hiding this comment.
这里更多的还是考虑以pc端的三种分享方式为分享起始点，而不是用户在微信和qq中的内置浏览器打开为起始点是吗？
是的，我以为这个我们已经达成一致了？
那如果在电脑端的qq或者复制别人分享的链接到来pc端，是不是应该算是一轮新的分享
如果定义“一轮分享”对应一个独立的 traffic-source-id，那么这应该不算；只有在我们的页面上，点击我们提供的分享按钮才算一次分享的开始
所以只有点击直接分享和录屏分享我们才会携带参数
截图分享也会涉及到 URL，那个也应该带参数
如果这样的话，携带参数的工作就可以不用放在project中，而是放在directShare和recordingShare里了
“project”是指 project 页面吗？为什么拼接参数的逻辑可以放在 project 页面？project 页面上并不知道最后要被分享出去的 URL 是什么页面的 URL（比如 directShare 分享出去的是 project 页面的 URL，而 recordingShare 分享出去的是 recording 页面的 URL）
There was a problem hiding this comment.
是的，我以为这个我们已经达成一致了？
直接分享我这里还是用的之前的逻辑 我修改一下
“project”是指 project 页面吗？
是的，这里是因为解析和初始化douzaiproject里，拼接也放在这里了，我调整一下
|
This PR has been deployed to the preview environment. You can explore it using the preview URL. Warning Please note that deployments in the preview environment are temporary and will be automatically cleaned up after a certain period. Make sure to explore it before it is removed. For any questions, contact the XBuilder team. |
| ipAddress: string | ||
| accessCount: number | ||
| } | ||
| export function createTrafficSource(platform: string, signal?: AbortSignal): Promise<TrafficSourceData> { |
There was a problem hiding this comment.
应该直接用隔壁的 apis/traffic-source.ts，而不是再建这么个文件？
| const qrData = await generateQRCodeDataUrl(currentShareURL, platform.basicInfo.color) | ||
| qrCodeData.value = qrData | ||
| shareURL = await initShareURL(platform.basicInfo.name, projectSharingLink.value) | ||
| shareURL = await addShareStateURL(shareURL) |
There was a problem hiding this comment.
可以再细想一下 addShareStateURL 应该谁来调用；我们加它是因为走微信、QQ 分享时第一次的打开不是被分享的人，但这是微信、QQ 的逻辑而不是所有平台的逻辑，如果有某个平台的分享不需要先在手机端打开这个页面，而是直接扫码后会构造一条消息并发送给其他人（就像抖音扫码后是直接构造一条视频发布一样），那么这个平台应该就是不用 addShareStateURL 的
从这个角度看，可能 addShareStateURL 做到微信、QQ 这俩 platform 的 shareURL 里会更合适
