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.

Sign up for GitHub

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

Jump to bottom

[JS] direct get apis include old Nats- headers creating duplicates #4573

Open
aricart opened this issue Sep 22, 2023 · 0 comments · May be fixed by #4618
Open

[JS] direct get apis include old Nats- headers creating duplicates #4573

aricart opened this issue Sep 22, 2023 · 0 comments · May be fixed by #4618
Labels
defect Suspected defect such as a bug or regression

Comments

@aricart
Copy link
Member

aricart commented Sep 22, 2023 •
edited

What version were you using?

2.10.1

What environment was the server running in?

any

Is this defect reproducible?

To reproduce the error:

Save this program to direct_issue.ts

import { parse } from "https://deno.land/std@0.202.0/flags/mod.ts";
import { connect, StorageType, nuid } from "https://raw.githubusercontent.com/nats-io/nats.deno/main/src/mod.ts";
import { JetStreamManagerImpl } from 'https://raw.githubusercontent.com/nats-io/nats.deno/main/jetstream/jsm.ts'
import { MsgHdrsImpl } from 'https://raw.githubusercontent.com/nats-io/nats.deno/main/nats-base-client/headers.ts'

const opts = parse(Deno.args);

if (opts.h || opts.help) {
  console.log("deno run -A direct_issue.ts --server hostport --help");
}
const servers = opts.server || opts.s || "localhost:4222";

const nc = await connect({ servers });

const jsm = await nc.jetstreamManager() as JetStreamManagerImpl;

const name = nuid.next();


await jsm.streams.add({
  name: name,
  subjects: ["A.>"],
  storage: StorageType.Memory,
  republish: {
    src: ">",
    dest: `$KV.${name}.>`,
  },
});

const js = nc.jetstream();
const B = await js.views.kv(name);

nc.publish("A.orange", "hey");
await js.publish("A.tomato", "hello");
await B.put("A.potato", "yo");

console.log("direct gets");
for (let i = 1; i <= 3; i++) {
  const m = await jsm.direct.getMessage(`KV_${name}`, { seq: i });
  console.log((m.header as MsgHdrsImpl).toString());
  console.log("-------------");
}
console.log("stream gets");
for (let i = 1; i <= 3; i++) {
  const m = await jsm.streams.getMessage(`KV_${name}`, { seq: i });
  console.log((m.header as MsgHdrsImpl).toString());
  console.log("-------------");
}

await jsm.streams.delete(name);
await B.destroy()
await nc.close()

Start a nats server with JetStream

> deno run -A direct_issue.ts

direct gets
NATS/1.0
Nats-Stream: Z0EZD0S7VBF6EL9SL4SN5I
Nats-Stream: KV_Z0EZD0S7VBF6EL9SL4SN5I
Nats-Subject: A.orange
Nats-Subject: $KV.Z0EZD0S7VBF6EL9SL4SN5I.A.orange
Nats-Sequence: 1
Nats-Sequence: 1
Nats-Last-Sequence: 0
Nats-Time-Stamp: 2023-09-22T11:35:36.861641Z


-------------
NATS/1.0
Nats-Stream: Z0EZD0S7VBF6EL9SL4SN5I
Nats-Stream: KV_Z0EZD0S7VBF6EL9SL4SN5I
Nats-Subject: A.tomato
Nats-Subject: $KV.Z0EZD0S7VBF6EL9SL4SN5I.A.tomato
Nats-Sequence: 2
Nats-Sequence: 2
Nats-Last-Sequence: 0
Nats-Time-Stamp: 2023-09-22T11:35:36.86172Z


-------------
NATS/1.0
Nats-Stream: KV_Z0EZD0S7VBF6EL9SL4SN5I
Nats-Subject: $KV.Z0EZD0S7VBF6EL9SL4SN5I.A.potato
Nats-Sequence: 3
Nats-Time-Stamp: 2023-09-22T11:35:36.862229Z


