search

nostr-protocol / nips

Nostr Implementation Possibilities
2.39k stars 582 forks source link

using colons in this way should be avoided. #1498

Closed RandyMcMillan closed 2 months ago

RandyMcMillan commented 2 months ago

https://github.com/nostr-protocol/nips/blob/efcc86950dd32721e00f868b480a74e9dbd9ca1b/34.md?plain=1#L81

Screenshot 2024-09-16 at 1 47 04 PM

this should probably be changed...

fiatjaf commented 2 months ago

Can you tell why you think it is completely rtrdd to use colons like this?

alexgleason commented 2 months ago

I'm guessing because you can't JSON.parse it, you have to String.split it. However this made me realize that d tags can't use colons in their value.

vitorpamplona commented 2 months ago

However this made me realize that d tags can't use colons in their value.

I am pretty sure I have seen d-tags with : somewhere. So, you should do split with a return limit of 3 substrings.

fiatjaf commented 2 months ago

Or just treat them as invalid, which is better.

mikedilger commented 2 months ago

However this made me realize that d tags can't use colons in their value.

Um... a colon is a character. d-tags are strings. Strings can contain just about any character (refer to JSON escaping and UTF-8). So d-tags can contain colons.

I am pretty sure I have seen d-tags with : somewhere. So, you should do split with a return limit of 3 substrings.

Um, no. d-tags are identifiers. Their value is the identifying string. It is opaque. Colons inside of strings are just characters inside of strings, no more nefarious or rtrdd then the underscore or the letter W. Because the d-tag as at the end of the a-tag, you can ignore colons inside of it when parsing the a tag. If javascript doesn't have good parsing functions, write a better one.

fiatjaf commented 2 months ago

Or just be a nice person and stop creating d tags with colons, as that is much easier for everybody. Can we agree to add this to the spec?

vitorpamplona commented 2 months ago

Because the d-tag as at the end of the a-tag, you can ignore colons inside of it when parsing the a tag.

That's what split with a return limit of 3 substrings mean. It splits the first 2 and the 3rd is the rest of the char sequence.

Or just be a nice person and stop creating d tags with colons

One thing I learned in Nostr is that if there is a way for the writing client to fuck up, they will fuck up. But yes, devs should avoid colons in the d-tag just because it's a "reserved character" of the parent encoding.

mikedilger commented 2 months ago

Or just be a nice person and stop creating d tags with colons

One thing I learned in Nostr is that if there is a way for the writing client to fuck up, they will fuck up. But yes, devs should avoid colons in the d-tag just because it's a "reserved character" of the parent encoding.

If we forbid colons in the 'd' tag, people will still fuck up (by generating random strings, and then by not parsing properly).

But to me this is such a minor issue. I have no opinion of great significance on this topic, so go with whatever the consensus is without counting me.

staab commented 1 month ago

Like with everything else, the spec should be clear about best practices, and clients have to deal with whatever garbage gets published. Prohibit colons, then parse addresses with extra colons properly.