consuldotnet icon indicating copy to clipboard operation
consuldotnet copied to clipboard

KV.Release is writing a wrong value

Open pablonardone opened this issue 3 years ago • 3 comments
trafficstars

Describe the bug

When using KV.Release to release a previously acquired lock on an item, the Release funcion always write the value bnVsbA== to the item, which corresponds to the base64 encoding of the text null.

This is happening because the Release function is constructing a request whith the form PutRequest<object, bool> and passing null as the body parameter.

Then when the body is serialized, it is serialized as System.Text.Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(value)), so the result is always bnVsbA==.

To correct this issue the request must be constructed whith the form PutRequest<byte[], bool> and passing the correct value in the body parameter, the same way that it's used in the Acquire request.

Steps To Reproduce

.

Expected behavior

.

Environment

  • OS: Windows 10
  • Consul Version: 1.6.10.4

Logs

No response

Additional context

No response

pablonardone avatar Mar 16 '22 15:03 pablonardone

In order to resolve this issue it would be enough to modify the KV.Release functions in the following way:

The current functions is:

      public Task<WriteResult<bool>> Release(KVPair p, WriteOptions q, CancellationToken ct = default(CancellationToken))
      {
          p.Validate();
          var req = _client.Put<object, bool>(string.Format("/v1/kv/{0}", p.Key.TrimStart('/')), null, q);
          if (p.Flags > 0)
          {
              req.Params["flags"] = p.Flags.ToString();
          }
          req.Params["release"] = p.Session;
          return req.Execute(ct);
      }

And the new function it would be:

    public Task<WriteResult<bool>> Release(KVPair p, WriteOptions q, CancellationToken ct = default(CancellationToken))
    {
        p.Validate();
        var req = _client.Put<byte[], bool>(string.Format("/v1/kv/{0}", p.Key.TrimStart('/')), p.Value, q);
        if (p.Flags > 0)
        {
            req.Params["flags"] = p.Flags.ToString();
        }
        req.Params["release"] = p.Session;
        return req.Execute(ct);
    }

pablonardone avatar Mar 16 '22 15:03 pablonardone

Hi @pablonardone,

Thanks for the detailed report.

Looks like you got the bug and accompanying fix figured out already, would you like to contribute a PR?

mfkl avatar Mar 17 '22 06:03 mfkl

When using KV.Release to release a previously acquired lock on an item, the Release funcion always write the value bnVsbA== to the item, which corresponds to the base64 encoding of the text null.

It'd be useful to have a unit test for this.

mfkl avatar Jul 22 '22 08:07 mfkl