search

ssbc / ssb-meta-feeds

10 stars 0 forks source link

bug: close metadata edgecase #77

Closed mixmix closed 1 year ago

mixmix commented 1 year ago

While reading the code, I noticed a nasty edge case you could fall into with metadata - because of how it was merged into bendy butt messages, you could over-write the type and purpose of a metafeed announce message accidentally.

I considered two options:

  1. (simple) bar some reserved words from use in metadata
  2. store metadata under a dedicated field in message

I went with (2) thinking it would be simple, but the tests turned up a counter edge case with metadata.recps being used for announce messages... so ... yes.

staltz commented 1 year ago

Hmm, before I delve into the details here, I think what you're proposing is a breaking change, not only to ssb-meta-feeds, but also to the "spec" (the implicit spec, i.e. the agreement between peers. We should make the implicit explicit, though).

Like, you may be creating an incompatibility with go-metafeed: https://github.com/ssbc/go-metafeed/search?q=metadata and I'd prefer some other solution which doesn't cause incompat or breaking change.

Also, because @arj03 was involved designing this, let's ask his review.

mixmix commented 1 year ago

@arj03 can you please review? (see the initial post for context)

arj03 commented 1 year ago

Oh sorry I never saw this. The spec as it is currently (it could be worded a bit better and have better examples, especially for metadata!), but everything in the content is metadata: https://github.com/ssbc/ssb-meta-feeds-spec#example-of-a-metafeed. So I would rather go for option 1 in you description.

mixmix commented 1 year ago

Replacing with https://github.com/ssbc/ssb-meta-feeds/pull/87