Invoices with integer descriptions are rejected

[shesek]

Similarly to #1291, descriptions that are JSON encoded as numbers are rejected with the error message description '...' not a string. If the number is encoded as a quoted string, the invoice is accepted.

There are two possible related fixes here:

1. Change lightning-cli invoice to always escape the description as a string. Other client libraries/utilities will have to make sure to do that as well.

2. Always cast the description to a string when processing it.

[rustyrussell]

This should be fixed in #1294 ?

[ZmnSCPxj]

#1294 fixes labels, this issue is descriptions.

[rustyrussell]

Ah, then the answer is WONTFIX.

From https://github.com/lightningnetwork/lightning-rfc/blob/master/11-payment-encoding.md#requirements-3:

...a writer SHOULD make d a complete description of the purpose of the payment

I'm having trouble thinking of the circumstances under which a number meets this requirement.

Remember: the description is what the user sees when they're paying money. It's important to describe what they're paying for!!

[ZmnSCPxj]

ACK #1312 (comment)

[ZmnSCPxj]

Maybe it is a database index to a table row describing the entire thing the payer is paying for. But then it would be pointless unless both payee and payee have access to that database.

[shesek]

Its just weird (a numeric string is a subset of valid strings) and opens up weird edge cases (say, when the description field is user-provided, it sometimes won't work).

I think that its reasonable not to implement (2), but I would consider (1) a bug.

[practicalswift]

Allowing A while disallowing 1 is confusing from a user perspective. Users cannot be expected to understand the difference between 1 and "1".

I've encountered this specific issue myself when trying out the tools using dummy parameters such as 1 for testing.

My immediate reaction was "oh, something is broken here" rather than "oh, I should have be more verbose in my dummy parameters when testing" :-)

[ZmnSCPxj]

Allowing A while disallowing 1 is confusing from a user perspective. Users cannot be expected to understand the difference between 1 and "1".

We could also argue, that programmers at least must be expected to understand the difference between 1 and "1", and that users are not expected to actually use lightning-cli, but instead, some GUI on top of lightningd that properly escapes descriptions as strings.

Unfortunately given #1294 we have existing precedent that sometimes numbers are automatically cast to strings in lightningd side when needful. The inconsistency here between labels and descriptions, we can also argue, is jarring.

(Going with (1) implies lightning-cli needs to be smarter (e.g. maybe it can query expected types for each parameter of the command before actually sending the command))

[shesek]

We could also argue, that programmers at least must be expected to understand the difference between 1 and "1"

The thing is, $ lightning-cli invoice 1000 myid "123" does not work either... there's no way to get this to work with lightning-cli, while other jsonrpc clients (like lightning-client-js) do allow it. This is somewhat awkward :s

[ZmnSCPxj]

The thing is, $ lightning-cli invoice 1000 myid "123" does not work either... there's no way to get this to work with lightning-cli, while other jsonrpc clients (like lightning-client-js) do allow it. This is somewhat awkward.

The issue is the shell, which also treats " as having special meaning. The correct way to do this on-shell is $ lightning-cli invoice 1000 myid '"'123'"' or $ lightning-cli invoice 1000 myid \"123\". Shells are nice but often baroque...

xref.

lightning/Makefile

Line 166 in 26f7014

CPPFLAGS = -DBINTOPKGLIBEXECDIR='"'$(shell sh tools/rel.sh $(bindir) $(pkglibexecdir))'"'

Which shows, how to get " across the shell launched by Makefile into the C compiler.

[shesek]

oh, I see. I didn't realize lightning-cli treated arguments as JSON-encoded and accepted literally quoted strings in addition to plain strings. How does that work? How does it know when the argument should be JSON decoded and when it should be a plain string? Some kind of heuristic, I presume?

Could it ever be problematic for an app to pass down user input as an argument expecting it to be a plain string, where the user can instead send a JSON encoded one with quotes? I can't seem to think of any potential ways to abuse this, other than perhaps creating some small inconsistencies: some overlay service like lightning charge that uses lightning-cli could store '"123"' as the description in its own db, while the description parsed out by c-lightning will be '123'.

Seeing that lightning-cli allows to pass explicit strings, I'm closing this.