Azure / azure-libraries-for-net

Azure libraries for .Net
MIT License
379 stars 192 forks source link

Setting TXT entries with value lengths greater than 255 fails with ArgumentOutOfRangeException #1296

Closed tskarman closed 2 years ago

tskarman commented 2 years ago

First, why a fix is important:

Now for the bug:

To be exact: it fails whenever value.Length > 255 and value.Length % 255 != 0

It fails because of this line: https://github.com/Azure/azure-libraries-for-net/blob/2aeabc281334f3edb6483ef697a1db54ff97c621/src/ResourceManagement/Dns/DnsRecordSetImpl.cs#L475

A fixed version would take into account the remaining string length - similar to this:

    value.AddRange(Enumerable.Range(0, (int)(Math.Ceiling(text.Length / (double)255))).Select(i => text.Substring(i * 255, Math.Min(255, text.Length - i * 255)));

Here is a repro:

var sb = new StringBuilder();
sb.Append('a', 255);
sb.Append('b', 255); // not required to showcase the exception, but it's important for a complete fix
sb.Append('c', 1);
var text = sb.ToString();
// This throws, demonstrating the reported issue
var result = Enumerable.Range(0, (int)(Math.Ceiling(text.Length / (double)255))).Select(i => text.Substring(i * 255, 255)).ToList();

This shows the desired behavior and result:

var sb = new StringBuilder();
sb.Append('a', 255);
sb.Append('b', 255);
sb.Append('c', 1);
var text = sb.ToString();
var result = Enumerable.Range(0, (int)(Math.Ceiling(text.Length / (double)255))).Select(i => text.Substring(i * 255, Math.Min(255, text.Length - i * 255)));

// Visual confirmation of the result
Console.WriteLine(string.Join("\n", result));
weidongxu-microsoft commented 2 years ago

@tskarman

Thanks for the detailed report. I will check the bug. However the SDK is in maintenance mode and might not get any release soon.

User is recommended to use https://aka.ms/azsdk/dotnet/mgmt

tskarman commented 2 years ago

@tskarman

Thanks for the detailed report. I will check the bug. However the SDK is in maintenance mode and might not get any release soon.

User is recommended to use https://aka.ms/azsdk/dotnet/mgmt

Many thanks for the quick reaction. One question regarding your last statement:

I had a look at the new libraries but to me it looked like the public DNS management was left out of the recent release rounds. All I could find was Azure.ResourceManager.Dns in 1.0.0-prevew.1 from 2020-09-24 (https://www.nuget.org/packages/Azure.ResourceManager.Dns/)

The Azure.ResourceManager.Network in 1.0.0-beta.5 contains only types dealing with PrivateDns (https://www.nuget.org/packages/Azure.ResourceManager.Network/).

The documentation is telling a similar story: https://azure.github.io/azure-sdk/releases/latest/all/dotnet.html?search=dns

Or am i overlooking something - is the non-private DNS handled by the private DNS APIs in some fashion?

weidongxu-microsoft commented 2 years ago

@tskarman

You are right, DNS in new SDK is not yet released. My mistake.

It should be released starting at Azure.ResourceManager.Dns 1.0.0-beta.1. Please ignore preview.#

It should happen within a month. I will update you when I hear it released.

tskarman commented 2 years ago

That was quick - thank you!

Will there be a NuGet release for this - if so, any time estimate on that? (https://www.nuget.org/packages/Microsoft.Azure.Management.Dns.Fluent/)

weidongxu-microsoft commented 2 years ago

@tskarman

Next release of this lib is still not scheduled.

Azure.ResourceManager.Dns could take longer than expected. You can use https://www.nuget.org/packages/Microsoft.Azure.Management.Dns if urgent, but note that this lib will be replaced by Azure.ResourceManager.Dns as well.

tskarman commented 2 years ago

OK, thanks - works for me.

If anyone requires a workaround. Here is what I am currently employing. It follows the above suggestion to use the non-fluent core implementation (or at least the more low-level fluent wrappers/generated types).

if (tte.Value.Length > 255)
{
    // _zone is: Azure.Authenticate(_azureAuthFilePath).WithDefaultSubscription().DnsZones.GetByResourceGroupAsync(_resourceGroup, _zoneName)
    var rs = _zone.Manager.Inner.RecordSets;
    RecordSetInner rs2;
    try
    {
        rs2 = await rs.GetAsync(_resourceGroup, _zoneName, tte.Name, RecordType.TXT);
    }
    catch (CloudException) // Taking this as a proxy for "there is no matching TXT record"
    {
        rs2 = new RecordSetInner();
        rs2.TxtRecords = new List<TxtRecord>();
        rs2.TTL = tte.TTL;
    }
    var text = tte.Value;
    var result = Enumerable.Range(0, (int)(Math.Ceiling(text.Length / (double)255)))
        .Select(i => text.Substring(i * 255, Math.Min(255, text.Length - i * 255)))
        .ToList();

    if (rs2.TxtRecords.Count == 0)
    {
    }
    else if (rs2.TxtRecords.Count == 1)
    {
        rs2.TxtRecords.Clear();
    }
    else
    {
        throw new ArgumentException($"Found multiple TXT records for {tte.Name} in {_zoneName}.", nameof(entries));
    }

    rs2.TxtRecords.Add(new TxtRecord(result));

    var result2 = await rs.CreateOrUpdateAsync(_resourceGroup, _zoneName, tte.Name, RecordType.TXT, rs2, rs2.Etag);
}
else
{
    // do what ever you normally do
}