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

Fix hand-crafted protobuf message #1016

Merged
merged 1 commit into from
Aug 30, 2019
Merged

Fix hand-crafted protobuf message #1016

merged 1 commit into from
Aug 30, 2019

Conversation

dsnet
Copy link
Contributor

@dsnet dsnet commented Aug 30, 2019

errorBody is an improperly hand-crafted protobuf Message.
In particular, field number 1 is used twice, causing this to
message to crash in the v2 re-implementation of Go protobufs.

This PR modifies the field numbers to be unique.
Since Error is a special field not in status.proto,
we choose a large field number to avoid conflicting with
any new field that may be added to the real Status message.
We also fix the field numbers to truly match up with Status.

errorBody is an improperly hand-crafted protobuf Message.
In particular, field number 1 is used twice, causing this to
message to crash in the v2 re-implementation of Go protobufs.

This PR modifies the field numbers to be unique.
Since Error is a special field not in status.proto,
we choose a large field number to avoid conflicting with
any new field that may be added to the real Status message.
We also fix the field numbers to truly match up with Status.
@dsnet
Copy link
Contributor Author

dsnet commented Aug 30, 2019

\cc @johanbrandhorst

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@a9bbe40). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1016   +/-   ##
=========================================
  Coverage          ?   53.41%           
=========================================
  Files             ?       40           
  Lines             ?     4038           
  Branches          ?        0           
=========================================
  Hits              ?     2157           
  Misses            ?     1679           
  Partials          ?      202
Impacted Files Coverage Δ
runtime/errors.go 47.36% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9bbe40...20c2791. Read the comment docs.

@achew22 achew22 merged commit 2da5bfa into grpc-ecosystem:master Aug 30, 2019
@achew22
Copy link
Collaborator

achew22 commented Aug 30, 2019

@dsnet, hope that wasn't too hard to track down. Sorry about that!

adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
errorBody is an improperly hand-crafted protobuf Message.
In particular, field number 1 is used twice, causing this to
message to crash in the v2 re-implementation of Go protobufs.

This PR modifies the field numbers to be unique.
Since Error is a special field not in status.proto,
we choose a large field number to avoid conflicting with
any new field that may be added to the real Status message.
We also fix the field numbers to truly match up with Status.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants