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.

Sign up for GitHub

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

Jump to bottom

Auth::newNid creates 40digit string, but mysql.sql has varchar(32) #37

Closed
iWhacko opened this issue Oct 5, 2020 · 2 comments · Fixed by #38
Closed

Auth::newNid creates 40digit string, but mysql.sql has varchar(32) #37

iWhacko opened this issue Oct 5, 2020 · 2 comments · Fixed by #38
Assignees
@tylerhall
Labels
bug

Comments

@iWhacko
Copy link

iWhacko commented Oct 5, 2020 •
edited

So, I rely heavily on generating nids for my objects, so that id'd are not guessable.
I ran into a weird problem that I could not lookup object from my database by their nid.
I have used this in the past too and never had a problem.

But now I found that I was passing the just generated nid to a function, to fetch the object I just saved. It uses the sha1() function which returns 40byte strings. However since the default mysql.sql had the column set as varchar(32) the nids were truncated to 32 bytes. Therefore all my queries returned false.

I'm not sure if this is an oversight and the database columns should be altered, or if something changed that I don't know about.
@iWhacko
Copy link
Author

iWhacko commented Oct 5, 2020

I think I have found the culprit. Instead of sha1, the newNid function previously used md5(), which is 32bytes. This explains why my other projects were working fine.
So I do think the database script should be updated. And maybe add documentation to the newNid() function on how many bytes it generates.

@tylerhall
Copy link
Owner

Ah, thanks for that. Completely and oversight on my part. I'll get the column length updated.

@tylerhall tylerhall self-assigned this Oct 5, 2020
@tylerhall tylerhall added the bug label Oct 5, 2020
@tylerhall tylerhall linked a pull request Oct 12, 2020 that will close this issue
Fixes #37 Increase size of users.nid column to varchar(40) #38
Merged
tylerhall added a commit that referenced this issue Oct 12, 2020
@tylerhall
Merge pull request #38 from tylerhall/fix-37
4ae734d 
Fixes #37 Increase size of users.nid column to varchar(40)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tylerhall tylerhall

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #37 Increase size of users.nid column to varchar(40) tylerhall/simple-php-framework
2 participants
@tylerhall @iWhacko