You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Xiao Wang (JIRA)" <ji...@apache.org> on 2019/01/07 09:33:00 UTC
[jira] [Updated] (THRIFT-4727) javascript compiler generates struct
code with duplicate `case 0` statements
[ https://issues.apache.org/jira/browse/THRIFT-4727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Xiao Wang updated THRIFT-4727:
------------------------------
Description:
*Reproducible steps:*
Javascript compiler would always generate duplicate `case 0` codes;
1. Below is a demo IDL file:
{code:java}
// test.thrift
struct Obj {
1: string name;
}
service Test {
Obj test()
}
{code}
2. Generate code using
{code:java}
thrift -r --gen js:node test.thrift{code}
3. The related problematic code is like below:
{code:java}
// part of Test.js
......
Test_test_result.prototype.read = function(input) {
input.readStructBegin();
while (true)
{
var ret = input.readFieldBegin();
var fname = ret.fname;
var ftype = ret.ftype;
var fid = ret.fid;
if (ftype == Thrift.Type.STOP) {
break;
}
switch (fid)
{
case 0:
if (ftype == Thrift.Type.STRUCT) {
this.success = new ttypes.Obj();
this.success.read(input);
} else {
input.skip(ftype);
}
break;
case 0:
input.skip(ftype);
break;
default:
input.skip(ftype);
}
input.readFieldEnd();
}
input.readStructEnd();
return;
};
......{code}
*_Root cause:_*
The code makes this bug was imported due to bugfix THRIFT-1089.
Commit address:[https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4|https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4.]
*_Potential fix:_*
I think remove below code should fix this problem:
{code:java}
if (fields.size() == 1) {
// pseudo case to make jslint happy
indent(out) << "case 0:" << endl;
indent(out) << " input.skip(ftype);" << endl;
indent(out) << " break;" << endl;
}
{code}
This code seems to satisfy jslint about 8 years ago, but now this also fails the static code scan both in jslint and sonarqube default profile.
I could propose a PR if it's OK to fix it this way.
was:
*Reproducible steps:*
Javascript compiler would always generate duplicate `case 0` codes;
1. Below is a demo IDL file:
{code:java}
// test.thrift
struct Obj {
1: string name;
}
service Test {
Obj test()
}
{code}
2. Generate code using
{code:java}
thrift -r --gen js:node test.thrift{code}
3. The related problematic code is like below:
{code:java}
// part of Test.js
......
Test_test_result.prototype.read = function(input) {
input.readStructBegin();
while (true)
{
var ret = input.readFieldBegin();
var fname = ret.fname;
var ftype = ret.ftype;
var fid = ret.fid;
if (ftype == Thrift.Type.STOP) {
break;
}
switch (fid)
{
case 0:
if (ftype == Thrift.Type.STRUCT) {
this.success = new ttypes.Obj();
this.success.read(input);
} else {
input.skip(ftype);
}
break;
case 0:
input.skip(ftype);
break;
default:
input.skip(ftype);
}
input.readFieldEnd();
}
input.readStructEnd();
return;
};
......{code}
*_Root cause:_*
The code makes this bug was imported due to bugfix [THRIFT-1089|https://issues.apache.org/jira/browse/THRIFT-1089].
Commit address:[https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4.]
*_Potential fix:_*
I think remove below code should fix this problem:
{code:java}
if (fields.size() == 1) {
// pseudo case to make jslint happy
indent(out) << "case 0:" << endl;
indent(out) << " input.skip(ftype);" << endl;
indent(out) << " break;" << endl;
}
{code}
This code seems to satisfy jslint about 8 years ago, but now this also fails the static code scan both in jslint and sonarqube default profile.
> javascript compiler generates struct code with duplicate `case 0` statements
> -----------------------------------------------------------------------------
>
> Key: THRIFT-4727
> URL: https://issues.apache.org/jira/browse/THRIFT-4727
> Project: Thrift
> Issue Type: Bug
> Components: JavaScript - Compiler
> Affects Versions: 0.7, 0.8, 0.9, 0.10.0, 0.11.0
> Reporter: Xiao Wang
> Priority: Major
>
> *Reproducible steps:*
> Javascript compiler would always generate duplicate `case 0` codes;
> 1. Below is a demo IDL file:
>
> {code:java}
> // test.thrift
> struct Obj {
> 1: string name;
> }
> service Test {
> Obj test()
> }
> {code}
> 2. Generate code using
> {code:java}
> thrift -r --gen js:node test.thrift{code}
> 3. The related problematic code is like below:
>
>
> {code:java}
> // part of Test.js
> ......
> Test_test_result.prototype.read = function(input) {
> input.readStructBegin();
> while (true)
> {
> var ret = input.readFieldBegin();
> var fname = ret.fname;
> var ftype = ret.ftype;
> var fid = ret.fid;
> if (ftype == Thrift.Type.STOP) {
> break;
> }
> switch (fid)
> {
> case 0:
> if (ftype == Thrift.Type.STRUCT) {
> this.success = new ttypes.Obj();
> this.success.read(input);
> } else {
> input.skip(ftype);
> }
> break;
> case 0:
> input.skip(ftype);
> break;
> default:
> input.skip(ftype);
> }
> input.readFieldEnd();
> }
> input.readStructEnd();
> return;
> };
> ......{code}
> *_Root cause:_*
> The code makes this bug was imported due to bugfix THRIFT-1089.
> Commit address:[https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4|https://github.com/apache/thrift/commit/da6e6ae91894bc52fc04234fbd2610f8969399f4.]
> *_Potential fix:_*
> I think remove below code should fix this problem:
> {code:java}
> if (fields.size() == 1) {
> // pseudo case to make jslint happy
> indent(out) << "case 0:" << endl;
> indent(out) << " input.skip(ftype);" << endl;
> indent(out) << " break;" << endl;
> }
> {code}
> This code seems to satisfy jslint about 8 years ago, but now this also fails the static code scan both in jslint and sonarqube default profile.
> I could propose a PR if it's OK to fix it this way.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)