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

OBJ Metadata Headers Json has the wrong format in the v2 Client. #458

Closed
niklasfp opened this issue Mar 29, 2024 · 3 comments · Fixed by #459
Closed

OBJ Metadata Headers Json has the wrong format in the v2 Client. #458

niklasfp opened this issue Mar 29, 2024 · 3 comments · Fixed by #459
Assignees

Comments

@niklasfp
Copy link
Contributor

niklasfp commented Mar 29, 2024

Observed behavior

After upgrading some object store code from .net v1 to .net v2 client we run into serialization bugs related to headers.

When trying to get information from a file stored (with headers) from nats cli or nats.net v1 the nats v2 client throws a json serialization exception (see exception below)

Also objects with headers added from the .nats v2 client cannot be read by nats cli:

❯ nats obj info HeaderTest FromV2_txt
nats: error: nats: object-store meta information invalid

And listing the same bucket hangs in the cli

❯ nats obj ls HeaderTest

Exception thrown

System.Text.Json.JsonException
  HResult=0x80131500
  Message=The JSON value could not be converted to System.String. Path: $.headers.abcdef | LineNumber: 0 | BytePositionInLine: 260.
  Source=System.Text.Json
  StackTrace:
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.Read[TValue](Utf8JsonReader& reader, JsonTypeInfo`1 jsonTypeInfo)
   at NATS.Client.Core.NatsJsonContextSerializer`1.Deserialize(ReadOnlySequence`1& buffer)
   at NATS.Client.Core.NatsJsonContextSerializer`1.NATS.Client.Core.INatsDeserialize<T>.Deserialize(ReadOnlySequence`1& buffer)
   at NATS.Client.ObjectStore.NatsObjStore.<GetInfoAsync>d__21.MoveNext()
   at System.Threading.Tasks.ValueTask`1.get_Result()
   at Program.<<Main>$>d__0.MoveNext() in C:\Source\Repos\NatsConnect\NatsConnect\Program.cs:line 32
   at Program.<<Main>$>d__0.MoveNext() in C:\Source\Repos\NatsConnect\NatsConnect\Program.cs:line 32
   at Program.<<Main>$>d__0.MoveNext() in C:\Source\Repos\NatsConnect\NatsConnect\Program.cs:line 32

  This exception was originally thrown at this call stack:
    [External Code]

Inner Exception 1:
InvalidOperationException: Cannot get the value of a token type 'StartArray' as a string.

The problem is that .nats .net v2 just adds headers as dictionary, where other clients adds them as a dictionary with a string key and of array of string value.

Object meta from object added with nats cli:

{"name":"FromCli_txt","headers":{"abcdef":["123456"]},"options":{"max_chunk_size":131072},"bucket":"HeaderTest","nuid":"iR25m0SOVnMT9BRS0DSn65","size":15,"mtime":"0001-01-01T00:00:00Z","chunks":1,"digest":"SHA-256=krdyOAo_jiepPlfm3uymwB2gf1qtzni7L7sg3hCmaSU="}

Object meta from object added with nats v2 cli:

{"name":"FromV2_txt","description":null,"bucket":"HeaderTest","nuid":"YiKN4TbWcLFuKYk0XZ7Fhq","size":13,"mtime":"2024-03-29T11:24:59.8718469+00:00","chunks":1,"digest":"SHA-256=3_1gIbsr1bCvZ2KQgJ7DpTGR3YHH9wpLKGiKNiGCmG8=","headers":{"abcdef":"123456"},"options":{"max_chunk_size":131072}}

Expected behavior

Nats v2 client can read objects with metadata headers added from other clients.
Nats v2 client can add objects with metadata headers compatible with other clients.

Server and client version

Server: 2.10.12
Cli: v0.1.4

Host environment

Windows 11
Nast server running in docker

Steps to reproduce

Create a bucket for testing

nats obj add HeaderTest

Add an object with headers

 "Hello, World!" > nats obj put HeaderTest --name="FromCli_txt" -H "abcdef:123456"

Run the following csharp code to verify that it breaks.

using NATS.Client.Core;
using NATS.Client.JetStream;
using NATS.Client.ObjectStore;
using NATS.Client.ObjectStore.Models;

const string BucketName = "HeaderTest";
const string FromCliFileName = "FromCli_txt";
const string FromNatsV2FileName = "FromV2_txt";


await using var conn = new NatsConnection();

var storeContext = new NatsObjContext(new NatsJSContext(conn));
var store = await storeContext.GetObjectStoreAsync(BucketName);

var objMeta = new ObjectMetadata()
{
    Name = FromNatsV2FileName,
    Bucket = BucketName,
    Headers = new Dictionary<string, string>()
    {
        { "abcdef", "123456" }
    },

};

// Put object (puts object with headers bad json)
await using var stream = new MemoryStream("Hello, World!"u8.ToArray());
await store.PutAsync(objMeta, stream);

// Get object added from CLI (fails)
var info = await store.GetInfoAsync(FromCliFileName);

After running the csharp code, run the following cli command

nats obj info HeaderTest FromV2_txt

will fail with nats: error: nats: object-store meta information invalid

@mtmk mtmk self-assigned this Mar 29, 2024
@mtmk
Copy link
Collaborator

mtmk commented Mar 29, 2024

thanks @niklasfp. Found the issue. as you probably guessed it was the Headers which ought to be string,string[] rather than string,string.

@niklasfp
Copy link
Contributor Author

niklasfp commented Mar 29, 2024

@mtmk when this is fixed and released, it might break for people that only used the v2 client to add headers to objects. Not a problem for us, our problem is that the headers are in the correct format (not v2 compatible) 😉

@mtmk
Copy link
Collaborator

mtmk commented Mar 29, 2024

yes it's going to be a breaking change unfortunately. as you said, if people have data (with headers) saved and used only with this client, data has to be migrated 😞

@mtmk mtmk linked a pull request Mar 29, 2024 that will close this issue
@mtmk mtmk closed this as completed in #459 Apr 2, 2024
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