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

Illegal name ';' when parsing the rhs of an option assignment that is not a simple strLit #1569

Closed
jeffory-orrok opened this issue Mar 24, 2021 · 2 comments · Fixed by #1571

Comments

@jeffory-orrok
Copy link
Contributor

protobuf.js version: 6.10.2

Although the Protocol Buffers Version 3 Language Specification dictates a fairly limited set of what can appear on the rhs of an option assignment (namely a fullIdent, a boolean, an integer, a float, or a quoted string that does not span multiple lines), in practice more complex, multi-line output is often encountered (see for example grpc-gateway and googleapis).

When I posted a question about this on the Protocol Buffers Google Group, I received a private reply stating that one should expect anything that can be produced by the TextFormat class.

Attached is a zipfile which demonstrates the issue.
example.zip

Steps to reproduce

mkdir /tmp/test_example
cd /tmp/test_example
mv ~/Downloads/example.zip .
unzip example.zip
tar zxf example.tgz
cd parse_example
npm install
node index.js

Output

node index.js
installing semver@^7.1.2
installing chalk@^4.0.0
installing glob@^7.1.6
installing jsdoc@^3.6.3
installing tmp@^0.2.0
installing uglify-js@^3.7.7
installing espree@^7.0.0
installing escodegen@^2.0.0
installing estraverse@^5.1.0
/private/tmp/test_example/parse_example/index.js:33
    if( err ) throw err;
              ^

Error: illegal name ';' (/private/tmp/test_example/example/company/v1/example_service.proto, line 17)
    at illegal (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:96:16)
    at parseOptionValue (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:570:27)
    at parseOptionValue (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:579:33)
    at parseOption (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:560:27)
    at parseCommon (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:256:17)
    at parseService_block (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:628:17)
    at ifBlock (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:290:17)
    at parseService (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:627:9)
    at parseCommon (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:269:17)
    at parse (/private/tmp/test_example/parse_example/node_modules/protobufjs/src/parse.js:757:21)
@jeffory-orrok
Copy link
Contributor Author

I don't know if this is muddying the waters, but when I look at text_format.cc A few lines caught my eye

Lines 600-602 in ConsumeField(), repeated at 641-643 in SkipField()

    // For historical reasons, fields may optionally be separated by commas or
    // semicolons.
    TryConsume(";") || TryConsume(",");

@jeffory-orrok
Copy link
Contributor Author

Please see #1571 for a possible resolution for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant