Skip to content

sbe: messages are found by template id alone, so two schemas sharing one collide (protowire-go#256, #258, #270) #134

Closed

Description

@bfrancojr

A template id is unique only within a schema. This port keys messages by template id alone and never compares a header's schema id. protowire-go fixed this in three commits: trendvidia/protowire-go#256 and #258 (issue #253, decided by the owner), and #270 (issue #257, owner chose option 1; spec trendvidia/protowire#571).

What C++ does now (by reading main a632c6e, not measured):

  • Codec::RegisterMessage (src/sbe/codec.cc) does by_id_[owned->template_id] = …. A second message with the same (sbe.template_id) silently replaces the first, in the same schema or in another one.
  • Codec::NewView looks up by_id_ by template id alone. It finds the last message registered and reads the other schema's bytes as that message.
  • Codec::UnmarshalInto (src/sbe/unmarshal.cc) checks the header's template id against the target, but not its schema id (header bytes 4-5). Schema 1's bytes for template 7 decode as schema 2's template 7.

Go's behaviour, to mirror:

  • Codec::New refuses two different messages with one (sbe.template_id) in one schema: sbe: messages %s and %s share (sbe.template_id) = %d in schema %d; it must be unique within a schema. The same message registered twice, as when a file is passed twice, is not a duplicate. Two schemas that share a template id still bind.
  • by_id_ is keyed by (schema id, template id), and NewView reads the schema id from header bytes 4-5. An unknown pair is refused with sbe: unknown template ID %d in schema %d, even if another schema has that template id.
  • Unmarshal and UnmarshalDescriptor refuse a schema id mismatch: sbe: schema ID mismatch: got %d, want %d for template %d. The version is not checked.
  • Tests ported from Go's encoding/sbe/template_id_test.go, plus TestView_BlockLengthTooSmall's header now setting a schema id.

This narrows what decodes. A caller that builds headers by hand must now set the schema id. The CHANGELOG should say so, as Go's does.

Note: #124 (stale-descriptor diagnosis) also edits RegisterMessage, so this work should stack on it or follow its merge.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions