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

getItem($key) - shouldn't you be trimming whitespace from the key as well as forward slashes? #329

Open
exts opened this issue Oct 24, 2016 · 4 comments

Comments

@exts
Copy link

exts commented Oct 24, 2016

eg.: trim($key, '/ ') Line: 107

Because you can't trust the developer to do that every time when the library is expecting a certain type of string.

$key = " some/key"; would be considered an malformed string going through that method, though it's not always easily caught.

@design1online
Copy link

I agree with spaces but not forward slashes.

@exts
Copy link
Author

exts commented Oct 13, 2017

@design1online the reason I said forward slashes because of how Stash deals with forward slashes and automatically grouping arrays. There's really no reason for a cache key to start or end with a forward slash as well.

I've never seen someone write keys like this:

  • user/1/
  • /user/1/
  • /user/1

There's no reason to write cache keys in stash like that at all. If you can give me a use case i'd be glad for you to give me a use case where it's appropriate to write any cache keys like that especially when the next line is an explode:

$key = explode('/', $keyString);

@design1online
Copy link

I use keys like that because I create keys based on the classname.

@exts
Copy link
Author

exts commented Oct 16, 2017

@design1online even when you use them based off class names, there's no reason to have forward slashes starting or ending in the key string.

namespace/subnamespace/classname

Anyways my concern with this issue is because Stash does not do checks for empty strings coming from forward slashes which causes problems when they explode a forward slash string and the key is empty.

//stash/should/break here

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

No branches or pull requests

2 participants