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

上传文件时,可以添加校验参数强制校验key和hash 音频元信息的接口 #365

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ramwin
Copy link

@ramwin ramwin commented Apr 26, 2020

上传文件时,可以添加校验参数强制校验key和hash
音频元信息的接口

@bachue
Copy link
Contributor

bachue commented Jul 9, 2021

上传的时候已经进行了校验检查了,不能通过返回值里的 key 和 hash 做检查,这两个参数可以被修改,不一定会返回

@ramwin
Copy link
Author

ramwin commented Jul 9, 2021

上传的时候已经进行了校验检查了,不能通过返回值里的 key 和 hash 做检查,这两个参数可以被修改,不一定会返回

能给出源码链接吗?我看到的源码里好像没有检查,出错了并不会报错。

@ramwin
Copy link
Author

ramwin commented Jul 9, 2021

上传的时候已经进行了校验检查了,不能通过返回值里的 key 和 hash 做检查,这两个参数可以被修改,不一定会返回

另外不能通过key和hash做检查也说不通啊,官网文档列子就是用key和hash做检查的。如果真的会被修改,那我们源码里修改就可以,用户反而不用考虑,直接传raise_exception=True就可以,多方便啊。

@ramwin
Copy link
Author

ramwin commented Mar 27, 2024

这个有人看下回复一下吗

@lihsai0
Copy link
Collaborator

lihsai0 commented Mar 27, 2024

@ramwin

SDK 内不能通过返回值里的 key 和 hash 做检查

原因是 hash 会由于上传方式的不同而变化。SDK 内的 etag 并不完全覆盖所有情况。即存在文件上传正确但 etag 计算出却不一致的情况。

上传的时候已经进行了校验检查了。

能给出源码链接吗?我看到的源码里好像没有检查,出错了并不会报错。

对于表单上传的时候会计算 crc32 并传递给服务端,如果服务端收到的文件计算 crc32 与收到的 crc32 不一致,服务端会返回非 200。

Details

crc = file_crc32(file_path)
ret, info = _form_put(
up_token, key, input_stream, params, mime_type,
crc, hostscache_dir, progress_handler, file_name,
modify_time=modify_time, keep_last_modified=keep_last_modified, metadata=metadata
)

if not crc32_int:
crc32_int = self.__get_crc32_int(data)
fields = self.__get_form_fields(
up_hosts=up_hosts,
up_token=up_token,
key=key,
crc32_int=crc32_int,
custom_vars=custom_vars,
metadata=metadata
)
ret, resp = self.__upload_data(
up_hosts=up_hosts,
fields=fields,
file_name=file_name,
data=data,
data_size=data_size,
mime_type=mime_type
)

对于分片上传 v1,SDK 会对服务端返回的 crc32 与本地计算结果比对,如果不一致返回结果为 None 与不一致的那个分片上传的请求响应。

Details

chunk_crc32 = io_crc32(chunked_data)
chunked_data.seek(0)
part, resp = None, None
for up_host in up_hosts:
url = '/'.join([
up_host,
'mkblk', str(chunk_info.chunk_size)
])
ret, resp = qn_http_client.post(
url=url,
data=chunked_data,
files=None,
headers={
'Authorization': 'UpToken {}'.format(up_token)
}
)
if resp.ok() and ret:
if ret.get('crc32', 0) != chunk_crc32:
return None, resp

对于分片上传 v2,每一个分片都会计算 md5 并传递给服务端,如果服务端收到的内容计算 md5 与收到的 md5 不一致,服务端会返回非 200。

Details

chunk_md5 = io_md5(chunked_data)
chunked_data.seek(0)
part, resp = None, None
for up_host in up_hosts:
url = self.__get_url_for_upload(
up_host,
bucket_name,
key,
upload_id=upload_id,
part_no=chunk_info.chunk_no
)
ret, resp = qn_http_client.put(
url=url,
data=chunked_data,
files=None,
headers={
'Content-Type': 'application/octet-stream',
'Content-MD5': chunk_md5,
'Authorization': 'UpToken {}'.format(up_token)
}
)


综上所述,在上传过程已保证数据完整性,如果出错,不会返回 key 与 hash 字段。当前已经不推荐使用 SDK 内的 etag 去做校验。

您看是否已经解答您的疑惑?

@ramwin
Copy link
Author

ramwin commented Mar 27, 2024

我加这个参数的意义在于, 希望本来要用3行代码完成的事:

ret, resp = auth.put_file(...)
if "key" not in ret:
    raise ValueError

变成1行

ret, resp = auth.put_file(..., raise_exception=True)

服务端会返回非 200, key不存在于ret. 这些都不会报错啊. 还是要在外面加判断

@lihsai0
Copy link
Collaborator

lihsai0 commented Mar 27, 2024

@ramwin 还是不太明白痛点在哪里。raise 的错误最后也是要处理的。

ret, resp = put_file(...)
if "key" not in ret:
    handle_upload_failed()

try:
    ret, resp = put_file(..., raise_exception=True)
except:
    handle_upload_failed()

两种区别不大。

@ramwin
Copy link
Author

ramwin commented Mar 28, 2024

@ramwin 还是不太明白痛点在哪里。raise 的错误最后也是要处理的。

ret, resp = put_file(...)
if "key" not in ret:
    handle_upload_failed()

try:
    ret, resp = put_file(..., raise_exception=True)
except:
    handle_upload_failed()

两种区别不大。

我们用sdk时, 不会 单独为了这一行代码做错误处理. 而是在外层框架上, 遇到报错时统一处理. 所以不会在内部函数里再加try except了

@lihsai0
Copy link
Collaborator

lihsai0 commented Mar 28, 2024

可以理解为您想要一套 exception 的机制吗?这个我们后面考虑下。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants