fix: update image URL construction to use new protocol with rkey #1
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the image URL construction logic to use a new protocol with rkey parameter, migrating from the legacy qpic.cn endpoint to the multimedia.nt.qq.com.cn endpoint. The changes ensure proper handling of rkey values and file IDs for image download URLs.
- Updates URL construction to use the multimedia.nt.qq.com.cn endpoint with proper query parameters
- Adds rkey sanitization to handle values that may be prefixed with "&rkey="
- Consolidates the URL format across both friend and group image elements
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // This is the new protocol case, which contains an rkey. | ||
| rKeyRaw := img.PbReserve.Url.Unwrap() | ||
| // Sanitize the rkey value, which often comes in the format "&rkey=...". | ||
| rKey := strings.TrimPrefix(rKeyRaw, "&rkey=") |
There was a problem hiding this comment.
The rkey sanitization logic assumes the prefix is always "&rkey=" but doesn't handle cases where the rkey might have different formats or no prefix at all. Consider adding validation to ensure the trimmed value is not empty and handle edge cases more robustly.
| rKey := strings.TrimPrefix(rKeyRaw, "&rkey=") | |
| // Robustly extract the rkey value, handling various formats. | |
| var rKey string | |
| // If rKeyRaw contains query parameters, parse them. | |
| if strings.Contains(rKeyRaw, "rkey=") { | |
| // Remove any leading '&' or '?' for ParseQuery compatibility. | |
| clean := strings.TrimLeft(rKeyRaw, "&?") | |
| values, err := url.ParseQuery(clean) | |
| if err == nil { | |
| rKey = values.Get("rkey") | |
| } | |
| } else { | |
| // Fallback: use the raw value if it looks like an rkey. | |
| rKey = rKeyRaw | |
| } | |
| // Validate that rKey is not empty. | |
| if rKey == "" { | |
| // Optionally, log or handle the missing rkey case. | |
| rKey = "invalid" | |
| } |
Copilot uses AI. Check for mistakes.
| // FilePath serves as the fileid. | ||
| fileID := img.FilePath.Unwrap() | ||
| // Construct the correct URL using the reliable multimedia endpoint. | ||
| url = fmt.Sprintf("https://multimedia.nt.qq.com.cn/download?appid=1406&fileid=%s&rkey=%s", fileID, rKey) |
There was a problem hiding this comment.
The hardcoded appid value '1406' appears multiple times in the code. Consider extracting this as a constant to improve maintainability and make it easier to update if the appid changes.
| url = fmt.Sprintf("https://multimedia.nt.qq.com.cn/download?appid=1406&fileid=%s&rkey=%s", fileID, rKey) | |
| url = fmt.Sprintf("https://multimedia.nt.qq.com.cn/download?appid=%d&fileid=%s&rkey=%s", AppID, fileID, rKey) |
Copilot uses AI. Check for mistakes.
| rKey := img.Elem2.Data.Friend.RKey.Unwrap() | ||
| url := fmt.Sprintf("https://%s%s%s&spec=0&rf=naio", domain, imgURL, rKey) | ||
| rKeyRaw := img.Elem2.Data.Friend.RKey.Unwrap() | ||
| rKey := strings.TrimPrefix(rKeyRaw, "&rkey=") |
There was a problem hiding this comment.
This is duplicated sanitization logic from line 598. Consider extracting this into a helper function to avoid code duplication and ensure consistent rkey processing across the codebase.
| rKey := strings.TrimPrefix(rKeyRaw, "&rkey=") | |
| rKey := sanitizeRKey(rKeyRaw) |
Copilot uses AI. Check for mistakes.
idea comes from https://github.com/LagrangeDev/LagrangeGo/blame/8479db44a25b5f33f461ed9721c333059111c243/client/operation.go#L568