Skip to content

Commit

Permalink
fix(cli): sigsev on null schema (#857)
Browse files Browse the repository at this point in the history
Previously, we did not handle schema being a nullptr

Fixes: #847

Co-authored-by: jon-chuang <jon-chuang@users.noreply.github.com>
Co-authored-by: james-rms <james.r.m.smith@gmail.com>
  • Loading branch information
3 people authored Mar 19, 2023
1 parent 0842cbf commit dfdafe9
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 44 deletions.
2 changes: 2 additions & 0 deletions go/cli/mcap/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ To install from the latest commit, use

go install github.com/foxglove/mcap/go/cli/mcap@latest

Ensure that the go installation directory is in your path (e.g. `$HOME/go/bin` by default on Ubuntu).

### Examples:

#### Bag to mcap conversion
Expand Down
100 changes: 56 additions & 44 deletions go/cli/mcap/cmd/cat.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,57 +109,68 @@ func printMessages(
}
continue
}
switch schema.Encoding {
case "ros1msg":
transcoder, ok := transcoders[channel.SchemaID]
if !ok {
packageName := strings.Split(schema.Name, "/")[0]
transcoder, err = ros.NewJSONTranscoder(packageName, schema.Data)
if err != nil {
return fmt.Errorf("failed to build transcoder for %s: %w", channel.Topic, err)
if schema == nil || schema.Encoding == "" {
switch channel.MessageEncoding {
case "json":
if _, err = msg.Write(message.Data); err != nil {
return fmt.Errorf("failed to write message bytes: %w", err)
}
transcoders[channel.SchemaID] = transcoder
}
msgReader.Reset(message.Data)
err = transcoder.Transcode(msg, msgReader)
if err != nil {
return fmt.Errorf("failed to transcode %s record on %s: %w", schema.Name, channel.Topic, err)
default:
return fmt.Errorf("For schema-less channels, JSON output is only supported with 'json' message encoding. Found: %s", channel.MessageEncoding)
}

This comment has been minimized.

Copy link
@defunctzombie

defunctzombie Mar 19, 2023

Contributor

small style nit: early return in the first if statement case would allow you to avoid nesting the remaining code portions.

case "protobuf":
messageDescriptor, ok := descriptors[channel.SchemaID]
if !ok {
fileDescriptorSet := &descriptorpb.FileDescriptorSet{}
if err := proto.Unmarshal(schema.Data, fileDescriptorSet); err != nil {
return fmt.Errorf("failed to build file descriptor set: %w", err)
} else {
switch schema.Encoding {
case "ros1msg":
transcoder, ok := transcoders[channel.SchemaID]
if !ok {
packageName := strings.Split(schema.Name, "/")[0]
transcoder, err = ros.NewJSONTranscoder(packageName, schema.Data)
if err != nil {
return fmt.Errorf("failed to build transcoder for %s: %w", channel.Topic, err)
}
transcoders[channel.SchemaID] = transcoder
}
files, err := protodesc.FileOptions{}.NewFiles(fileDescriptorSet)
msgReader.Reset(message.Data)
err = transcoder.Transcode(msg, msgReader)
if err != nil {
return fmt.Errorf("failed to create file descriptor: %w", err)
return fmt.Errorf("failed to transcode %s record on %s: %w", schema.Name, channel.Topic, err)
}
descriptor, err := files.FindDescriptorByName(protoreflect.FullName(schema.Name))
case "protobuf":
messageDescriptor, ok := descriptors[channel.SchemaID]
if !ok {
fileDescriptorSet := &descriptorpb.FileDescriptorSet{}
if err := proto.Unmarshal(schema.Data, fileDescriptorSet); err != nil {
return fmt.Errorf("failed to build file descriptor set: %w", err)
}
files, err := protodesc.FileOptions{}.NewFiles(fileDescriptorSet)
if err != nil {
return fmt.Errorf("failed to create file descriptor: %w", err)
}
descriptor, err := files.FindDescriptorByName(protoreflect.FullName(schema.Name))
if err != nil {
return fmt.Errorf("failed to find descriptor: %w", err)
}
messageDescriptor = descriptor.(protoreflect.MessageDescriptor)
descriptors[channel.SchemaID] = messageDescriptor
}
protoMsg := dynamicpb.NewMessage(messageDescriptor.(protoreflect.MessageDescriptor))
if err := proto.Unmarshal(message.Data, protoMsg); err != nil {
return fmt.Errorf("failed to parse message: %w", err)
}
bytes, err := protojson.Marshal(protoMsg)
if err != nil {
return fmt.Errorf("failed to find descriptor: %w", err)
return fmt.Errorf("failed to marshal message: %w", err)
}
messageDescriptor = descriptor.(protoreflect.MessageDescriptor)
descriptors[channel.SchemaID] = messageDescriptor
}
protoMsg := dynamicpb.NewMessage(messageDescriptor.(protoreflect.MessageDescriptor))
if err := proto.Unmarshal(message.Data, protoMsg); err != nil {
return fmt.Errorf("failed to parse message: %w", err)
}
bytes, err := protojson.Marshal(protoMsg)
if err != nil {
return fmt.Errorf("failed to marshal message: %w", err)
}
if _, err = msg.Write(bytes); err != nil {
return fmt.Errorf("failed to write message bytes: %w", err)
}
case "jsonschema":
if _, err = msg.Write(message.Data); err != nil {
return fmt.Errorf("failed to write message bytes: %w", err)
if _, err = msg.Write(bytes); err != nil {
return fmt.Errorf("failed to write message bytes: %w", err)
}
case "jsonschema":
if _, err = msg.Write(message.Data); err != nil {
return fmt.Errorf("failed to write message bytes: %w", err)
}
default:
return fmt.Errorf("JSON output only supported for ros1msg, protobuf, and jsonschema schemas. Found: %s", schema.Encoding)
}
default:
return fmt.Errorf("JSON output only supported for ros1msg, protobuf, and JSON schemas")
}
target.Topic = channel.Topic
target.Sequence = message.Sequence
Expand Down Expand Up @@ -237,5 +248,6 @@ func init() {
catCmd.PersistentFlags().Int64VarP(&catStart, "start-secs", "", 0, "start time")
catCmd.PersistentFlags().Int64VarP(&catEnd, "end-secs", "", math.MaxInt64, "end time")
catCmd.PersistentFlags().StringVarP(&catTopics, "topics", "", "", "comma-separated list of topics")
catCmd.PersistentFlags().BoolVarP(&catFormatJSON, "json", "", false, "print messages as JSON")
catCmd.PersistentFlags().BoolVarP(&catFormatJSON, "json", "", false,
`print messages as JSON. Supported message encodings: ros1, protobuf, and json.`)
}

0 comments on commit dfdafe9

Please sign in to comment.