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

Bug: .encode doesn't work properly on nested oneof members. #583

Closed
kfir-drivenets opened this issue Dec 26, 2016 · 5 comments
Closed

Bug: .encode doesn't work properly on nested oneof members. #583

kfir-drivenets opened this issue Dec 26, 2016 · 5 comments

Comments

@kfir-drivenets
Copy link

protobuf.js version: 6.2.1

Encoding message that includes nested oneof elements results in an empty buffer.

Using the following protobuf as an example:

syntax='proto2';					
message A {								
	required int32 b = 1;			
}														
														
message B {									
	required int32 c = 1;			
}														
														
message C {									
	oneof child {						
		A a = 1;							
		B b = 2;								
	}													
} 

message D {  
    oneof child {
        C c = 1; 
        B b = 2; 
    }
}

And the relevant js

var D = root.lookup("D");
var d_message = D.create({
	c: {
  		a: {
    			b: 10
   		}
	}
});
console.log(d_message);
buffer = D.encode(d_message).finish();
console.log(buffer.length, buffer);

The resulting buffer will be empty.

In this version however it does work

var C = root.lookup("C");
var D = root.lookup("D");
var c_message = C.create({
  a: {
    b: 10
  }
});
var d_message = D.create({});
d_message.c = c_message;
console.log(d_message);
buffer = D.encode(d_message).finish();
console.log(buffer.length, buffer);

The following jsfiddle snippet shows the error, printed to console
https://jsfiddle.net/ju7h93rr/

The followign jsfiddle snippet shows a modified version in which it works as expected
https://jsfiddle.net/2tvqLtLo/

@dcodeIO
Copy link
Member

dcodeIO commented Dec 26, 2016

Here, you are creating a runtime message for D, but using a plain object for C. Plain objects do not have virtual oneof properties so the encoder cannot efficiently know which field is present:

var d_message = D.create({
  c: {
    a: {
      b: 10
    }
  }
});

This should work:

var d_message = D.create({
  c: {
    a: {
      b: 10
    },
    child: "a"
  }
});

Or this:

var d_message = D.create({
  c: C.create({
    a: {
      b: 10
    }
  })
});

@kfir-drivenets
Copy link
Author

I just tested your version of the code and it seems to work just fine. 👍
Is there a documentation page that explains this behavior?
Also I think that some error should be raised if this case is detected and not just to return an empty buffer..

@dcodeIO
Copy link
Member

dcodeIO commented Dec 26, 2016

The empty buffer you see there might be related to #581 - or does this also happen with master?

@dcodeIO
Copy link
Member

dcodeIO commented Dec 26, 2016

Also, with master, it should be possible to use Message.from with (mixed) JSON data like in your case:

var d_message = D.from({
  c: {
    a: {
      b: 10
    }
  }
});

Related: #575

@kfir-drivenets
Copy link
Author

Checked on v6.3.0 tag and it seems to work.
Thanks 👍

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

No branches or pull requests

2 participants