protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

protoc: emptyStatement is not allowed in extend

Open WillAbides opened this issue 4 years ago • 4 comments

protoc errors on emptyStatement (a single ";") in an extend definition.

The proto2 spec defines extend as:

extend = "extend" messageType "{" {field | group | emptyStatement} "}"

Based on that I think an extra ";" should be valid.

What version of protobuf and what language are you using? Version: 3.19.1 Language: N/A

What operating system (Linux, Windows, ...) and version? macOS 11.6

What runtime / compiler are you using (e.g., python version or gcc version) N/A

What did you do? Steps to reproduce the behavior:

  1. Create a.proto with this content:
syntax = "proto2";
package a;
import "google/protobuf/descriptor.proto";

extend google.protobuf.FileOptions {
  optional bool foo = 1000;;
}
  1. Run protoc -o ./protodump a.proto

What did you expect to see

File descriptor written to protodump and exit 0.

What did you see instead?

exit code: 1 stderr:

a.proto:6:28: Expected "required", "optional", or "repeated".
a.proto:6:28: Expected type name.

WillAbides avatar Nov 06 '21 17:11 WillAbides

Sorry for the delayed response. Based on the spec I don't see why an empty line with just a semicolon should be valid. Why do you want to be able to add the extra semicolon?

deannagarcia avatar Jul 14 '22 22:07 deannagarcia

When I created this issue I was using protoc as an oracle while fuzz testing another program that parses proto files. Testing revealed a discrepancy when there is a second semicolon at the end of a line in an extend. The inclusion of emptyStatement in proto2 specification indicates that it should be allowed because it is defined as emptyStatement = "'".

To answer your question of why I want to add the extra semicolon, I don't. I just created the issue because I thought the maintainers would be interested to know that protoc is divergent from the published spec.

protoc's actual behavior would be better describe by

extend = "extend" messageType "{" (field | group) {field | group} "}"

because emptyStatement isn't allowed and at least one field or group is required.

I am not currently working with protoc or protocol buffers, so I don't have a stake in the outcome of this issue. Feel free to close it or keep it open based on the needs of the project.

WillAbides avatar Jul 15 '22 21:07 WillAbides

It turns out that the production for oneof in the documented grammar is similarly incorrect. It also claims to allow emptyStatement, but protoc does not allow it.

syntax = "proto3";
message Foo {
  oneof a {
    string s = 3;
    bytes b = 4;
    ;
  }
}

Compiling the above with protoc produces an error:

test.proto:6:5: Expected type name.

jhump avatar Sep 01 '22 01:09 jhump

FYI, this is closely related to #5075

jhump avatar Sep 07 '22 14:09 jhump

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar Feb 09 '24 10:02 github-actions[bot]

emptyStatement has been removed from extend. See: https://protobuf.dev/reference/protobuf/proto2-spec/#extend

googleberg avatar Feb 09 '24 21:02 googleberg