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

Remove additional word character channel name restriction #2589

Merged
merged 1 commit into from Nov 25, 2016

Conversation

@Kissaki
Copy link
Member

commented Oct 22, 2016

The server has a regular expression setting for channel names.
The additional restriction of having a word character is probably
reasonable for most users.
However, it is an arbitrary limitation that users may want to circumvent.
For example: A channel could not be named with hyphens only "---".

As this is an arbitrary and invisible limitation, whichs use case is covered
by the channelname setting (with a default and adjustable by the user)
remove it.

@@ -863,12 +863,6 @@ void Server::msgChannelState(ServerUser *uSource, MumbleProto::ChannelState &msg
return;
}

QRegExp re2("\\w");
if (re2.indexIn(qsName) == -1) {

This comment has been minimized.

Copy link
@hacst

hacst Oct 23, 2016

Member

If we remove this we have no guarantee the user won't accidentally allow an empty string. Are we sure this won't break any of the code paths/database interaction in mumble or murmur?

This comment has been minimized.

Copy link
@mkrautz

mkrautz Oct 26, 2016

Member

I'm in favor of at least checking if its length is > 0.

Would also be interesting to check if these same rules are applied for RPC.

This comment has been minimized.

Copy link
@Kissaki

Kissaki Oct 26, 2016

Author Member

Nothing should interpret an empty channel string as something though, right?
Fair enough. I’ll add a 1 char requirement/check.

This comment has been minimized.

Copy link
@Kissaki

Kissaki Nov 2, 2016

Author Member

done

Remove most of channel name restriction
The server has a regular expression setting for channel names.
The additional restriction of having a word character is probably
reasonable for most users.
However, it is an arbitrary limitation that users may want to circumvent.
For example: A channel could not be named with hyphens only "---".

As this is an arbitrary and invisible limitation, whichs use case is
covered by the channelname setting (with a default, and adjustable by the
user) remove it.

We still prevent 0-length channel names as a special case, as they could
lead to issues in existing code paths (even when they should not with
consistently correct implementation).

@Kissaki Kissaki force-pushed the Kissaki:rem-channame-restriction branch from e82e69a to 628ddc3 Nov 2, 2016

@Kissaki

This comment has been minimized.

Copy link
Member Author

commented Nov 8, 2016

PTAL

@mkrautz

This comment has been minimized.

Copy link
Member

commented Nov 14, 2016

Still feeling a bit uneasy about this... For no good reason, maybe...

@mkrautz

This comment has been minimized.

Copy link
Member

commented Nov 25, 2016

OK, let's do it. People who dare touch the regexp should know what they're doing, and for the default regexp, this new check is equivalent to the old check.

@mkrautz mkrautz merged commit 0d76ff9 into mumble-voip:master Nov 25, 2016

@Kissaki Kissaki deleted the Kissaki:rem-channame-restriction branch Dec 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.