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

capnp schema version mismatch #78

Closed
smoreinis opened this issue Nov 13, 2017 · 4 comments
Closed

capnp schema version mismatch #78

smoreinis opened this issue Nov 13, 2017 · 4 comments

Comments

@smoreinis
Copy link

Hi!

I've got a question about how capnp-ts deals with fields being added to the underlying capnp schema.
I had a struct with two fields (a text field and a pointer to another struct), which was part of a larger message that I serialized into bytes using the capnp-ts library.
I then added two more (text) fields at the end of this struct (with tags 2 and 3), regenerated the .capnp.js implementations, and tried to read these new fields from the previously serialized bytes.

The result was
Error: CAPNP-TS024 Attempted to convert pointer Pointer_1@0x00000020,[f4 ff ff ff 00 00 04 00],limit:0x7ffffffa to a 1 type.
and digging into it a little bit more it seemed like https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/serialization/pointers/pointer.ts#L895 was getting a pointer type of 0 when it was expecting a pointer type of 1.

I see that https://github.com/jdiaz5513/capnp-ts/blob/master/packages/capnp-ts/src/serialization/pointers/struct.ts#L603 acknowledges the struct size mismatch might happen, but to the best of my understanding the "hole" of zeroes in the message also zeroes out the pointer types which causes validation to fail if any other pointer type is expected.

Please let me know if there's any more information that would be helpful here or if there's a better way to deal with this situation.

Thanks!

@jdiaz5513
Copy link
Owner

Can you attach the following:

  • first version of capnp file
  • second (failing) version of capnp file
  • dumped message

Please scrub any potentially sensitive information.

@smoreinis
Copy link
Author

I'd rather not share the exact capnp schema I'm working on so I made a simple example to reproduce the issue I've been seeing. Although I initially reported an issue trying to read the newly added field in the new schema version, below I've shown issues even trying to read the old field in the new schema version (which seem related). Let me know if there's anything else I can help with!

Here's foo.capnp (first version):

@0xe0b7ff464fbc7ee1;
struct Foo {
  bar @0 :Text;
}

Here's foo_new.capnp (with a new text field added at the end):

@0xe0b7ff464fbc7ee1;
struct Foo {
  bar @0 :Text;
  baz @1 :Text;
}

Here's some failing code which tries to serialize an old foo message and deserialize it as the new foo message:

  const OldFoo = require('./foo.capnp.js').Foo
  const NewFoo = require('./foo_new.capnp.js').Foo

  const oldMessage = new capnp.Message()
  const oldFoo = oldMessage.initRoot(OldFoo)
  oldFoo.setBar("bar")
  const packed = Buffer.from(oldMessage.toPackedArrayBuffer())
  console.log(packed.toString('base64'))

  const newMessage = capnp.Message.fromPackedBuffer(packed)
  const newFoo = newMessage.getRoot(NewFoo)
  console.log(newFoo.getBar())

The serialized packed message in base64 is EANAAREBIgdiYXI= and I get an error trying to read the old field from the new message: Error: CAPNP-TS024 Attempted to convert pointer Pointer_1@0x00000010,[fc ff ff ff 00 00 02 00],limit:0x7ffffffe to a 1 type. although this doesn't seem to trigger the struct resize I mentioned originally.

Here's a slightly more complicated foo.capnp (first version):

@0xe0b7ff464fbc7ee1;
struct Foo {
  bar @0 :Bar;
}
struct Bar {
  one @0 :Text;
}

Here's foo_new.capnp (with a new text field added at the end):

@0xe0b7ff464fbc7ee1;
struct Foo {
  bar @0 :Bar;
}
struct Bar {
  one @0 :Text;
  two @1 :Text;
}

And here's some failing code which tries to serialize an old foo message and deserialize it as the new foo message:

  const OldFoo = require('./foo.capnp.js').Foo
  const NewFoo = require('./foo_new.capnp.js').Foo

  const oldMessage = new capnp.Message()
  const oldFoo = oldMessage.initRoot(OldFoo)
  oldFoo.getBar().setOne("one")
  const packed = Buffer.from(oldMessage.toPackedArrayBuffer())
  console.log(packed.toString('base64'))

  const newMessage = capnp.Message.fromPackedBuffer(packed)
  const newFoo = newMessage.getRoot(NewFoo)
  console.log(newFoo.getBar().getOne())

The serialized packed message in base64 is EARAAUABEQEiB29uZQ== and this does trigger the struct resize but fails again when trying to read the old field: Error: CAPNP-TS024 Attempted to convert pointer Pointer_1@0x00000010,[fc ff ff ff 00 00 02 00],limit:0x7ffffffd to a 1 type.

@jdiaz5513
Copy link
Owner

jdiaz5513 commented Nov 15, 2017 via email

@jdiaz5513
Copy link
Owner

Both of these cases trigger pathological far pointer behavior, which admittedly was poorly tested.

Patch incoming along with a pretty big refactor.

cmbartschat added a commit to 8thwall/capnp-ts that referenced this issue Sep 24, 2018
Fixes issue similar to jdiaz5513#78 (perhaps same) where a sender doesn't set a text field in the source proto can cause a unexpected validation error on the receiver when parsing the proto message.
What appears to happen is a null source pointer is dereferenced and the data is assigned to the destination pointer in the resize function.
We are able to repro this consistently when parsing large nested proto messages when presumably the dereferenced data in the source segment happens to contain nonzero data.
jdiaz5513 pushed a commit that referenced this issue Sep 26, 2018
…size (#107)

Fixes issue similar to #78 (perhaps same) where a sender doesn't set a text field in the source proto can cause a unexpected validation error on the receiver when parsing the proto message.

What appears to happen is a null source pointer is dereferenced and the data is assigned to the destination pointer in the resize function.
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