Skip to content

Commit

Permalink
CLI: fix bug where mcap merge crashes on channels with schema_id ==…
Browse files Browse the repository at this point in the history
… 0 (#830)

Fixes a crash in `mcap merge` where a schemaless channel (ie. one with schema_id == 0) would cause the CLI tool to crash.
  • Loading branch information
james-rms authored Feb 23, 2023
1 parent ab391a5 commit 33fa531
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
11 changes: 8 additions & 3 deletions go/cli/mcap/cmd/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ func (m *mcapMerger) outputChannelID(inputID int, inputChannelID uint16) (uint16
}

func (m *mcapMerger) outputSchemaID(inputID int, inputSchemaID uint16) (uint16, bool) {
if inputSchemaID == 0 {
return 0, true
}
v, ok := m.schemaIDs[schemaID{
inputID: inputID,
schemaID: inputSchemaID,
Expand Down Expand Up @@ -183,9 +186,11 @@ func (m *mcapMerger) mergeInputs(w io.Writer, inputs []io.Reader) error {
}
return fmt.Errorf("failed to read first message on input %d: %w", inputID, err)
}
schema.ID, err = m.addSchema(writer, inputID, schema)
if err != nil {
return fmt.Errorf("failed to add initial schema for input %d: %w", inputID, err)
if schema != nil {
schema.ID, err = m.addSchema(writer, inputID, schema)
if err != nil {
return fmt.Errorf("failed to add initial schema for input %d: %w", inputID, err)
}
}
message.ChannelID, err = m.addChannel(writer, inputID, channel)
if err != nil {
Expand Down
36 changes: 33 additions & 3 deletions go/cli/mcap/cmd/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ func prepInput(t *testing.T, w io.Writer, schemaID uint16, channelID uint16, top
assert.Nil(t, err)

assert.Nil(t, writer.WriteHeader(&mcap.Header{Profile: "testprofile"}))
assert.Nil(t, writer.WriteSchema(&mcap.Schema{
ID: schemaID,
}))
if schemaID != 0 {
assert.Nil(t, writer.WriteSchema(&mcap.Schema{
ID: schemaID,
}))
}
assert.Nil(t, writer.WriteChannel(&mcap.Channel{
ID: channelID,
SchemaID: schemaID,
Expand Down Expand Up @@ -96,3 +98,31 @@ func TestMultiChannelInput(t *testing.T) {
assert.Equal(t, 100, messages["/bar"])
assert.Equal(t, 100, messages["/baz"])
}
func TestSchemalessChannelInput(t *testing.T) {
buf1 := &bytes.Buffer{}
buf2 := &bytes.Buffer{}
prepInput(t, buf1, 0, 1, "/foo")
prepInput(t, buf2, 1, 1, "/bar")
merger := newMCAPMerger(mergeOpts{})
output := &bytes.Buffer{}
assert.Nil(t, merger.mergeInputs(output, []io.Reader{buf1, buf2}))

// output should now be a well-formed mcap
reader, err := mcap.NewReader(output)
assert.Nil(t, err)
assert.Equal(t, reader.Header().Profile, "testprofile")
it, err := reader.Messages(readopts.UsingIndex(false))
assert.Nil(t, err)
messages := make(map[string]int)
schemaIDs := make(map[uint16]int)
err = mcap.Range(it, func(schema *mcap.Schema, channel *mcap.Channel, message *mcap.Message) error {
messages[channel.Topic]++
schemaIDs[channel.SchemaID]++
return nil
})
assert.Nil(t, err)
assert.Equal(t, 100, messages["/foo"])
assert.Equal(t, 100, messages["/bar"])
assert.Equal(t, 100, schemaIDs[0])
assert.Equal(t, 100, schemaIDs[1])
}

0 comments on commit 33fa531

Please sign in to comment.