Reject fractional integer sliders

Amolith created 3 days ago

Make integer slider eligibility reject fractional bounds, current
values, or options before choosing the slider UI. Those forms now use
the numeric text input instead of relying on slider rounding.

Change summary

src/main/java/eu/siacs/conversations/entities/Conversation.java     | 29 
src/test/java/eu/siacs/conversations/entities/ConversationTest.java | 38 
2 files changed, 61 insertions(+), 6 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/entities/Conversation.java

@@ -1875,21 +1875,26 @@ public class Conversation extends AbstractEntity
         if (!isNumericDatatype(datatype)) return null;
 
         final Element range = validate.findChild("range", "http://jabber.org/protocol/xdata-validate");
-        final Float min = rangeFloat(range, "min");
-        final Float max = rangeFloat(range, "max");
+        final String minValue = range == null ? null : range.getAttribute("min");
+        final String maxValue = range == null ? null : range.getAttribute("max");
+        final boolean integerDatatype = isIntegerDatatype(datatype);
+        if (integerDatatype && (!isIntegerLexicalValue(minValue) || !isIntegerLexicalValue(maxValue))) return null;
+        final Float min = parseFloat(minValue);
+        final Float max = parseFloat(maxValue);
         if (min == null || max == null || min >= max) return null;
 
         final String value = firstValue(field);
         if (value == null || value.equals("")) return null;
+        if (integerDatatype && !isIntegerLexicalValue(value)) return null;
         final Float parsedValue = parseFloat(value);
         if (parsedValue == null || parsedValue < min || parsedValue > max) return null;
 
-        final List<Float> options = optionValues(field);
+        final List<Float> options = optionValues(field, integerDatatype);
         if (options == null) return null;
 
         final Float step;
         if (options.size() < 2) {
-            step = isIntegerDatatype(datatype) ? 1f : 0f;
+            step = integerDatatype ? 1f : 0f;
         } else {
             step = uniformStep(options);
             if (step == null) return null;
@@ -1923,10 +1928,12 @@ public class Conversation extends AbstractEntity
         return step;
     }
 
-    private static List<Float> optionValues(final Element field) {
+    private static List<Float> optionValues(final Element field, final boolean integerDatatype) {
         final List<Float> values = new ArrayList<>();
         for (final Option option : Option.forField(field)) {
-            final Float value = parseFloat(option.getValue());
+            final String optionValue = option.getValue();
+            if (integerDatatype && !isIntegerLexicalValue(optionValue)) return null;
+            final Float value = parseFloat(optionValue);
             if (value == null) return null;
             values.add(value);
         }
@@ -1961,6 +1968,16 @@ public class Conversation extends AbstractEntity
         return Math.abs(Math.rint(multiple) - multiple) < 0.0001f;
     }
 
+    private static boolean isIntegerLexicalValue(final String value) {
+        if (value == null || value.equals("")) return false;
+        final int start = value.charAt(0) == '+' || value.charAt(0) == '-' ? 1 : 0;
+        if (start == value.length()) return false;
+        for (int i = start; i < value.length(); i++) {
+            if (!Character.isDigit(value.charAt(i))) return false;
+        }
+        return true;
+    }
+
     private static boolean isIntegerDatatype(final String datatype) {
         return "xs:integer".equals(datatype)
                 || "xs:int".equals(datatype)

src/test/java/eu/siacs/conversations/entities/ConversationTest.java

@@ -220,6 +220,27 @@ public class ConversationTest {
         Assert.assertNull(Conversation.steppedSliderStep(malformedMax));
     }
 
+    @Test
+    public void steppedSliderStepRejectsIntegerValueThatCannotLandOnStep() {
+        final var field = sliderField("xs:integer", "0", "10", "0.5");
+
+        Assert.assertNull(Conversation.steppedSliderStep(field));
+    }
+
+    @Test
+    public void steppedSliderStepRejectsFractionalIntegerBounds() {
+        final var field = sliderField("xs:integer", "0.5", "10.5", "1.5");
+
+        Assert.assertNull(Conversation.steppedSliderStep(field));
+    }
+
+    @Test
+    public void steppedSliderStepRejectsFractionalIntegerOptions() {
+        final var field = sliderField("xs:integer", "0", "1", "0", "0", "0.5", "1");
+
+        Assert.assertNull(Conversation.steppedSliderStep(field));
+    }
+
     @Test
     public void formatSliderValueDoesNotClampLargeIntegerDatatypesToInt() {
         Assert.assertEquals("3000000000", Conversation.formatSliderValue(3_000_000_000f, "xs:long"));
@@ -310,6 +331,23 @@ public class ConversationTest {
         }
     }
 
+    @Test
+    public void rangedIntegerFieldWithFractionalValuesFallsBackToTextInput() {
+        final var session = withOccupantId.pagerAdapter.new CommandSession(
+                "test", "node", mock(XmppConnectionService.class));
+        try {
+            session.responseElement = new Element("x", Namespace.DATA);
+            session.responseElement.setAttribute("type", "form");
+            final var fractionalBounds = sliderField("xs:integer", "0.5", "10.5", "1.5");
+            final var fractionalOptions = sliderField("xs:integer", "0", "1", "0", "0", "0.5", "1");
+
+            Assert.assertEquals(session.TYPE_TEXT_FIELD, session.mkField(fractionalBounds).viewType);
+            Assert.assertEquals(session.TYPE_TEXT_FIELD, session.mkField(fractionalOptions).viewType);
+        } finally {
+            session.loadingTimer.cancel();
+        }
+    }
+
     @Test
     public void sliderFieldViewHolderRebindsDifferentStepSizesWithoutCrashing() {
         final var context = new ContextThemeWrapper(