Proto enums can produce invalid yaml in swagger files

[ianrose14]

🐛 Bug Report

If an input proto file contains an enum where some of the individual enum values have comments, the yaml produced by the protoc-gen-openapiv2 plugin can be invalid.

To Reproduce

1. Start with a proto file like the following:

syntax = "proto3";
package repro;
option go_package = "repro/foobar";

import "google/api/annotations.proto";

service Repro {
    rpc GetRollup(RollupRequest) returns (RollupResponse) {
        option (google.api.http) = {
	  get: "/rollup"
        };
    }
}

message RollupRequest {
  RollupType type = 1;
}
message RollupResponse {}

enum RollupType {
    UNKNOWN_ROLLUP = 0;

    // Apples are good
    APPLE = 1;
    BANANA = 2;

    // Carrots are mediocre
    CARROT = 3;
}

2. Name it repro.proto and save it in a directory named repro along with the usual annotations.proto and http.proto files.
3. Run the following command:

protoc repro/repro.proto --proto_path=repro \
    --openapiv2_out=repro --openapiv2_opt logtostderr=true --openapiv2_opt output_format=yaml

4. The resulting file (repro.swagger.yaml) is invalid. You can verify this with various online yaml parsers or with a small Go program like this:

package main

import (
	"fmt"
	"os"

	"gopkg.in/yaml.v2"
)

func main() {
	r, err := os.Open("repro/repro.swagger.yaml")
	if err != nil {
		panic(err)
	}

	m := make(map[string]any)
	err = yaml.NewDecoder(r).Decode(m)
	if err != nil {
		panic(err)
	}

	fmt.Printf("ok: +%v\n", m)
}

For convenience, here's the bit of the yaml that comes out broken:

            parameters:
                - name: type
                  description: |4-
                     - APPLE: Apples are good
                     - CARROT: Carrots are mediocre
                  in: query
                  required: false

Expected behavior

The swagger file produced should be valid yaml.

Actual Behavior

The swagger file is invalid yaml.

Your Environment

Darwin MacBook-Pro 21.5.0
macOS v12.5
protoc --version --> libprotoc 3.19.4
go version --> go version go1.18.3 darwin/arm64

[johanbrandhorst]

Oh dear! That definitely looks like a bug. The YAML format is a reasonably new addition, so it's not surprising that there are some kinks to work out. It was contributed by @hedhyw in #2579. I wonder if this is a problem with our YAML library? Would you be interested in helping contribute a fix for this?

Thanks for the excellent bug report, by the way!

[hedhyw]

@ianrose14

Try the version v2.10.2.

go install \
    github.com/grpc-ecosystem/grpc-gateway/v2/[email protected] \
    github.com/grpc-ecosystem/grpc-gateway/v2/[email protected]

v2.10.3 reproduces error for me, v2.10.2 - doesn't

[hedhyw]

It seems it was broken in #2729

[hedhyw]

@johanbrandhorst

The problem is with yaml.v3 library: go-yaml/yaml#643

I've just tried it in the playground: https://go.dev/play/p/tTc7F_T86vF

We can solve it as:

1. Revert Update gopkg.in/yaml.v3 #2729
2. Trim leading spaces for all fields (it's like a crutch)
3. Fix the YAML library: v3: Invalid output (bad indentation) when an indentation indicator/hint is used in a scalar block go-yaml/yaml#643

[ianrose14]

github.com/grpc-ecosystem/grpc-gateway/v2/[email protected]

Yes, that seems to work for me! Sorry for not including my grpc-gateway version in the original bug report - silly oversight.

[johanbrandhorst]

I suspected it might be the version upgrade, thanks a lot for digging into it @hedhyw! Since the original reason for upgrading to v3 was "just because", I'm leaning towards just reverting for now. @sousandrei @hedhyw @ianrose14 would one of you be interested in taking charge of reverting the other change to fix this?

[hedhyw]

@johanbrandhorst As I see it makes sense to revert only a part of protoc-gen-openapiv2. I can do it.

[hedhyw]

I created a fix for the YAML library: go-yaml/yaml#864, we can wait until it will be merged if it will be.

[johanbrandhorst]

That's even better, thank you!

[hedhyw]

It seems like go-yaml/yaml#864 is not going to be merged (go-yaml/yaml#643), so let's decide:

1. pt. 1-2: Proto enums can produce invalid yaml in swagger files #2745 (comment)
2. replace to fork with the fix.
3. something else?

[johanbrandhorst]

Hm, sounds to me like we should just revert back to v2 until there is any movement merging of the v3 fix. What do you think?