Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server features should be []any, not []string #7932

Closed
dfawley opened this issue Dec 13, 2024 · 2 comments
Closed

Server features should be []any, not []string #7932

dfawley opened this issue Dec 13, 2024 · 2 comments
Assignees

Comments

@dfawley
Copy link
Member

dfawley commented Dec 13, 2024

ServerFeatures []string `json:"server_features,omitempty"`

Per A30:

      // A list of features supported by the server.  New values will
      // be added over time.  For forward compatibility reasons, the
      // client will ignore any entry in the list that it does not
      // understand, ***regardless of type***.

(*'s added for emphasis)

We need to not fail to parse if other types are encountered here.

@dfawley
Copy link
Member Author

dfawley commented Dec 13, 2024

It turns out Go's json umarshalling will result in an empty string instead of failing, so this may not be urgent.

https://go.dev/play/p/T-Qa9ygkcsC

Also, CC @ejona86 who says Java is (also?) decoding it as an array of strings, and @markdroth as FYI.

ejona86 added a commit to ejona86/grpc-java that referenced this issue Dec 13, 2024
It was clearly defined in gRFC A30. The relevant text was copied as a
comment in the code.

As discovered due to grpc/grpc-go#7932
ejona86 added a commit to grpc/grpc-java that referenced this issue Dec 16, 2024
It was clearly defined in gRFC A30. The relevant text was copied as a
comment in the code.

As discovered due to grpc/grpc-go#7932
ejona86 added a commit to ejona86/grpc-java that referenced this issue Dec 16, 2024
It was clearly defined in gRFC A30. The relevant text was copied as a
comment in the code.

As discovered due to grpc/grpc-go#7932
ejona86 added a commit to ejona86/grpc-java that referenced this issue Dec 16, 2024
It was clearly defined in gRFC A30. The relevant text was copied as a
comment in the code.

As discovered due to grpc/grpc-go#7932
ejona86 added a commit to grpc/grpc-java that referenced this issue Dec 17, 2024
It was clearly defined in gRFC A30. The relevant text was copied as a
comment in the code.

As discovered due to grpc/grpc-go#7932
@dfawley
Copy link
Member Author

dfawley commented Dec 17, 2024

I'm going to close this since we can just add support for non-string types (however that might work eventually) when we need them. We won't fail to parse newer configs that specify them with today's code.

@dfawley dfawley closed this as completed Dec 17, 2024
ejona86 added a commit to grpc/grpc-java that referenced this issue Dec 19, 2024
It was clearly defined in gRFC A30. The relevant text was copied as a
comment in the code.

As discovered due to grpc/grpc-go#7932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants