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

ulong correlation_id incorrecly serialized as uuid #394

Closed
woppa684 opened this issue Jan 25, 2023 · 3 comments · Fixed by #396
Closed

ulong correlation_id incorrecly serialized as uuid #394

woppa684 opened this issue Jan 25, 2023 · 3 comments · Fixed by #396

Comments

@woppa684
Copy link

My backend sends a ulong for correlation_id, which is fine according to the spec.

When the ulong is decoded it returns a Number when possible and a Buffer otherwise.

rhea/lib/types.js

Lines 228 to 236 in e826e8b

function read_ulong(buffer, offset) {
var hi = buffer.readUInt32BE(offset);
var lo = buffer.readUInt32BE(offset + 4);
if (hi < 2097153) {
return hi * MAX_UINT + lo;
} else {
return buffer.slice(offset, offset + 8);
}
}

So far so good, I can cope with these differences.

The problem occurs when I try to pass that same correlation_id to a new message (as is the most useful thing to do of course). Since, for big numbers, I'm passing a Buffer, it is encoded as a Uuid, which is different from the ulong I wanted!

rhea/lib/types.js

Lines 490 to 502 in e826e8b

types.wrap_message_id = function(o) {
var t = typeof o;
if (t === 'string') {
return types.wrap_string(o);
} else if (t === 'number' || o instanceof Number) {
return types.wrap_ulong(o);
} else if (Buffer.isBuffer(o)) {
return types.wrap_uuid(o);
} else {
//TODO handle uuids
throw new errors.TypeError('invalid message id:' + o);
}
};

To me this is a bug. the only way I can now send a correlation_id of type ulong from the frontend is by making it a Number, which is too small... And sending back correlation_id's from my backend can just fail depending on what value it sends.

@grs
Copy link
Member

grs commented Jan 25, 2023

I agree, that is a bug. I've checked in a fix that will allow you to pre-wrap the value as the type you wish. E.g.

response.correlation_id = rhea.types.wrap_ulong(request.correlation_id);

This isn't perfect as it does require that you determine whether or not you need to do this, but hopefully it at least provides a workaround.

@woppa684
Copy link
Author

Thanks! Would it also be possible to make a release with this fix?

@grs
Copy link
Member

grs commented Jan 26, 2023

Would it also be possible to make a release with this fix?

Released as 3.0.2

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

Successfully merging a pull request may close this issue.

2 participants