search

tompave / fun_with_flags_ui

Web dashboard for the FunWithFlags Elixir package
MIT License
128 stars 29 forks source link

Support for slash in group name #36

Open bmteller opened 1 year ago

bmteller commented 1 year ago

Encode slash as %2F. If group name contained a / character then this would be interpreted by phoenix router as a different path segment. I'm not sure if this works across all browsers or network setups because maybe some browsers will unescape values in the path before submitting the path or middle boxes might rewrite the path to an unescaped form.

I'm also not sure what other characters would cause the functionality to break.

Alternatively, the UI should ban you from creating flags with characters it does not support. However, I'm not sure that this is realistic because the non-ui part of the project does not put any restriction on the characters that can be contained in a name.

tompave commented 1 year ago

Thank you for using the package and the PR.

I'm not sure I can accept this, as I'd like to keep this part of the package simple. I'm just afraid that once I start allowing for these exceptions, it becomes a slippery slope.

For reference, I documented this sort of limitation in the readme: https://github.com/tompave/fun_with_flags_ui#caveats

Why do you want to use / in group names?

bmteller commented 1 year ago

in that case do you think it makes sense to update def validate in utils.ex to reject the / character like it already rejects the ? character for group names. these characters are already blocked from the flag name but not the group name. https://github.com/tompave/fun_with_flags_ui/blob/master/lib/fun_with_flags/ui/utils.ex#L161

tompave commented 1 year ago

Yes, that would make sense. I'd be happy to accept a PR for that.