From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com>
Date: Fri, 27 Mar 2020 15:01:18 +0000 (+0100)
Subject: Add consistency checks for input and output units.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8723bc11356d0fdf125fe6214b473de4d954c6cd;p=icu

Add consistency checks for input and output units.
---

diff --git a/icu4c/source/i18n/getunitsdata.cpp b/icu4c/source/i18n/getunitsdata.cpp
index 7724f4cce44..830d89a0ed3 100644
--- a/icu4c/source/i18n/getunitsdata.cpp
+++ b/icu4c/source/i18n/getunitsdata.cpp
@@ -300,36 +300,43 @@ MaybeStackVector<ConversionRateInfo> getConversionRatesInfo(const MeasureUnit so
     ures_getByKey(unitsBundle.getAlias(), "convertUnits", convertUnitsBundle.getAlias(), &status);
 
     ConversionRateDataSink convertSink(result);
-    if (baseCompoundUnit != NULL) {
-        *baseCompoundUnit = MeasureUnit();
-    }
+    MeasureUnit sourceBaseUnit;
     for (int i = 0; i < sourceUnitsLength; i++) {
         MeasureUnit baseUnit;
         processSingleUnit(sourceUnits[i], convertUnitsBundle.getAlias(), convertSink, &baseUnit, status);
-        if (baseCompoundUnit != NULL) {
-            if (source.getComplexity(status) == UMEASURE_UNIT_SEQUENCE) {
-                // TODO(hugovdm): add consistency checks.
-                *baseCompoundUnit = baseUnit;
+        if (source.getComplexity(status) == UMEASURE_UNIT_SEQUENCE) {
+            if (i == 0) {
+                sourceBaseUnit = baseUnit;
             } else {
-                *baseCompoundUnit = baseCompoundUnit->product(baseUnit, status);
+                if (baseUnit != sourceBaseUnit) {
+                    status = U_ILLEGAL_ARGUMENT_ERROR;
+                    return result;
+                }
             }
+        } else {
+            sourceBaseUnit = sourceBaseUnit.product(baseUnit, status);
         }
     }
-    if (baseCompoundUnit != NULL) {
-        *baseCompoundUnit = MeasureUnit();
-    }
+    MeasureUnit targetBaseUnit;
     for (int i = 0; i < targetUnitsLength; i++) {
         MeasureUnit baseUnit;
         processSingleUnit(targetUnits[i], convertUnitsBundle.getAlias(), convertSink, &baseUnit, status);
-        if (baseCompoundUnit != NULL) {
-            if (target.getComplexity(status) == UMEASURE_UNIT_SEQUENCE) {
-                // TODO(hugovdm): add consistency checks.
-                *baseCompoundUnit = baseUnit;
-            } else {
-                *baseCompoundUnit = baseCompoundUnit->product(baseUnit, status);
+        if (target.getComplexity(status) == UMEASURE_UNIT_SEQUENCE) {
+            // WIP/TODO(hugovdm): add consistency checks.
+            if (baseUnit != sourceBaseUnit) {
+                status = U_ILLEGAL_ARGUMENT_ERROR;
+                return result;
             }
+            targetBaseUnit = baseUnit;
+        } else {
+            targetBaseUnit = targetBaseUnit.product(baseUnit, status);
         }
     }
+    if (targetBaseUnit != sourceBaseUnit) {
+        status = U_ILLEGAL_ARGUMENT_ERROR;
+        return result;
+    }
+    if (baseCompoundUnit != NULL) { *baseCompoundUnit = sourceBaseUnit; }
     return result;
 }
 
diff --git a/icu4c/source/i18n/getunitsdata.h b/icu4c/source/i18n/getunitsdata.h
index 3b9e0fb5005..1437b686f8d 100644
--- a/icu4c/source/i18n/getunitsdata.h
+++ b/icu4c/source/i18n/getunitsdata.h
@@ -49,10 +49,13 @@ struct U_I18N_API UnitPreference {
  * Collects and returns ConversionRateInfo needed to convert from source to
  * baseUnit.
  *
+ * If source and target are not compatible for conversion, status will be set to
+ * U_ILLEGAL_ARGUMENT_ERROR.
+ *
  * @param source The source unit (the unit type converted from).
  * @param target The target unit (the unit type converted to).
  * @param baseCompoundUnit Output parameter: if not NULL, it will be set to the
- * base unit type used as pivot for converting from source to target.
+ * compound base unit type used as pivot for converting from source to target.
  * @param status Receives status.
  */
 MaybeStackVector<ConversionRateInfo> U_I18N_API getConversionRatesInfo(MeasureUnit source,