github-actions[bot] commented on code in PR #4363: URL: https://github.com/apache/iggy/pull/4363#discussion_r4161218993
########## foreign/csharp/Iggy_SDK/Configuration/ConnectionString.cs: ########## @@ -0,0 +1,250 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +using System.Globalization; +using System.Text.RegularExpressions; +using Apache.Iggy.Enums; +using Apache.Iggy.Utils; + +namespace Apache.Iggy.Configuration; + +// Parsed by hand because Uri escaping and host normalization accept strings this format rejects. +internal static class ConnectionString +{ + private const string DefaultScheme = "iggy"; + private const string TcpScheme = "iggy+tcp"; + private const string SchemePrefix = "iggy+"; + + // Unbracketed hosts may not contain ':', so 2001:db8::1:8090 and host:1:2 are rejected. + private static readonly Regex AuthorityPattern = new(@"^(?:\[([^\]\s\p{Cc}]+)\]|([^:\[\]\s\p{Cc}]+)):([0-9]+)\z", + RegexOptions.CultureInvariant); + + internal static IggyClientConfigurator Parse(string connectionString) + { + ArgumentNullException.ThrowIfNull(connectionString); + + var protocolParts = connectionString.Split("://"); + if (protocolParts.Length != 2) + { + throw Invalid(); + } + + ValidateScheme(protocolParts[0]); + + var parts = protocolParts[1].Split('@'); + if (parts.Length != 2) + { + throw Invalid(); + } + + var autoLoginSettings = ParseCredentials(parts[0]); + + var serverAndOptions = parts[1].Split('?'); + if (serverAndOptions.Length > 2) + { + throw Invalid(); + } + + var serverAddress = serverAndOptions[0]; + var serverHost = ParseHost(serverAddress); + var (tlsSettings, reconnectionSettings, heartbeatInterval) = + ParseOptions(serverAndOptions.ElementAtOrDefault(1), serverHost); + + return new IggyClientConfigurator + { + BaseAddress = serverAddress, + Protocol = Protocol.Tcp, + HeartbeatInterval = heartbeatInterval, + TlsSettings = tlsSettings, + ReconnectionSettings = reconnectionSettings, + AutoLoginSettings = autoLoginSettings + }; + } + + private static void ValidateScheme(string scheme) + { + if (scheme == DefaultScheme || scheme == TcpScheme) + { + return; + } + + if (scheme.Equals(DefaultScheme, StringComparison.OrdinalIgnoreCase) + || scheme.Equals(TcpScheme, StringComparison.OrdinalIgnoreCase)) + { + throw new FormatException( + $"Connection string schemes are case-sensitive, use \"{DefaultScheme}\" or \"{TcpScheme}\"."); + } + + // Only a plain transport name is echoed, a malformed scheme may carry credentials. + if (scheme.StartsWith(SchemePrefix, StringComparison.Ordinal) + && scheme.Length > SchemePrefix.Length + && scheme[SchemePrefix.Length..].All(char.IsAsciiLetter)) + { + throw new FormatException( + $"Unsupported transport \"{scheme[SchemePrefix.Length..]}\", connection strings support tcp only."); + } + + throw Invalid(); + } + + private static AutoLoginSettings ParseCredentials(string userInfo) + { + var credentials = userInfo.Split(':'); + if (credentials.Length > 2 || credentials.Any(string.IsNullOrEmpty)) + { + throw Invalid(); + } + + return credentials.Length == 1 + ? AutoLoginSettings.ForPersonalAccessToken(credentials[0]) + : AutoLoginSettings.For(credentials[0], credentials[1]); + } + + private static string ParseHost(string serverAddress) + { + var addressMatch = AuthorityPattern.Match(serverAddress); + if (!addressMatch.Success || !ushort.TryParse(addressMatch.Groups[3].Value, NumberStyles.None, + CultureInfo.InvariantCulture, out _)) + { + throw Invalid(); + } + + return addressMatch.Groups[1].Success + ? addressMatch.Groups[1].Value + : addressMatch.Groups[2].Value; + } + + private static (TlsSettings, ReconnectionSettings, TimeSpan) ParseOptions(string? query, string serverHost) + { + // The certificate name check needs a hostname when tls_domain is missing. + var tlsSettings = new TlsSettings { Hostname = serverHost }; + var reconnectionSettings = new ReconnectionSettings + { + Enabled = true, + MaxRetries = 0, + InitialDelay = TimeSpan.FromSeconds(1), + UseExponentialBackoff = false + }; + var heartbeatInterval = TimeSpan.FromSeconds(5); + + foreach (var option in query?.Split('&') ?? []) + { + var (name, value) = SplitOption(option); + switch (name) + { + case "tls": + tlsSettings.Enabled = ParseBoolean(name, value); + break; + case "tls_domain": + tlsSettings.Hostname = value.Length == 0 ? serverHost : value; + break; + case "tls_ca_file": + tlsSettings.CertificatePath = value; + break; + case "reconnection_retries": + ApplyReconnectionRetries(reconnectionSettings, name, value); + break; + case "reconnection_interval": + reconnectionSettings.InitialDelay = ParseInterval(name, value); + break; + case "reestablish_after": + // Validated only, reconnection_interval paces every redial. + ParseDuration(name, value, IggyDurationParser.ParseParts); + break; + case "heartbeat_interval": + heartbeatInterval = ParseInterval(name, value); + break; + case "nodelay": + // Validated only, the TCP client always disables Nagle. + ParseBoolean(name, value); + break; + default: + throw new FormatException($"Unknown option \"{name}\"."); + } + } + + if (tlsSettings.Enabled && string.IsNullOrEmpty(tlsSettings.CertificatePath)) + { + throw new FormatException("Option \"tls\" requires \"tls_ca_file\"."); + } + + return (tlsSettings, reconnectionSettings, heartbeatInterval); + } + + private static (string Name, string Value) SplitOption(string option) + { + var optionParts = option.Split('='); + if (optionParts.Length != 2) + { + throw Invalid(); + } + + return (optionParts[0], optionParts[1]); + } + + private static void ApplyReconnectionRetries(ReconnectionSettings settings, string name, string value) + { + var unlimited = value == "unlimited"; + uint retries = 0; + if (!unlimited && !uint.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out retries)) + { + throw new FormatException($"Option \"{name}\" must be \"unlimited\" or an integer up to {uint.MaxValue}."); + } + + // MaxRetries treats 0 as unlimited, so zero retries means reconnection off. + settings.Enabled = unlimited || retries != 0; + settings.MaxRetries = retries <= int.MaxValue ? (int)retries : 0; Review Comment: warning: `ApplyReconnectionRetries` accepts counts up to 4294967295 but turns any count above `int.MaxValue` into `MaxRetries = 0`, which the reconnect loop reads as unlimited retries. Reject counts above `int.MaxValue` instead of reinterpreting them. ########## foreign/csharp/README.md: ########## @@ -148,6 +148,50 @@ await client.ConnectAsync(); TCP applies the connection, heartbeat, TLS, and auto-login settings. HTTP `ConnectAsync` does no network work and requires explicit login. For TLS configuration, see the [TcpTls example](../../examples/csharp/README.md#tcptls). +### Connection String + +A TCP client can also be created from the connection string format shared with the other SDKs. The +credentials become the auto-login settings, so the client signs in on `ConnectAsync`: + +```c# +using var client = IggyClientFactory.CreateClient("iggy://iggy:[email protected]:8090"); +await client.ConnectAsync(); + +// A personal access token instead of a username and password, plus options +using var tokenClient = IggyClientFactory.CreateClient( + "iggy+tcp://[email protected]:8090?heartbeat_interval=10s&reconnection_retries=5"); + +// Parse into a configurator to adjust the remaining settings +var config = IggyClientConfigurator.FromConnectionString("iggy://iggy:[email protected]:8090"); +config.LoggerFactory = loggerFactory; +``` + +| Option | Default | Description | +|-------------------------|-------------|--------------------------------------------------------------------------------| +| `tls` | `false` | `true` or `false` | +| `tls_domain` | server host | Server name for the TLS handshake, used for every node the client dials | +| `tls_ca_file` | empty | CA certificate path, read when connecting, required with `tls=true` | +| `reconnection_retries` | `unlimited` | Count or `unlimited`. `0` turns reconnection off, also after a lost connection | Review Comment: nit: This row contradicts the shared cross-SDK table at `core/common/src/types/configuration/tcp_config/tcp_client_reconnection_config.rs:44`, which lists `0` as unlimited for C#. Update the C# row, or state that the table describes the `MaxRetries` field alone. ########## foreign/csharp/Iggy_SDK/Utils/IggyDurationParser.cs: ########## @@ -0,0 +1,399 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +using System.Text; + +namespace Apache.Iggy.Utils; + +/// <summary> +/// Parses humantime durations such as <c>5s</c> or <c>1h 30m</c>. The input is lowercased and the zero spellings +/// are mapped first. The grammar keeps u64 overflow checks and exact fraction division, so a value that loses +/// precision is rejected, not rounded. +/// </summary> +internal static class IggyDurationParser +{ + private const ulong NanosPerSecond = 1_000_000_000; + + private const string OverflowMessage = + "number is too large or cannot be represented without a lack of precision (values below 1ns are not supported)"; + + private static readonly Dictionary<string, Unit> Units = new() + { + ["nanos"] = Unit.Nanosecond, + ["nsec"] = Unit.Nanosecond, + ["ns"] = Unit.Nanosecond, + ["usec"] = Unit.Microsecond, + ["us"] = Unit.Microsecond, + ["µs"] = Unit.Microsecond, + ["millis"] = Unit.Millisecond, + ["msec"] = Unit.Millisecond, + ["ms"] = Unit.Millisecond, + ["seconds"] = Unit.Second, + ["second"] = Unit.Second, + ["secs"] = Unit.Second, + ["sec"] = Unit.Second, + ["s"] = Unit.Second, + ["minutes"] = Unit.Minute, + ["minute"] = Unit.Minute, + ["min"] = Unit.Minute, + ["mins"] = Unit.Minute, + ["m"] = Unit.Minute, + ["hours"] = Unit.Hour, + ["hour"] = Unit.Hour, + ["hr"] = Unit.Hour, + ["hrs"] = Unit.Hour, + ["h"] = Unit.Hour, + ["days"] = Unit.Day, + ["day"] = Unit.Day, + ["d"] = Unit.Day, + ["weeks"] = Unit.Week, + ["week"] = Unit.Week, + ["wk"] = Unit.Week, + ["wks"] = Unit.Week, + ["w"] = Unit.Week, + ["months"] = Unit.Month, + ["month"] = Unit.Month, + ["years"] = Unit.Year, + ["year"] = Unit.Year, + ["yr"] = Unit.Year, + ["yrs"] = Unit.Year, + ["y"] = Unit.Year + }; + + private enum Unit + { + Nanosecond, + Microsecond, + Millisecond, + Second, + Minute, + Hour, + Day, + Week, + Month, + Year + } + + /// <summary> + /// Parses a duration such as <c>5s</c>, <c>1h 30m</c> or <c>unlimited</c> (zero). + /// </summary> + /// <exception cref="FormatException"> + /// Thrown when the value does not parse or does not fit in a <see cref="TimeSpan" />. + /// </exception> + internal static TimeSpan Parse(string value) + { + var (seconds, nanoseconds) = ParseParts(value); + try + { + // TimeSpan counts 100 ns ticks, so a sub-tick remainder is dropped. + return TimeSpan.FromTicks(checked((long)seconds * TimeSpan.TicksPerSecond + nanoseconds / 100)); + } + catch (OverflowException) + { + throw new FormatException(OverflowMessage); + } + } + + /// <summary> + /// Parses like <see cref="Parse" />, but without the <see cref="TimeSpan" /> range limit. + /// </summary> + /// <exception cref="FormatException">Thrown when the value does not parse.</exception> + internal static (ulong Seconds, uint Nanoseconds) ParseParts(string value) + { + var lowered = value.ToLowerInvariant(); + return lowered is "0" or "unlimited" or "disabled" or "none" ? (0, 0) : ParseHumantime(lowered); + } + + /// <summary> + /// Parses a lowercase humantime duration into whole seconds plus the subsecond nanoseconds. + /// </summary> + /// <exception cref="FormatException">Thrown when the value does not parse.</exception> + internal static (ulong Seconds, uint Nanoseconds) ParseHumantime(string input) + { + try + { + return new Parser(input).Parse(); + } + catch (OverflowException) + { + throw new FormatException(OverflowMessage); + } + } + + private static ulong Mul(ulong left, ulong right) + { + return checked(left * right); + } + + private static ulong Add(ulong left, ulong right) + { + return checked(left + right); + } + + // A division that is not exact would lose precision, which humantime reports as an overflow. + private static ulong Div(ulong dividend, ulong divisor) + { + if (dividend % divisor != 0) + { + throw new FormatException(OverflowMessage); + } + + return dividend / divisor; + } + + private readonly record struct Fraction(ulong Numerator, ulong Denominator); + + /// <summary> + /// Walks the input by char. Every accepted character, including Unicode whitespace and <c>µ</c>, is a single + /// UTF-16 char, so a surrogate fails at its first half. Error offsets are UTF-8 byte offsets. + /// </summary> + private sealed class Parser(string source) + { + private int _index; + private ulong _nanoseconds; + private ulong _seconds; + + internal (ulong Seconds, uint Nanoseconds) Parse() + { + var number = ParseFirstChar() ?? throw new FormatException("value was empty"); + while (true) + { + Fraction? fraction = null; + var offset = _index; + while (Next() is { } c) + { + if (IsDigit(c)) + { + number = Add(Mul(number, 10), Digit(c)); + } + else if (char.IsWhiteSpace(c)) + { + } + else if (IsUnitChar(c)) + { + break; + } + else if (c == '.') + { + fraction = ParseFractionalPart(ref offset); + break; + } + else + { + throw InvalidCharacter(offset); + } + + offset = _index; + } + + var start = offset; + offset = _index; + var nextNumber = false; + while (Next() is { } c) + { + if (IsDigit(c)) + { + AddUnit(number, fraction, start, offset); + number = Digit(c); + nextNumber = true; + break; + } + + if (char.IsWhiteSpace(c)) + { + break; + } + + if (!IsUnitChar(c)) + { + throw InvalidCharacter(offset); + } + + offset = _index; + } + + if (nextNumber) + { + continue; + } + + AddUnit(number, fraction, start, offset); + if (ParseFirstChar() is not { } next) + { + return (_seconds, (uint)_nanoseconds); + } + + number = next; + } + } + + private ulong? ParseFirstChar() + { + var offset = _index; + while (Next() is { } c) + { + if (IsDigit(c)) + { + return Digit(c); + } + + if (!char.IsWhiteSpace(c)) + { + throw new FormatException($"expected number at {ByteOffset(offset)}"); + } + } + + return null; + } + + private Fraction ParseFractionalPart(ref int offset) + { + ulong numerator = 0; + ulong denominator = 1; + var zeros = true; + while (Next() is { } c) + { + if (c == '0') + { + denominator = Mul(denominator, 10); + if (!zeros) Review Comment: simplification: The `zeros` flag in `ParseFractionalPart` is true exactly while `numerator` is zero, so the multiply it guards is a no-op. Drop the flag and always multiply `numerator` in the `'0'` branch, which saves three lines. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
