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

WorkspaceName is far too permissive #5125

Closed
bwaidelich opened this issue Jun 4, 2024 · 1 comment · Fixed by #5193
Closed

WorkspaceName is far too permissive #5125

bwaidelich opened this issue Jun 4, 2024 · 1 comment · Fixed by #5193
Labels

Comments

@bwaidelich
Copy link
Member

We use a lot of value objects with Neos 9.0. One of the reasons for that is a better type safety.
For example: Because we know, that NodeAggregateId can't contain certain characters, we can safely use them for delimiters (e.g. in Cache entry/tag identifiers).

The constraints for WorkspaceName are really loose currently: /^[\p{L}\p{P}\d \.]{1,200}$/u (basically 1 - 200 characters of any class).

We should reduce length (how many characters are allowed) and width (which characters are allowed) drastically IMO.
As a side effect we could decide to omit the SHA1 hashing for cache tags

@bwaidelich bwaidelich added the 9.0 label Jun 4, 2024
@mhsdesign
Copy link
Member

Also related, during the discussion of exposing the workspace name in the node we considered the idea of virtual workspace names:

WorkspaceName::fromString() must be stricter without colons (so that we could allow in the future for extension, e.g. WorkspaceName::detached(ContentStreamid $csId) (with an internal representation like “cs:”)

idk if this is still legit as the discussion is age old now and we only wanted to limit it to be able to implement future ideas

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

Successfully merging a pull request may close this issue.

2 participants