Is parseInt coercion in toNumberIfNumber function intentional for matching integer-only?

[bparth24]

@dlongley

The toNumberIfNumber function in lib/query/util.js uses parseInt(x, 10) for numeric coercion. I wanted to check whether this is intentional to restrict coercion to integer values only, or whether it should handle decimal values and reject partially numeric strings as well.

Currently:

toNumberIfNumber("3.14")    // -> 3    (decimal truncated)
toNumberIfNumber("21abc")   // -> 21   (trailing characters ignored)

This means a credential with "score": "3.14" would match a query expecting 3, and "value": "21abc" would match a query expecting 21.

Is this the intended behavior?

Asking because I'm documenting the normative matching algorithm for QueryByExample in the VCALM specification (w3c/vcalm#574), and the answer determines whether the spec should say "integer coercion" or "numeric coercion."

If this is not intentional, the following describes the expected behavior and a suggested fix.

Expected behavior

toNumberIfNumber("3.14")   // -> 3.14  (decimal preserved)
toNumberIfNumber("21abc")  // -> "21abc" (rejected, not a valid number)

Suggested fix

Replace parseInt with Number() and a non-empty string guard:

export function toNumberIfNumber(x) {
  if(typeof x === 'number') {
    return x;
  }
  if(typeof x === 'string' && x.trim() !== '' && !isNaN(Number(x))) {
    return Number(x);
  }
  return x;
}

This ensures:

• "21" → 21 (valid, preserved)
• "3.14" → 3.14 (valid, decimal preserved)
• "21abc" → "21abc" (rejected, not entirely numeric)
• "" → "" (rejected, prevents empty string matching 0)

[dlongley]

Hmm, well it shouldn't map to incorrect integers, i.e., it should not map the string 3.14 => 3 or the string 21abc => 21. We should definitely fix that behavior. We probably should rename it to toIntegerIfInteger() though; I suspect this was the original intention. Changing decimals expressed as strings might cause trouble so I'd say we ought to avoid it for now.

[bparth24]

Hmm, well it shouldn't map to incorrect integers, i.e., it should not map the string 3.14 => 3 or the string 21abc => 21. We should definitely fix that behavior. We probably should rename it to toIntegerIfInteger() though; I suspect this was the original intention. Changing decimals expressed as strings might cause trouble so I'd say we ought to avoid it for now.

@dlongley Understood.

Do you think the code below should fix the above concerns:

export function toIntegerIfInteger(x) {
  if(typeof x === 'number') {
    return x;
  }
  if(typeof x === 'string' && /^-?\d+$/.test(x.trim())) {
    return parseInt(x, 10);
  }
  return x;
}

The behavior becomes:

"21" -> 21  (valid integer string, coerce)
"3.14" -> "3.14"  (not an integer, rejected)
"21abc" -> "21abc"  (not valid, rejected)
"" -> ""  (rejected)
"-5" -> -5  (valid negative integer, coerce)
42 -> 42  (already a number, pass through)

[dlongley]

Perhaps:

export function toIntegerIfInteger(x) {
  if(typeof x === 'number') {
    return x;
  }
  if(typeof x === 'string') {
    const i = parseInt(x, 10);
    return i.toString() === x ? i : x;
  }
  return x;
}