grpc-ecosystem/grpc-gateway

Nested message names can collide silently in swagger generation and result in randomized swagger message definitions

Open

#1066 opened on Oct 21, 2019

View on GitHub
 (6 comments) (0 reactions) (0 assignees)Go (16,971 stars) (2,250 forks)batch import
bughelp wantedopenapi

Description

Generated swagger files can silently have name collisions between message names when messages are nested. For example:

syntax = "proto3";

message FooBar {
  string value = 1;
}

message Foo {
  message Bar {
    string value = 1;
  }
  Bar bar = 1;
}

service FooBarService {
  rpc BarYourFoos(FooBar) returns (Foo);
}

Here you will sometimes end up with a different definition of FooBar depending on how two go maps are presumably being unioned? For example:

{
  "swagger": "2.0",
  "info": {
    "title": "test.proto",
    "version": "version not set"
  },
  "schemes": [
    "http",
    "https"
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {},
  "definitions": {
    "Foo": {
      "type": "object",
      "properties": {
        "bar": {
          "$ref": "#/definitions/FooBar"
        }
      }
    },
    "FooBar": {
      "type": "object",
      "properties": {
        "value": {
          "type": "string"
        }
      }
    }
  }
}

Which contains a ref to the wrong field type. This behavior can happen across imports so if you have one nested message in another file you can end up with unexpected random collisions which lead to different file output.

This was causing a CI pipeline to randomly fail internally as one message field in one file randomly collided with another.

I would expect that a collision like this should instead error or have sufficient namespacing so as to be avoided.

Have prepared an example repo with commited generated files, you can clone it and run:

prototool generate

To reproduce and if you use nix a shell is provided to pin against the latest release.

https://github.com/johnchildren/grpc-gateway-namespacing-issue-example

edit: Actually upon re-running it looks like it is only randomly wrong when you import it from another file? I can extend my test case but it is still behaving incorrectly in the example.

Contributor guide