Skip to content

Conversation

@tylerhall
Copy link
Owner

Started from discussion here

e9f5242#commitcomment-139251599

by @cargilcm

tylerhall referenced this pull request Mar 1, 2024
…e shortened url (slug) is the same five characters s-t-a-t-s to access stats in the url route
Copy link

@cargilcm cargilcm left a comment

Choose a reason for hiding this comment

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

I approved your change with comments below

Copy link

@cargilcm cargilcm left a comment

Choose a reason for hiding this comment

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

Tylers issue #12 fix (d1a2c76) is mostly sufficient/is better than the one I proposed for a number of reasons.

To state the current status for the PR there are number of ways to arrive at a slug of stats being created and viewed.

I guess we could have tickets open for each(I'm not familiar with GitHub workflow, I just commit a few changes and hope they fix what the original author wants fixed and they merge it into the veritable project master).

For me, I would use $pw* in the config file set to non empty string to create a slug, view slug stats

If $pw_stats is non empty i don't see an issue with the app route http://domain/stats/yourpassword
even if the links table has a stats slug entered.

We could change create link to do something in the event all (26^5 )-1 combinations of slugs are in use; but I think this is just an administrative action for the owner to change slug length eventually increasing it.

Or there could be a notice on the read me about how the application handles cases of conflict with slug names/app routes. Again, the conflict only is realizable if pw stats isn't set.

As for other slug routes being guarded I think if the $pw_* contains any special characters outside of $random_chars the routes remain accessible.

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