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

Replaces short-id by nano-id #2130

Merged
merged 1 commit into from
May 21, 2024
Merged

Conversation

marijnvandevoorde
Copy link
Contributor

Hey there,

According to this: https://github.com/dylang/shortid?tab=readme-ov-file, shortid has been deprecated as it's deemed unsafe. Although the PHP version looks to be better, it relies on yet other libraries and is pretty big for what it does.

NanoId is a lot simpler and smaller, so I'd suggest to switch. This should do the trick.

@acelaya
Copy link
Member

acelaya commented May 21, 2024

Thanks!

Yeah, I have checked this library for a replacement a couple of times in the past, but never really found anything good enough.

This one looks like a pretty straightforward replacement if it works as it should.

Copy link
Member

@acelaya acelaya left a comment

Choose a reason for hiding this comment

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

Thanks!

I gave this a quick test and it works quite well. Benchmarks also show this is much faster, although we are talking about ms ranges.

The failing step in the pipeline is not caused by anything introduced here, so I'll merge this and fix it after.

@acelaya acelaya merged commit e2d8334 into shlinkio:develop May 21, 2024
20 of 21 checks passed
@acelaya acelaya added this to the 4.1.1 milestone May 21, 2024
@marijnvandevoorde marijnvandevoorde deleted the nanoid branch May 21, 2024 18:43
@marijnvandevoorde
Copy link
Contributor Author

Thanks Alejandro, much appreciated. And indeed, it's not so much the speed but the security aspect that we care about! Thanks!

acelaya added a commit to acelaya-forks/shlink that referenced this pull request May 23, 2024
@acelaya
Copy link
Member

acelaya commented May 23, 2024

I have just released Shlink 4.1.1 which includes this.

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

Successfully merging this pull request may close these issues.

2 participants