jcasale/LdifHelper

Wrap Extension method may exit loop prematurely.

Closed this issue · 1 comments

The Wrap extension method will exit prematurely in some cases (depending on string length). Example with possible fix:

using System;
using System.Text;

namespace WrapIt
{
    class Program
    {
        static void Main(string[] args)
        {
            var generator = new Random();
            var data = new byte[3763];
            generator.NextBytes(data);
            var s = data.ToBase64();
            Console.WriteLine($"notWrapped:: {s}");
            Console.WriteLine($"wrap:: {s}".Wrap());
            Console.WriteLine($"wrapNew:: {s}".WrapNew());
        }
    }

    public static class Constants
    {
        public static int MaxLineLength { get; } = 76;
    }

    public static class Extensions
    {
        public static string ToBase64(this byte[] b)
        {
            return Convert.ToBase64String(b);
        }

        /// <summary>
        /// Wraps lines according to RFC2849 note 10 compliance.
        /// </summary>
        /// <param name="value">The combined attribute type and value line.</param>
        /// <returns>A wrapped string if required.</returns>
        public static string Wrap(this string value)
        {
            if (value == null)
            {
                throw new ArgumentNullException(nameof(value));
            }

            StringBuilder stringBuilder = new StringBuilder();

            int startIndex = 0;

            for (int i = 0; i < value.Length; i += Constants.MaxLineLength)
            {
                if (i > 0)
                {
                    // Subtract 1 from all but the first line to accommodate leading space.
                    stringBuilder.AppendFormat("{0} ", Environment.NewLine);
                    stringBuilder.Append(value.Substring(startIndex, Math.Min(Constants.MaxLineLength - 1, value.Length - startIndex)));
                    startIndex += Constants.MaxLineLength - 1;
                }
                else
                {
                    stringBuilder.Append(value.Substring(startIndex, Math.Min(Constants.MaxLineLength, value.Length - startIndex)));
                    startIndex += Constants.MaxLineLength;
                }
            }

            return stringBuilder.ToString();
        }

        /// <summary>
        /// Wraps lines according to RFC2849 note 10 compliance.
        /// </summary>
        /// <param name="value">The combined attribute type and value line.</param>
        /// <returns>A wrapped string if required.</returns>
        public static string WrapNew(this string value)
        {
            if (value == null)
            {
                throw new ArgumentNullException(nameof(value));
            }

            StringBuilder stringBuilder = new StringBuilder();

            var startIndex = 0;
            var maxLen = value.Length;
            var wrapLen = Constants.MaxLineLength - 1;
            while(startIndex < maxLen)
            {
                int len = 0;
                if (startIndex > 0)
                {
                    // Subtract 1 from all but the first line to accommodate leading space.
                    stringBuilder.AppendFormat("{0} ", Environment.NewLine);
                    len = Math.Min(wrapLen, maxLen - startIndex);
                }
                else
                {
                    len = Math.Min(Constants.MaxLineLength, maxLen - startIndex);
                }
                stringBuilder.Append(value.Substring(startIndex, len));
                startIndex += len;
            }

            return stringBuilder.ToString();
        }
    }
}

Looking at the logic in the original Wrap code, I'm saddened that made it in and has not manifested yet. Brilliant catch, I'll update it with your proposal as soon as I can get a test written.

Thanks.