-------------
stream gets
NATS/1.0
Nats-Stream: Z0EZD0S7VBF6EL9SL4SN5I
Nats-Subject: A.orange
Nats-Sequence: 1
Nats-Last-Sequence: 0


-------------
NATS/1.0
Nats-Stream: Z0EZD0S7VBF6EL9SL4SN5I
Nats-Subject: A.tomato
Nats-Sequence: 2
Nats-Last-Sequence: 0
-------------

Given the capability you are leveraging, describe your expectation?

The headers from the added by the stream republish onboarding into KV shouldn't be reported. The side effect of that is Get operations trying to figure out the name of the key will fail, as there are multiple Nats-Subject entries (as well as sequence etc.).

Nats-Subject: A.tomato
Nats-Subject: $KV.1RHFTJKE86C02I6P51LXR3.A.tomato

For KV implementations, there's a check on the subject header - depending on what the map implementation does, it may get one or the other header (go gets the second). The algorithm will remove the prefix, which then yields a key that is different from what the client is looking for resulting in a not found error on the lookup.

Given the expectation, what is the defect you are observing?

Nats- headers shouldn't have duplicates.
@aricart aricart added the defect Suspected defect such as a bug or regression label Sep 22, 2023
@aricart aricart mentioned this issue Sep 22, 2023
Closed
aricart added a commit that referenced this issue Oct 3, 2023
@aricart
[FIX] direct get APIs can contain duplicate Nats-* headers, because…
34c9222 
… it simply appends JSON bytes to existing headers. If the message was onboarded on a republish, this will contain system headers. Since headers are not ordered and can contain multiple values for the same header, this can break KV clients as well as create ambiguity on the stream that yielded the message.

Fix #4573
@aricart aricart linked a pull request Oct 3, 2023 that will close this issue
Open
7 tasks
aricart added a commit that referenced this issue Oct 3, 2023
@aricart
[FIX] direct get APIs can contain duplicate Nats-* headers, because…
7c12741 
… it simply appends JSON bytes to existing headers. If the message was onboarded on a republish, this will contain system headers. Since headers are not ordered and can contain multiple values for the same header, this can break KV clients as well as create ambiguity on the stream that yielded the message.

Fix #4573
aricart added a commit that referenced this issue Oct 3, 2023
@aricart
[FIX] direct get APIs can contain duplicate Nats-* headers, because…
2916bb0 
… it simply appends JSON bytes to existing headers. If the message was onboarded on a republish, this will contain system headers. Since headers are not ordered and can contain multiple values for the same header, this can break KV clients as well as create ambiguity on the stream that yielded the message.

Fix #4573
aricart added a commit that referenced this issue Oct 3, 2023
@aricart
[FIX] direct get APIs can contain duplicate Nats-* headers, because…
36b4e2d 
… it simply appends JSON bytes to existing headers. If the message was onboarded on a republish, this will contain system headers. Since headers are not ordered and can contain multiple values for the same header, this can break KV clients as well as create ambiguity on the stream that yielded the message.

Fix #4573
aricart added a commit that referenced this issue Oct 3, 2023
@aricart
[FIX] direct get APIs can contain duplicate Nats-* headers, because…
b7b341b 
… it simply appends JSON bytes to existing headers. If the message was onboarded on a republish, this will contain system headers. Since headers are not ordered and can contain multiple values for the same header, this can break KV clients as well as create ambiguity on the stream that yielded the message.

Fix #4573
aricart added a commit that referenced this issue Oct 4, 2023
@aricart
[FIX] direct get APIs can contain duplicate Nats-* headers, because…
7138157 
… it simply appends JSON bytes to existing headers. If the message was onboarded on a republish, this will contain system headers. Since headers are not ordered and can contain multiple values for the same header, this can break KV clients as well as create ambiguity on the stream that yielded the message.

Fix #4573
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[FIX] remove extra system headers on direct gets nats-io/nats-server
1 participant
@aricart