diff options
author | Jon Skeet <jonskeet@google.com> | 2016-01-20 18:43:00 +0000 |
---|---|---|
committer | Jon Skeet <jonskeet@google.com> | 2016-01-20 18:43:00 +0000 |
commit | dd43dcca8c3a0af761ae981edcadd7e78e875fe8 (patch) | |
tree | 57451afd4dc559df5b267357f90a11cbc2d3a410 /csharp/src/Google.Protobuf | |
parent | 8c5260b21bb0e217e91375893d507a0ef6578d67 (diff) | |
download | protobuf-dd43dcca8c3a0af761ae981edcadd7e78e875fe8.tar.gz protobuf-dd43dcca8c3a0af761ae981edcadd7e78e875fe8.tar.bz2 protobuf-dd43dcca8c3a0af761ae981edcadd7e78e875fe8.zip |
Ensure that FieldMask, Timestamp and Duration ToString() calls don't throw
The usage of ICustomDiagnosticMessage here is non-essential - ToDiagnosticString
doesn't actually get called by ToString() in this case, due to JsonFormatter code. It was
intended to make it clearer that it *did* have a custom format... but then arguably I should
do the same for Value, Struct, Any etc.
Moving some of the code out of JsonFormatter and into Duration/Timestamp/FieldMask likewise
feels somewhat nice, somewhat nasty... basically there are JSON-specific bits of formatting, but
also domain-specific bits of computation. <sigh>
Thoughts welcome.
Diffstat (limited to 'csharp/src/Google.Protobuf')
5 files changed, 284 insertions, 69 deletions
diff --git a/csharp/src/Google.Protobuf/Google.Protobuf.csproj b/csharp/src/Google.Protobuf/Google.Protobuf.csproj index 617497e6..f95bae07 100644 --- a/csharp/src/Google.Protobuf/Google.Protobuf.csproj +++ b/csharp/src/Google.Protobuf/Google.Protobuf.csproj @@ -132,6 +132,7 @@ <Compile Include="WellKnownTypes\DurationPartial.cs" />
<Compile Include="WellKnownTypes\Empty.cs" />
<Compile Include="WellKnownTypes\FieldMask.cs" />
+ <Compile Include="WellKnownTypes\FieldMaskPartial.cs" />
<Compile Include="WellKnownTypes\SourceContext.cs" />
<Compile Include="WellKnownTypes\Struct.cs" />
<Compile Include="WellKnownTypes\TimeExtensions.cs" />
diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index 21239cf2..17495747 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -37,6 +37,7 @@ using System.Text; using Google.Protobuf.Reflection; using Google.Protobuf.WellKnownTypes; using System.Linq; +using System.Collections.Generic; namespace Google.Protobuf { @@ -122,6 +123,8 @@ namespace Google.Protobuf private readonly Settings settings; + private bool DiagnosticOnly => ReferenceEquals(this, diagnosticFormatter); + /// <summary> /// Creates a new formatted with the given settings. /// </summary> @@ -181,7 +184,7 @@ namespace Google.Protobuf WriteNull(builder); return; } - if (ReferenceEquals(this, diagnosticFormatter)) + if (DiagnosticOnly) { ICustomDiagnosticMessage customDiagnosticMessage = message as ICustomDiagnosticMessage; if (customDiagnosticMessage != null) @@ -513,60 +516,32 @@ namespace Google.Protobuf private void WriteTimestamp(StringBuilder builder, IMessage value) { - builder.Append('"'); // TODO: In the common case where this *is* using the built-in Timestamp type, we could // avoid all the reflection at this point, by casting to Timestamp. In the interests of // avoiding subtle bugs, don't do that until we've implemented DynamicMessage so that we can prove // it still works in that case. int nanos = (int) value.Descriptor.Fields[Timestamp.NanosFieldNumber].Accessor.GetValue(value); long seconds = (long) value.Descriptor.Fields[Timestamp.SecondsFieldNumber].Accessor.GetValue(value); - - // Even if the original message isn't using the built-in classes, we can still build one... and its - // conversion will check whether or not it's normalized. - // TODO: Perhaps the diagnostic-only formatter should not throw for non-normalized values? - Timestamp ts = new Timestamp { Seconds = seconds, Nanos = nanos }; - // Use .NET's formatting for the value down to the second, including an opening double quote (as it's a string value) - DateTime dateTime = ts.ToDateTime(); - builder.Append(dateTime.ToString("yyyy'-'MM'-'dd'T'HH:mm:ss", CultureInfo.InvariantCulture)); - AppendNanoseconds(builder, Math.Abs(ts.Nanos)); - builder.Append("Z\""); + builder.Append(Timestamp.ToJson(seconds, nanos, DiagnosticOnly)); } private void WriteDuration(StringBuilder builder, IMessage value) { - builder.Append('"'); // TODO: Same as for WriteTimestamp int nanos = (int) value.Descriptor.Fields[Duration.NanosFieldNumber].Accessor.GetValue(value); long seconds = (long) value.Descriptor.Fields[Duration.SecondsFieldNumber].Accessor.GetValue(value); - - // TODO: Perhaps the diagnostic-only formatter should not throw for non-normalized values? - // Even if the original message isn't using the built-in classes, we can still build one... and then - // rely on it being normalized. - if (!Duration.IsNormalized(seconds, nanos)) - { - throw new InvalidOperationException("Non-normalized duration value"); - } - - // The seconds part will normally provide the minus sign if we need it, but not if it's 0... - if (seconds == 0 && nanos < 0) - { - builder.Append('-'); - } - - builder.Append(seconds.ToString("d", CultureInfo.InvariantCulture)); - AppendNanoseconds(builder, Math.Abs(nanos)); - builder.Append("s\""); + builder.Append(Duration.ToJson(seconds, nanos, DiagnosticOnly)); } private void WriteFieldMask(StringBuilder builder, IMessage value) { - IList paths = (IList) value.Descriptor.Fields[FieldMask.PathsFieldNumber].Accessor.GetValue(value); - WriteString(builder, string.Join(",", paths.Cast<string>().Select(ToCamelCaseForFieldMask))); + var paths = (IList<string>) value.Descriptor.Fields[FieldMask.PathsFieldNumber].Accessor.GetValue(value); + builder.Append(FieldMask.ToJson(paths, DiagnosticOnly)); } private void WriteAny(StringBuilder builder, IMessage value) { - if (ReferenceEquals(this, diagnosticFormatter)) + if (DiagnosticOnly) { WriteDiagnosticOnlyAny(builder, value); return; @@ -627,31 +602,6 @@ namespace Google.Protobuf return parts[1]; } - /// <summary> - /// Appends a number of nanoseconds to a StringBuilder. Either 0 digits are added (in which - /// case no "." is appended), or 3 6 or 9 digits. - /// </summary> - private static void AppendNanoseconds(StringBuilder builder, int nanos) - { - if (nanos != 0) - { - builder.Append('.'); - // Output to 3, 6 or 9 digits. - if (nanos % 1000000 == 0) - { - builder.Append((nanos / 1000000).ToString("d3", CultureInfo.InvariantCulture)); - } - else if (nanos % 1000 == 0) - { - builder.Append((nanos / 1000).ToString("d6", CultureInfo.InvariantCulture)); - } - else - { - builder.Append(nanos.ToString("d9", CultureInfo.InvariantCulture)); - } - } - } - private void WriteStruct(StringBuilder builder, IMessage message) { builder.Append("{ "); @@ -785,7 +735,7 @@ namespace Google.Protobuf /// <remarks> /// Other than surrogate pair handling, this code is mostly taken from src/google/protobuf/util/internal/json_escaping.cc. /// </remarks> - private void WriteString(StringBuilder builder, string text) + internal static void WriteString(StringBuilder builder, string text) { builder.Append('"'); for (int i = 0; i < text.Length; i++) diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/DurationPartial.cs b/csharp/src/Google.Protobuf/WellKnownTypes/DurationPartial.cs index b8eba9d3..e5247e90 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/DurationPartial.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/DurationPartial.cs @@ -31,12 +31,14 @@ #endregion using System; +using System.Globalization; +using System.Text; namespace Google.Protobuf.WellKnownTypes { // Manually-written partial class for the Duration well-known type, // providing a conversion to TimeSpan and convenience operators. - public partial class Duration + public partial class Duration : ICustomDiagnosticMessage { /// <summary> /// The number of nanoseconds in a second. @@ -73,7 +75,6 @@ namespace Google.Protobuf.WellKnownTypes return Math.Sign(seconds) * Math.Sign(nanoseconds) != -1; } - /// <summary> /// Converts this <see cref="Duration"/> to a <see cref="TimeSpan"/>. /// </summary> @@ -180,5 +181,90 @@ namespace Google.Protobuf.WellKnownTypes } return new Duration { Seconds = seconds, Nanos = nanoseconds }; } + + /// <summary> + /// Converts a duration specified in seconds/nanoseconds to a string. + /// </summary> + /// <remarks> + /// If the value is a normalized duration in the range described in <c>duration.proto</c>, + /// <paramref name="diagnosticOnly"/> is ignored. Otherwise, if the parameter is <c>true</c>, + /// a JSON object with a warning is returned; if it is <c>false</c>, an <see cref="InvalidOperationException"/> is thrown. + /// </remarks> + /// <param name="seconds">Seconds portion of the duration.</param> + /// <param name="nanoseconds">Nanoseconds portion of the duration.</param> + /// <param name="diagnosticOnly">Determines the handling of non-normalized values</param> + /// <exception cref="InvalidOperationException">The represented duration is invalid, and <paramref name="diagnosticOnly"/> is <c>false</c>.</exception> + internal static string ToJson(long seconds, int nanoseconds, bool diagnosticOnly) + { + if (IsNormalized(seconds, nanoseconds)) + { + var builder = new StringBuilder(); + builder.Append('"'); + // The seconds part will normally provide the minus sign if we need it, but not if it's 0... + if (seconds == 0 && nanoseconds < 0) + { + builder.Append('-'); + } + + builder.Append(seconds.ToString("d", CultureInfo.InvariantCulture)); + AppendNanoseconds(builder, Math.Abs(nanoseconds)); + builder.Append("s\""); + return builder.ToString(); + } + if (diagnosticOnly) + { + // Note: the double braces here are escaping for braces in format strings. + return string.Format(CultureInfo.InvariantCulture, + "{{ \"@warning\": \"Invalid Duration\", \"seconds\": \"{0}\", \"nanos\": {1} }}", + seconds, + nanoseconds); + } + else + { + throw new InvalidOperationException("Non-normalized duration value"); + } + } + + /// <summary> + /// Returns a string representation of this <see cref="Duration"/> for diagnostic purposes. + /// </summary> + /// <remarks> + /// Normally the returned value will be a JSON string value (including leading and trailing quotes) but + /// when the value is non-normalized or out of range, a JSON object representation will be returned + /// instead, including a warning. This is to avoid exceptions being thrown when trying to + /// diagnose problems - the regular JSON formatter will still throw an exception for non-normalized + /// values. + /// </remarks> + /// <returns>A string representation of this value.</returns> + public string ToDiagnosticString() + { + return ToJson(Seconds, Nanos, true); + } + + /// <summary> + /// Appends a number of nanoseconds to a StringBuilder. Either 0 digits are added (in which + /// case no "." is appended), or 3 6 or 9 digits. This is internal for use in Timestamp as well + /// as Duration. + /// </summary> + internal static void AppendNanoseconds(StringBuilder builder, int nanos) + { + if (nanos != 0) + { + builder.Append('.'); + // Output to 3, 6 or 9 digits. + if (nanos % 1000000 == 0) + { + builder.Append((nanos / 1000000).ToString("d3", CultureInfo.InvariantCulture)); + } + else if (nanos % 1000 == 0) + { + builder.Append((nanos / 1000).ToString("d6", CultureInfo.InvariantCulture)); + } + else + { + builder.Append(nanos.ToString("d9", CultureInfo.InvariantCulture)); + } + } + } } } diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/FieldMaskPartial.cs b/csharp/src/Google.Protobuf/WellKnownTypes/FieldMaskPartial.cs new file mode 100644 index 00000000..df1292dc --- /dev/null +++ b/csharp/src/Google.Protobuf/WellKnownTypes/FieldMaskPartial.cs @@ -0,0 +1,122 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2016 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#endregion + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using System.Text; + +namespace Google.Protobuf.WellKnownTypes +{ + // Manually-written partial class for the FieldMask well-known type. + public partial class FieldMask : ICustomDiagnosticMessage + { + /// <summary> + /// Converts a timestamp specified in seconds/nanoseconds to a string. + /// </summary> + /// <remarks> + /// If the value is a normalized duration in the range described in <c>field_mask.proto</c>, + /// <paramref name="diagnosticOnly"/> is ignored. Otherwise, if the parameter is <c>true</c>, + /// a JSON object with a warning is returned; if it is <c>false</c>, an <see cref="InvalidOperationException"/> is thrown. + /// </remarks> + /// <param name="paths">Paths in the field mask</param> + /// <param name="diagnosticOnly">Determines the handling of non-normalized values</param> + /// <exception cref="InvalidOperationException">The represented duration is invalid, and <paramref name="diagnosticOnly"/> is <c>false</c>.</exception> + internal static string ToJson(IList<string> paths, bool diagnosticOnly) + { + var firstInvalid = paths.FirstOrDefault(p => !ValidatePath(p)); + if (firstInvalid == null) + { + var builder = new StringBuilder(); + JsonFormatter.WriteString(builder, string.Join(",", paths.Select(JsonFormatter.ToCamelCase))); + return builder.ToString(); + } + else + { + if (diagnosticOnly) + { + var builder = new StringBuilder(); + builder.Append("{ \"@warning\": \"Invalid FieldMask\", \"paths\": "); + JsonFormatter.Default.WriteList(builder, (IList) paths); + builder.Append(" }"); + return builder.ToString(); + } + else + { + throw new InvalidOperationException($"Invalid field mask to be converted to JSON: {firstInvalid}"); + } + } + } + + /// <summary> + /// Camel-case converter with added strictness for field mask formatting. + /// </summary> + /// <exception cref="InvalidOperationException">The field mask is invalid for JSON representation</exception> + private static bool ValidatePath(string input) + { + for (int i = 0; i < input.Length; i++) + { + char c = input[i]; + if (c >= 'A' && c <= 'Z') + { + return false; + } + if (c == '_' && i < input.Length - 1) + { + char next = input[i + 1]; + if (next < 'a' || next > 'z') + { + return false; + } + } + } + return true; + } + + /// <summary> + /// Returns a string representation of this <see cref="FieldMask"/> for diagnostic purposes. + /// </summary> + /// <remarks> + /// Normally the returned value will be a JSON string value (including leading and trailing quotes) but + /// when the value is non-normalized or out of range, a JSON object representation will be returned + /// instead, including a warning. This is to avoid exceptions being thrown when trying to + /// diagnose problems - the regular JSON formatter will still throw an exception for non-normalized + /// values. + /// </remarks> + /// <returns>A string representation of this value.</returns> + public string ToDiagnosticString() + { + return ToJson(Paths, true); + } + } +} diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/TimestampPartial.cs b/csharp/src/Google.Protobuf/WellKnownTypes/TimestampPartial.cs index 7c50a3d7..86998e27 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/TimestampPartial.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/TimestampPartial.cs @@ -31,10 +31,12 @@ #endregion using System; +using System.Globalization; +using System.Text; namespace Google.Protobuf.WellKnownTypes { - public partial class Timestamp + public partial class Timestamp : ICustomDiagnosticMessage { private static readonly DateTime UnixEpoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc); // Constants determined programmatically, but then hard-coded so they can be constant expressions. @@ -43,11 +45,11 @@ namespace Google.Protobuf.WellKnownTypes internal const long UnixSecondsAtBclMinValue = -BclSecondsAtUnixEpoch; internal const int MaxNanos = Duration.NanosecondsPerSecond - 1; - private bool IsNormalized => - Nanos >= 0 && - Nanos <= MaxNanos && - Seconds >= UnixSecondsAtBclMinValue && - Seconds <= UnixSecondsAtBclMaxValue; + private static bool IsNormalized(long seconds, int nanoseconds) => + nanoseconds >= 0 && + nanoseconds <= MaxNanos && + seconds >= UnixSecondsAtBclMinValue && + seconds <= UnixSecondsAtBclMaxValue; /// <summary> /// Returns the difference between one <see cref="Timestamp"/> and another, as a <see cref="Duration"/>. @@ -111,7 +113,7 @@ namespace Google.Protobuf.WellKnownTypes /// incorrectly normalized or is outside the valid range.</exception> public DateTime ToDateTime() { - if (!IsNormalized) + if (!IsNormalized(Seconds, Nanos)) { throw new InvalidOperationException(@"Timestamp contains invalid values: Seconds={Seconds}; Nanos={Nanos}"); } @@ -181,5 +183,59 @@ namespace Google.Protobuf.WellKnownTypes } return new Timestamp { Seconds = seconds, Nanos = nanoseconds }; } + + /// <summary> + /// Converts a timestamp specified in seconds/nanoseconds to a string. + /// </summary> + /// <remarks> + /// If the value is a normalized duration in the range described in <c>timestamp.proto</c>, + /// <paramref name="diagnosticOnly"/> is ignored. Otherwise, if the parameter is <c>true</c>, + /// a JSON object with a warning is returned; if it is <c>false</c>, an <see cref="InvalidOperationException"/> is thrown. + /// </remarks> + /// <param name="seconds">Seconds portion of the duration.</param> + /// <param name="nanoseconds">Nanoseconds portion of the duration.</param> + /// <param name="diagnosticOnly">Determines the handling of non-normalized values</param> + /// <exception cref="InvalidOperationException">The represented duration is invalid, and <paramref name="diagnosticOnly"/> is <c>false</c>.</exception> + internal static string ToJson(long seconds, int nanoseconds, bool diagnosticOnly) + { + if (IsNormalized(seconds, nanoseconds)) + { + // Use .NET's formatting for the value down to the second, including an opening double quote (as it's a string value) + DateTime dateTime = UnixEpoch.AddSeconds(seconds); + var builder = new StringBuilder(); + builder.Append('"'); + builder.Append(dateTime.ToString("yyyy'-'MM'-'dd'T'HH:mm:ss", CultureInfo.InvariantCulture)); + Duration.AppendNanoseconds(builder, nanoseconds); + builder.Append("Z\""); + return builder.ToString(); + } + if (diagnosticOnly) + { + return string.Format(CultureInfo.InvariantCulture, + "{{ \"@warning\": \"Invalid Timestamp\", \"seconds\": \"{0}\", \"nanos\": {1} }}", + seconds, + nanoseconds); + } + else + { + throw new InvalidOperationException("Non-normalized timestamp value"); + } + } + + /// <summary> + /// Returns a string representation of this <see cref="Timestamp"/> for diagnostic purposes. + /// </summary> + /// <remarks> + /// Normally the returned value will be a JSON string value (including leading and trailing quotes) but + /// when the value is non-normalized or out of range, a JSON object representation will be returned + /// instead, including a warning. This is to avoid exceptions being thrown when trying to + /// diagnose problems - the regular JSON formatter will still throw an exception for non-normalized + /// values. + /// </remarks> + /// <returns>A string representation of this value.</returns> + public string ToDiagnosticString() + { + return ToJson(Seconds, Nanos, true); + } } } |