Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add download file API #696

Merged
merged 3 commits into from
Sep 12, 2024
Merged

Add download file API #696

merged 3 commits into from
Sep 12, 2024

Conversation

feiniks
Copy link
Contributor

@feiniks feiniks commented Sep 10, 2024

No description provided.

if len(parts) < 3 {
msg := "Invalid URL\n"
return &appError{nil, msg, http.StatusBadRequest}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个检查应该是没有必要的吧,go router 里面已经匹配过了,不匹配的是不会进来的。


decPath, err := url.PathUnescape(filePath)
if err != nil {
err := fmt.Errorf("failed to unescape file path %s: %v", filePath, err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不能解码参数应该是一个 400 错误。

}

token := utils.GetAuthorizationToken(r.Header)
cookie := r.Header.Get("Cookie")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果两个认真信息都没有设置,应该返回一个错误。

return "", &appError{err, "", http.StatusInternalServerError}
}
if status != http.StatusOK {
msg := "Access token not found"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里应该返回 No permission to access file. C 的代码里面也改一下。

User string `json:"user"`
}

func queryAccessToken(repoID, token, cookie, filePath, op string) (string, *appError) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不要用 access token 这个概念,因为会和原来代码里面的 access token 概念混淆。应该叫 checkFileAccess。上面的数据结构就叫 UserInfo 就行了。C 代码也相应改一下概念。


object = json_loadb (rsp_content, rsp_size, 0, &jerror);
if (!object) {
seaf_warning ("Parse response failed: %s.\n", jerror.text);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不应该叫 Parse response failed,应该用一个更明确的消息,比如 Failed to parse response when check file access in Seahub.


user = json_object_get_string_member (object, "user");
if (!user) {
seaf_warning ("Failed to find user in json.\n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个消息也要改一下。

g_free (cookie_header);
}
headers = curl_slist_append (headers, "Content-Type: application/json");
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

上面这些头部的设置应该都可以放在 http_post_common 里面的,http_get_common 也相应改一下。除了 Cookie 那个是特殊的。

if (rsp_status != HTTP_OK) {
ret = -1;
seaf_warning ("Failed to get check access token %s from seahub %s: %d.\n",
token, rsp_content, rsp_status);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里意味着权限错误,不需要打印错误日志。

seaf_warning ("Failed to get check access token %s from seahub %s: %d.\n",
token, rsp_content, rsp_status);
goto out;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

上面两个问题我看在获取 nickname 和获取链接信息的代码也有,都需要优化一下。

@killing killing merged commit b2bde11 into master Sep 12, 2024
6 checks passed
@killing killing deleted the download_files branch September 12, 2024 03:02
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