You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by "ngavalas (via GitHub)" <gi...@apache.org> on 2023/05/11 23:26:45 UTC

[GitHub] [thrift] ngavalas commented on a diff in pull request #2805: Stop sharing headers across all instances of transports

ngavalas commented on code in PR #2805:
URL: https://github.com/apache/thrift/pull/2805#discussion_r1191774635


##########
lib/nodejs/test/header.test.js:
##########
@@ -39,6 +39,19 @@ const cases = {
     assert.equals(headers.Trace, "abcde");
     assert.end();
   },
+  "Should read different headers from different payload": function(assert) {
+    const transport = new TFramedTransport();
+    const buf = Buffer.from(headerPayload);
+    buf[24] = 115; // Change Parent to Parens
+    buf[32] = 122; // Change shoobar to shoobaz
+    transport.inBuf = buf;
+
+    const headers = transport.readHeaders();
+    assert.equals(headers.Parent, undefined);

Review Comment:
   Just to be 100% clear, before this fix, `headers` would have contained `{ Parent: "shoobar", Parens: "shoobaz:", Trace: "abcde" }` because it was reusing the same object as the first test. This is not test-only though and happens in real life.
   
   Similar story for the write test.



##########
lib/nodejs/test/header.test.js:
##########
@@ -39,6 +39,19 @@ const cases = {
     assert.equals(headers.Trace, "abcde");
     assert.end();
   },
+  "Should read different headers from different payload": function(assert) {
+    const transport = new TFramedTransport();
+    const buf = Buffer.from(headerPayload);
+    buf[24] = 115; // Change Parent to Parens
+    buf[32] = 122; // Change shoobar to shoobaz
+    transport.inBuf = buf;
+
+    const headers = transport.readHeaders();
+    assert.equals(headers.Parent, undefined);

Review Comment:
   Just to be 100% clear, before this fix, `headers` would have contained `{ Parent: "shoobar", Parens: "shoobaz", Trace: "abcde" }` because it was reusing the same object as the first test. This is not test-only though and happens in real life.
   
   Similar story for the write test.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org