Restrict group names to 32 graphemes.

Uses some code from #10132 hence co-author:

Co-authored-by: Fumiaki Yoshimatsu <fumiakiy@gmail.com>
fork-5.53.8
Alan Evans 2021-01-13 19:31:37 -04:00 zatwierdzone przez Greyson Parrelli
rodzic 0ccc7e3c06
commit 2d39e43677
8 zmienionych plików z 346 dodań i 15 usunięć

Wyświetl plik

@ -27,6 +27,7 @@ import com.bumptech.glide.request.target.CustomTarget;
import com.bumptech.glide.request.transition.Transition;
import com.dd.CircularProgressButton;
import org.signal.core.util.EditTextUtil;
import org.thoughtcrime.securesms.LoggingFragment;
import org.thoughtcrime.securesms.R;
import org.thoughtcrime.securesms.groups.ui.GroupMemberListView;
@ -104,6 +105,7 @@ public class AddGroupDetailsFragment extends LoggingFragment {
avatar.setOnClickListener(v -> showAvatarSelectionBottomSheet());
members.setRecipientClickListener(this::handleRecipientClick);
EditTextUtil.addGraphemeClusterLimitFilter(name, FeatureFlags.getMaxGroupNameGraphemeLength());
name.addTextChangedListener(new AfterTextChanged(editable -> viewModel.setName(editable.toString())));
toolbar.setNavigationOnClickListener(unused -> callback.onNavigationButtonPressed());
create.setOnClickListener(v -> handleCreateClicked());

Wyświetl plik

@ -27,6 +27,7 @@ import androidx.navigation.Navigation;
import com.bumptech.glide.load.engine.DiskCacheStrategy;
import com.dd.CircularProgressButton;
import org.signal.core.util.EditTextUtil;
import org.signal.core.util.StreamUtil;
import org.signal.core.util.logging.Log;
import org.thoughtcrime.securesms.LoggingFragment;
@ -63,7 +64,6 @@ public class EditProfileFragment extends LoggingFragment {
private static final String TAG = Log.tag(EditProfileFragment.class);
private static final short REQUEST_CODE_SELECT_AVATAR = 31726;
private static final int MAX_GROUP_NAME_LENGTH = 32;
private Toolbar toolbar;
private View title;
@ -213,15 +213,12 @@ public class EditProfileFragment extends LoggingFragment {
this.avatar.setOnClickListener(v -> startAvatarSelection());
this.givenName.addTextChangedListener(new AfterTextChanged(s -> {
trimInPlace(s, isEditingGroup);
viewModel.setGivenName(s.toString());
}));
view.findViewById(R.id.mms_group_hint)
.setVisibility(isEditingGroup && groupId.isMms() ? View.VISIBLE : View.GONE);
if (isEditingGroup) {
EditTextUtil.addGraphemeClusterLimitFilter(givenName, FeatureFlags.getMaxGroupNameGraphemeLength());
givenName.addTextChangedListener(new AfterTextChanged(s -> viewModel.setGivenName(s.toString())));
givenName.setHint(R.string.EditProfileFragment__group_name);
givenName.requestFocus();
toolbar.setTitle(R.string.EditProfileFragment__edit_group_name_and_photo);
@ -231,8 +228,12 @@ public class EditProfileFragment extends LoggingFragment {
view.findViewById(R.id.description_text).setVisibility(View.GONE);
view.<ImageView>findViewById(R.id.avatar_placeholder).setImageResource(R.drawable.ic_group_outline_40);
} else {
this.givenName.addTextChangedListener(new AfterTextChanged(s -> {
trimInPlace(s);
viewModel.setGivenName(s.toString());
}));
this.familyName.addTextChangedListener(new AfterTextChanged(s -> {
trimInPlace(s, false);
trimInPlace(s);
viewModel.setFamilyName(s.toString());
}));
LearnMoreTextView descriptionText = view.findViewById(R.id.description_text);
@ -335,7 +336,7 @@ public class EditProfileFragment extends LoggingFragment {
controller.onProfileNameUploadCompleted();
}
@RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)
@RequiresApi(api = 21)
private void handleFinishedLollipop() {
int[] finishButtonLocation = new int[2];
int[] revealLocation = new int[2];
@ -374,9 +375,8 @@ public class EditProfileFragment extends LoggingFragment {
animation.start();
}
private static void trimInPlace(Editable s, boolean isGroup) {
int trimmedLength = isGroup ? StringUtil.trimToFit(s.toString(), MAX_GROUP_NAME_LENGTH).length()
: StringUtil.trimToFit(s.toString(), ProfileName.MAX_PART_LENGTH).length();
private static void trimInPlace(Editable s) {
int trimmedLength = StringUtil.trimToFit(s.toString(), ProfileName.MAX_PART_LENGTH).length();
if (s.length() > trimmedLength) {
s.delete(trimmedLength, s.length());

Wyświetl plik

@ -14,7 +14,6 @@ import org.signal.core.util.logging.Log;
import org.thoughtcrime.securesms.BuildConfig;
import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
import org.thoughtcrime.securesms.groups.SelectionLimits;
import org.thoughtcrime.securesms.jobs.RefreshAttributesJob;
import org.thoughtcrime.securesms.jobs.RemoteConfigRefreshJob;
import org.thoughtcrime.securesms.keyvalue.SignalStore;
@ -53,6 +52,7 @@ public final class FeatureFlags {
private static final String USERNAMES = "android.usernames";
private static final String GROUPS_V2_RECOMMENDED_LIMIT = "global.groupsv2.maxGroupSize";
private static final String GROUPS_V2_HARD_LIMIT = "global.groupsv2.groupSizeHardLimit";
private static final String GROUP_NAME_MAX_LENGTH = "global.groupsv2.maxNameLength";
private static final String INTERNAL_USER = "android.internalUser";
private static final String VERIFY_V2 = "android.verifyV2";
private static final String PHONE_NUMBER_PRIVACY_VERSION = "android.phoneNumberPrivacyVersion";
@ -89,7 +89,8 @@ public final class FeatureFlags {
GROUP_CALLING,
SEND_VIEWED_RECEIPTS,
CUSTOM_VIDEO_MUXER,
CDS_REFRESH_INTERVAL
CDS_REFRESH_INTERVAL,
GROUP_NAME_MAX_LENGTH
);
@VisibleForTesting
@ -122,7 +123,8 @@ public final class FeatureFlags {
GROUP_CALLING,
GV1_MIGRATION_JOB,
CUSTOM_VIDEO_MUXER,
CDS_REFRESH_INTERVAL
CDS_REFRESH_INTERVAL,
GROUP_NAME_MAX_LENGTH
);
/**
@ -278,6 +280,11 @@ public final class FeatureFlags {
return getInteger(CDS_REFRESH_INTERVAL, (int) TimeUnit.HOURS.toSeconds(48));
}
/** The maximum number of grapheme */
public static int getMaxGroupNameGraphemeLength() {
return Math.max(32, getInteger(GROUP_NAME_MAX_LENGTH, -1));
}
/** Only for rendering debug info. */
public static synchronized @NonNull Map<String, Object> getMemoryValues() {
return new TreeMap<>(REMOTE_VALUES);

Wyświetl plik

@ -33,7 +33,6 @@
android:background="@null"
android:hint="@string/AddGroupDetailsFragment__group_name_required"
android:inputType="text"
android:maxLength="34"
android:maxLines="1"
app:layout_constraintBottom_toBottomOf="@id/group_avatar"
app:layout_constraintEnd_toEndOf="parent"

Wyświetl plik

@ -0,0 +1,146 @@
package org.signal.core.util;
import android.os.Build;
import androidx.annotation.NonNull;
import androidx.annotation.RequiresApi;
import java.util.Iterator;
public abstract class BreakIteratorCompat implements Iterable<CharSequence> {
public static final int DONE = -1;
private CharSequence charSequence;
public abstract int first();
public abstract int next();
public void setText(CharSequence charSequence) {
this.charSequence = charSequence;
}
public static BreakIteratorCompat getInstance() {
if (Build.VERSION.SDK_INT >= 24) {
return new AndroidIcuBreakIterator();
} else {
return new FallbackBreakIterator();
}
}
public int countBreaks() {
int breakCount = 0;
first();
while (next() != DONE) {
breakCount++;
}
return breakCount;
}
@Override
public @NonNull Iterator<CharSequence> iterator() {
return new Iterator<CharSequence>() {
int index1 = BreakIteratorCompat.this.first();
int index2 = BreakIteratorCompat.this.next();
@Override
public boolean hasNext() {
return index2 != DONE;
}
@Override
public CharSequence next() {
CharSequence c = index2 != DONE ? charSequence.subSequence(index1, index2) : "";
index1 = index2;
index2 = BreakIteratorCompat.this.next();
return c;
}
};
}
/**
* Take {@param atMost} graphemes from the start of string.
*/
public final CharSequence take(int atMost) {
if (atMost <= 0) return "";
StringBuilder stringBuilder = new StringBuilder(charSequence.length());
int count = 0;
for (CharSequence grapheme : this) {
stringBuilder.append(grapheme);
count++;
if (count >= atMost) break;
}
return stringBuilder.toString();
}
/**
* An BreakIteratorCompat implementation that delegates calls to `android.icu.text.BreakIterator`.
* This class handles grapheme clusters fine but requires Android API >= 24.
*/
@RequiresApi(24)
private static class AndroidIcuBreakIterator extends BreakIteratorCompat {
private final android.icu.text.BreakIterator breakIterator = android.icu.text.BreakIterator.getCharacterInstance();
@Override
public int first() {
return breakIterator.first();
}
@Override
public int next() {
return breakIterator.next();
}
@Override
public void setText(CharSequence charSequence) {
super.setText(charSequence);
if (Build.VERSION.SDK_INT >= 29) {
breakIterator.setText(charSequence);
} else {
breakIterator.setText(charSequence.toString());
}
}
}
/**
* An BreakIteratorCompat implementation that delegates calls to `java.text.BreakIterator`.
* This class may or may not handle grapheme clusters well depending on the underlying implementation.
* In the emulator, API 23 implements ICU version of the BreakIterator so that it handles grapheme
* clusters fine. But API 21 implements RuleBasedIterator which does not handle grapheme clusters.
* <p>
* If it doesn't handle grapheme clusters correctly, in most cases the combined characters are
* broken up into pieces when the code tries to trim a string. For example, an emoji that is
* a combination of a person, gender and skin tone, trimming the character using this class may result
* in trimming the parts of the character, e.g. a dark skin frowning woman emoji may result in
* a neutral skin frowning woman emoji.
*/
private static class FallbackBreakIterator extends BreakIteratorCompat {
private final java.text.BreakIterator breakIterator = java.text.BreakIterator.getCharacterInstance();
@Override
public int first() {
return breakIterator.first();
}
@Override
public int next() {
return breakIterator.next();
}
@Override
public void setText(CharSequence charSequence) {
super.setText(charSequence);
breakIterator.setText(charSequence.toString());
}
}
}

Wyświetl plik

@ -0,0 +1,20 @@
package org.signal.core.util;
import android.text.InputFilter;
import android.widget.EditText;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
public final class EditTextUtil {
private EditTextUtil() {
}
public static void addGraphemeClusterLimitFilter(EditText text, int maximumGraphemes) {
List<InputFilter> filters = new ArrayList<>(Arrays.asList(text.getFilters()));
filters.add(new GraphemeClusterLimitFilter(maximumGraphemes));
text.setFilters(filters.toArray(new InputFilter[0]));
}
}

Wyświetl plik

@ -0,0 +1,53 @@
package org.signal.core.util;
import android.text.InputFilter;
import android.text.Spanned;
import org.signal.core.util.logging.Log;
/**
* This filter will constrain edits not to make the number of character breaks of the text
* greater than the specified maximum.
* <p>
* This means it will limit to a maximum number of grapheme clusters.
*/
public final class GraphemeClusterLimitFilter implements InputFilter {
private static final String TAG = Log.tag(GraphemeClusterLimitFilter.class);
private final BreakIteratorCompat breakIteratorCompat;
private final int max;
public GraphemeClusterLimitFilter(int max) {
this.breakIteratorCompat = BreakIteratorCompat.getInstance();
this.max = max;
}
@Override
public CharSequence filter(CharSequence source, int start, int end, Spanned dest, int dstart, int dend) {
CharSequence sourceFragment = source.subSequence(start, end);
CharSequence head = dest.subSequence(0, dstart);
CharSequence tail = dest.subSequence(dend, dest.length());
breakIteratorCompat.setText(String.format("%s%s%s", head, sourceFragment, tail));
int length = breakIteratorCompat.countBreaks();
if (length > max) {
breakIteratorCompat.setText(sourceFragment);
int sourceLength = breakIteratorCompat.countBreaks();
CharSequence trimmedSource = breakIteratorCompat.take(sourceLength - (length - max));
breakIteratorCompat.setText(String.format("%s%s%s", head, trimmedSource, tail));
int newExpectedCount = breakIteratorCompat.countBreaks();
if (newExpectedCount > max) {
Log.w(TAG, "Failed to create string under the required length " + newExpectedCount);
return "";
}
return trimmedSource;
}
return source;
}
}

Wyświetl plik

@ -0,0 +1,104 @@
package org.signal.core.util;
import org.junit.Test;
import java.util.ArrayList;
import java.util.List;
import static java.util.Arrays.asList;
import static org.junit.Assert.assertEquals;
public final class BreakIteratorCompatTest {
@Test
public void empty() {
BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance();
breakIterator.setText("");
assertEquals(BreakIteratorCompat.DONE, breakIterator.next());
}
@Test
public void single() {
BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance();
breakIterator.setText("a");
assertEquals(1, breakIterator.next());
assertEquals(BreakIteratorCompat.DONE, breakIterator.next());
}
@Test
public void count_empty() {
BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance();
breakIterator.setText("");
assertEquals(0, breakIterator.countBreaks());
assertEquals(BreakIteratorCompat.DONE, breakIterator.next());
}
@Test
public void count_simple_text() {
BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance();
breakIterator.setText("abc");
assertEquals(3, breakIterator.countBreaks());
assertEquals(BreakIteratorCompat.DONE, breakIterator.next());
}
@Test
public void two_counts() {
BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance();
breakIterator.setText("abc");
assertEquals(3, breakIterator.countBreaks());
assertEquals(BreakIteratorCompat.DONE, breakIterator.next());
assertEquals(3, breakIterator.countBreaks());
}
@Test
public void count_multi_character_graphemes() {
String hindi = "समाजो गयेग";
BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance();
breakIterator.setText(hindi);
assertEquals(7, breakIterator.countBreaks());
assertEquals(BreakIteratorCompat.DONE, breakIterator.next());
}
@Test
public void iterate_multi_character_graphemes() {
String hindi = "समाजो गयेग";
BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance();
breakIterator.setText(hindi);
assertEquals(asList("स", "मा", "जो", " ", "ग", "ये", "ग"), toList(breakIterator));
assertEquals(BreakIteratorCompat.DONE, breakIterator.next());
}
@Test
public void split_multi_character_graphemes() {
String hindi = "समाजो गयेग";
BreakIteratorCompat breakIterator = BreakIteratorCompat.getInstance();
breakIterator.setText(hindi);
assertEquals("समाजो गयेग", breakIterator.take(8));
assertEquals("समाजो गयेग", breakIterator.take(7));
assertEquals("समाजो गये", breakIterator.take(6));
assertEquals("समाजो ग", breakIterator.take(5));
assertEquals("समाजो ", breakIterator.take(4));
assertEquals("समाजो", breakIterator.take(3));
assertEquals("समा", breakIterator.take(2));
assertEquals("स", breakIterator.take(1));
assertEquals("", breakIterator.take(0));
assertEquals("", breakIterator.take(-1));
}
private List<CharSequence> toList(BreakIteratorCompat breakIterator) {
List<CharSequence> list = new ArrayList<>();
breakIterator.forEach(list::add);
return list;
}
}