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

Fix: off-by-one random generation erro in hex_octet #109 #118

Merged
merged 2 commits into from
Oct 27, 2015

Conversation

juvenn
Copy link
Member

@juvenn juvenn commented Oct 27, 2015

The original hex_octet has probability 6% of generating
octet less-than-4 characters, which is not well-formed
to use in file key.

@see comments on #109.

The original hex_octet has probability 6% of generating
octet less-than-4 characters, which is not well-formed
to use in file key.

@see comments on leancloud#109.
@@ -133,7 +133,7 @@ def save(self):
base64.encode(self._source, output)
self._source.seek(0)
output.seek(0)
hex_octet = lambda: hex(int(1 + random.random() * 0x10000))[2:]
hex_octet = lambda: hex(int(0x10000 * (1 + random.random())))[2:]
Copy link
Member Author

Choose a reason for hiding this comment

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

请大家帮忙仔细审阅,上面的算法需要与下面一致:

g = (1 + random.random()) * 0x10000
h = hex(int(g))[2:]

我之前的PR #109 在这里犯了严重的错误。:(

@aisk
Copy link
Contributor

aisk commented Oct 27, 2015

好像还是有些问题,新的算法生成出来的都是五位字符,并且第一位总是为 1

@juvenn
Copy link
Member Author

juvenn commented Oct 27, 2015

hoo... 抱歉,确实有问题。

新的问题在于 python 中 hex 生成的 string 包括了 0x 前缀,结果应该是取最后的 4 位才对,而不是从第3 位开始取。

hex(int(0x10000 * (1 + random.random())))[-4:]

我再 push 一个 commit 上来。

@aisk
Copy link
Contributor

aisk commented Oct 27, 2015

我觉得应该没有问题了 👍

@juvenn
Copy link
Member Author

juvenn commented Oct 27, 2015

谢谢!

@aisk aisk closed this Oct 27, 2015
@aisk aisk reopened this Oct 27, 2015
aisk added a commit that referenced this pull request Oct 27, 2015
Fix: off-by-one random generation erro in hex_octet #109
@aisk aisk merged commit 724964a into leancloud:master Oct 27, 2015
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