Standardize HTTP error results, better rejection message when parsing validated strings (#2663)

1. When parsing a `ValidatedString` from JSON and it fails, include a message about the expected format of the string.
   - Reworked the classes using C#11 features to reduce the amount of boilerplate needed to add a new validated string type.

2. Change to use [RFC7807](https://www.rfc-editor.org/rfc/rfc7807) format for HTTP error responses. At the moment we returned the `Error` type which was undocumented.
3. Update CLI to parse RFC7807 responses.

Old error looked like:

```console
$ onefuzz containers create AbCd
ERROR:cli:command failed: request did not succeed: HTTP 500 -
```

New error looks like:

```console
$ onefuzz containers create AbCd
ERROR:cli:command failed: request did not succeed (400: INVALID_REQUEST): Unable
to parse 'AbCd' as a Container: Container name must be 3-63 lowercase letters, numbers,
or non-consecutive hyphens (see: https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftstorage)
```

Closes #2661.
This commit is contained in:
George Pollard
2022-12-02 10:33:14 +13:00
committed by GitHub
parent 88e8c11a02
commit 38dfa668bc
28 changed files with 304 additions and 294 deletions

View File

@ -1,5 +1,4 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.CodeAnalysis;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.RegularExpressions;
@ -7,44 +6,73 @@ using Azure.Core;
namespace Microsoft.OneFuzz.Service;
static class Check {
private static readonly Regex _isAlnum = new(@"\A[a-zA-Z0-9]+\z", RegexOptions.Compiled);
public static bool IsAlnum(string input) => _isAlnum.IsMatch(input);
static partial class Check {
[GeneratedRegex("\\A[a-zA-Z0-9]+\\z")]
private static partial Regex IsAlnumRegex();
public static bool IsAlnum(string input) => IsAlnumRegex().IsMatch(input);
private static readonly Regex _isAlnumDash = new(@"\A[a-zA-Z0-9\-]+\z", RegexOptions.Compiled);
public static bool IsAlnumDash(string input) => _isAlnumDash.IsMatch(input);
[GeneratedRegex("\\A[a-zA-Z0-9\\-]+\\z")]
private static partial Regex IsAlnumDashRegex();
public static bool IsAlnumDash(string input) => IsAlnumDashRegex().IsMatch(input);
// Permits 1-64 characters: alphanumeric, underscore, period, or dash.
private static readonly Regex _isNameLike = new(@"\A[._a-zA-Z0-9\-]{1,64}\z", RegexOptions.Compiled);
public static bool IsNameLike(string input) => _isNameLike.IsMatch(input);
[GeneratedRegex("\\A[._a-zA-Z0-9\\-]{1,64}\\z")]
private static partial Regex IsNameLikeRegex();
public static bool IsNameLike(string input) => IsNameLikeRegex().IsMatch(input);
// This regex is based upon DNS labels but more restricted.
// It is used for many different Storage resources.
// See: https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftstorage
// - 3-63
// - Lowercase letters, numbers, and hyphens.
// - Start with lowercase letter or number. Can't use consecutive hyphens.
[GeneratedRegex(@"\A(?!-)(?!.*--)[a-z0-9\-]{3,63}\z")]
private static partial Regex StorageDnsLabelRegex();
public static bool IsStorageDnsLabel(string input) => StorageDnsLabelRegex().IsMatch(input);
}
// Base class for types that are wrappers around a validated string.
public abstract record ValidatedString(string String) {
public interface IValidatedString<T> where T : IValidatedString<T> {
public static abstract T Parse(string input);
public static abstract bool IsValid(string input);
public static abstract string Requirements { get; }
public string String { get; }
}
public abstract record ValidatedStringBase<T> where T : IValidatedString<T> {
protected ValidatedStringBase(string value) {
if (!T.IsValid(value)) {
throw new ArgumentException(T.Requirements);
}
String = value;
}
public string String { get; }
public sealed override string ToString() => String;
public static bool TryParse(string input, [NotNullWhen(returnValue: true)] out T? result) {
try {
result = T.Parse(input);
return true;
} catch (ArgumentException) {
result = default;
return false;
}
}
}
// JSON converter for types that are wrappers around a validated string.
public abstract class ValidatedStringConverter<T> : JsonConverter<T> where T : ValidatedString {
protected abstract bool TryParse(string input, out T? output);
public sealed override bool CanConvert(Type typeToConvert)
=> typeToConvert == typeof(T);
public sealed class ValidatedStringConverter<T> : JsonConverter<T> where T : IValidatedString<T> {
public sealed override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) {
if (reader.TokenType != JsonTokenType.String) {
throw new JsonException("expected a string");
}
var value = reader.GetString();
if (value is null) {
throw new JsonException("expected a string");
throw new JsonException("Expected a string");
}
if (TryParse(value, out var result)) {
if (ValidatedStringBase<T>.TryParse(value, out var result)) {
return result;
} else {
throw new JsonException($"unable to parse input as a {typeof(T).Name}");
throw new JsonException($"Unable to parse '{value}' as a {typeof(T).Name}: {T.Requirements}");
}
}
@ -52,110 +80,32 @@ public abstract class ValidatedStringConverter<T> : JsonConverter<T> where T : V
=> writer.WriteStringValue(value.String);
}
[JsonConverter(typeof(Converter))]
public sealed record PoolName : ValidatedString {
[JsonConverter(typeof(ValidatedStringConverter<PoolName>))]
public sealed record PoolName : ValidatedStringBase<PoolName>, IValidatedString<PoolName> {
private PoolName(string value) : base(value) { }
public static PoolName Parse(string input) => new(input);
public static string Requirements => "Pool name must have only numbers, letters, underscores, periods, or dashes";
// NOTE: PoolName is currently _not_ validated, since this
// can break existing users. When CSHARP-RELEASE happens, we can
// try to synchronize other breaking changes with that.
private static bool IsValid(string input) => true;
private PoolName(string value) : base(value) {
Debug.Assert(IsValid(value));
}
public static PoolName Parse(string input) {
if (TryParse(input, out var result)) {
return result;
}
throw new ArgumentException("Pool name must have only numbers, letters, underscores, periods, or dashes");
}
public static bool TryParse(string input, [NotNullWhen(returnValue: true)] out PoolName? result) {
if (!IsValid(input)) {
result = default;
return false;
}
result = new PoolName(input);
return true;
}
public sealed class Converter : ValidatedStringConverter<PoolName> {
protected override bool TryParse(string input, out PoolName? output)
=> PoolName.TryParse(input, out output);
}
public static bool IsValid(string input) => true;
}
[JsonConverter(typeof(Converter))]
public record Region : ValidatedString {
private static bool IsValid(string input) => Check.IsAlnum(input);
private Region(string value) : base(value.ToLowerInvariant()) {
Debug.Assert(IsValid(value));
}
public static Region Parse(string input) {
if (TryParse(input, out var result)) {
return result;
}
throw new ArgumentException("Region name must have only numbers or letters");
}
public static bool TryParse(string input, [NotNullWhen(returnValue: true)] out Region? result) {
if (!IsValid(input)) {
result = default;
return false;
}
result = new Region(input);
return true;
}
[JsonConverter(typeof(ValidatedStringConverter<Region>))]
public sealed record Region : ValidatedStringBase<Region>, IValidatedString<Region> {
private Region(string value) : base(value.ToLowerInvariant()) { }
public static Region Parse(string input) => new(input);
public static bool IsValid(string input) => Check.IsAlnum(input);
public static string Requirements => "Region name must have only numbers or letters";
public static implicit operator AzureLocation(Region me) => new(me.String);
public static implicit operator Region(AzureLocation it) => new(it.Name);
public sealed class Converter : ValidatedStringConverter<Region> {
protected override bool TryParse(string input, out Region? output)
=> Region.TryParse(input, out output);
}
}
[JsonConverter(typeof(Converter))]
public record Container : ValidatedString {
// See: https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftstorage
// - 3-63
// - Lowercase letters, numbers, and hyphens.
// - Start with lowercase letter or number. Can't use consecutive hyphens.
private static readonly Regex _containerRegex = new(@"\A(?!-)(?!.*--)[a-z0-9\-]{3,63}\z", RegexOptions.Compiled);
private static bool IsValid(string input) => _containerRegex.IsMatch(input);
private Container(string value) : base(value) {
Debug.Assert(IsValid(value));
}
public static Container Parse(string input) {
if (TryParse(input, out var result)) {
return result;
}
throw new ArgumentException("Container name must have only numbers, letters or dashes");
}
public static bool TryParse(string input, [NotNullWhen(returnValue: true)] out Container? result) {
if (!IsValid(input)) {
result = default;
return false;
}
result = new Container(input);
return true;
}
public sealed class Converter : ValidatedStringConverter<Container> {
protected override bool TryParse(string input, out Container? output)
=> Container.TryParse(input, out output);
}
[JsonConverter(typeof(ValidatedStringConverter<Container>))]
public sealed record Container : ValidatedStringBase<Container>, IValidatedString<Container> {
private Container(string value) : base(value) { }
public static Container Parse(string input) => new(input);
public static bool IsValid(string input) => Check.IsStorageDnsLabel(input);
public static string Requirements => "Container name must be 3-63 lowercase letters, numbers, or non-consecutive hyphens (see: https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftstorage)";
}

View File

@ -2,6 +2,7 @@
using System.Net;
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Text.Json.Serialization;
using Faithlife.Utility;
using Microsoft.Azure.Functions.Worker.Http;
using Microsoft.OneFuzz.Service.OneFuzzLib.Orm;
@ -12,6 +13,48 @@ public interface IRequestHandling {
Async.Task<HttpResponseData> NotOk(HttpRequestData request, Error error, string context, HttpStatusCode statusCode = HttpStatusCode.BadRequest);
}
// See: https://www.rfc-editor.org/rfc/rfc7807#section-3
public sealed class ProblemDetails {
[JsonConstructor]
public ProblemDetails(int status, string title, string detail) {
Status = status;
Title = title;
Detail = detail;
}
public ProblemDetails(HttpStatusCode code, Error error) {
Status = (int)code;
Title = error.Code.ToString();
Detail = error.Errors?.Join("\n") ?? "";
}
// We do not yet use the type/instance properties:
/// A URI reference [RFC3986] that identifies the problem type. This
/// specification encourages that, when dereferenced, it provide
/// human-readable documentation for the problem type (e.g., using HTML
/// [W3C.REC-html5-20141028]). When this member is not present, its value
/// is assumed to be "about:blank".
// public string? Type { get; set; } = "about:blank";
/// A URI reference that identifies the specific occurrence of the problem.
/// It may or may not yield further information if dereferenced.
// public string? Instance { get; set; }
/// A short, human-readable summary of the problem type. It SHOULD NOT
/// change from occurrence to occurrence of the problem, except for purposes
/// of localization (e.g., using proactive content negotiation; see
/// [RFC7231], Section 3.4).
public string Title { get; set; }
/// The HTTP status code ([RFC7231], Section 6) generated by the origin
/// server for this occurrence of the problem.
public int Status { get; set; }
// A human-readable explanation specific to this occurrence of the problem.
public string Detail { get; set; }
}
public class RequestHandling : IRequestHandling {
private readonly ILogTracer _log;
public RequestHandling(ILogTracer log) {
@ -22,9 +65,14 @@ public class RequestHandling : IRequestHandling {
if (statusNum >= 400 && statusNum <= 599) {
_log.Error($"request error: {context:Tag:Context} - {error:Tag:Error}");
var response = HttpResponseData.CreateResponse(request);
await response.WriteAsJsonAsync(error);
response.StatusCode = statusCode;
// emit standardized errors according to RFC7807:
// https://www.rfc-editor.org/rfc/rfc7807
var response = request.CreateResponse();
await response.WriteAsJsonAsync(
new ProblemDetails(statusCode, error),
"application/problem+json",
statusCode);
return response;
}
@ -33,7 +81,6 @@ public class RequestHandling : IRequestHandling {
public static async Async.Task<OneFuzzResult<T>> ParseRequest<T>(HttpRequestData req)
where T : BaseRequest {
Exception? exception = null;
try {
var t = await req.ReadFromJsonAsync<T>();
if (t != null) {
@ -72,16 +119,8 @@ public class RequestHandling : IRequestHandling {
$"Failed to deserialize message into type: {typeof(T)} - null");
}
} catch (Exception e) {
exception = e;
return OneFuzzResult<T>.Error(ConvertError(e));
}
if (exception != null) {
return OneFuzzResult<T>.Error(ConvertError(exception));
}
return OneFuzzResult<T>.Error(
ErrorCode.INVALID_REQUEST,
$"Failed to deserialize message into type: {typeof(T)} - {await req.ReadAsStringAsync()}"
);
}
public static async Async.Task<OneFuzzResult<T>> ParseUri<T>(HttpRequestData req) {
@ -107,6 +146,14 @@ public class RequestHandling : IRequestHandling {
}
public static Error ConvertError(Exception exception) {
if (exception is AggregateException agg) {
var flattened = agg.Flatten();
if (flattened.InnerExceptions.Count == 1) {
// if we only have one inner exception, remove wrapping
return ConvertError(flattened.InnerExceptions[0]);
}
}
return new Error(
ErrorCode.INVALID_REQUEST,
new string[] {

View File

@ -236,8 +236,12 @@ public class EntityConverter {
} else if (ef.type == typeof(long)) {
return long.Parse(stringValue);
} else if (ef.type.IsClass) {
if (ef.type.IsAssignableTo(typeof(ValidatedString))) {
return ef.type.GetMethod("Parse")!.Invoke(null, new[] { stringValue });
try {
if (ef.type.GetMethod("Parse", BindingFlags.Static | BindingFlags.Public) is MethodInfo mi) {
return mi.Invoke(null, new[] { stringValue });
}
} catch (Exception ex) {
throw new ArgumentException($"Unable to parse '{stringValue}' as {ef.type}", ex);
}
return Activator.CreateInstance(ef.type, new[] { stringValue });

View File

@ -75,12 +75,12 @@ public static class JsonElementExt {
return e.GetProperty(property).GetBoolean();
}
public static T GetObjectProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T>, new() {
return new T().Convert(e.GetProperty(property)!);
public static T GetObjectProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T> {
return T.Convert(e.GetProperty(property));
}
public static T? GetNullableObjectProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T>, new() {
return e.GetProperty(property).ValueKind == JsonValueKind.Null ? default(T) : new T().Convert(e.GetProperty(property)!);
public static T? GetNullableObjectProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T> {
return e.GetProperty(property).ValueKind == JsonValueKind.Null ? default : T.Convert(e.GetProperty(property));
}
public static IDictionary<string, string>? GetNullableStringDictProperty(this JsonElement e, string property) {
@ -91,10 +91,10 @@ public static class JsonElementExt {
return e.GetProperty(property).Deserialize<IDictionary<string, string>>()!;
}
public static IDictionary<string, T> GetDictProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T>, new() {
public static IDictionary<string, T> GetDictProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T> {
return new Dictionary<string, T>(
e.GetProperty(property)!.Deserialize<IDictionary<string, JsonElement>>()!.Select(
kv => KeyValuePair.Create(kv.Key, new T().Convert(kv.Value))
kv => KeyValuePair.Create(kv.Key, T.Convert(kv.Value))
)
);
}
@ -116,36 +116,36 @@ public static class JsonElementExt {
}
public static IEnumerable<T> GetEnumerableProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T>, new() {
return e.GetProperty(property).EnumerateArray().Select(e => new T().Convert(e)!);
public static IEnumerable<T> GetEnumerableProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T> {
return e.GetProperty(property).EnumerateArray().Select(T.Convert);
}
public static IEnumerable<T>? GetEnumerableNullableProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T>, new() {
public static IEnumerable<T>? GetEnumerableNullableProperty<T>(this JsonElement e, string property) where T : IFromJsonElement<T> {
if (e.GetProperty(property).ValueKind == JsonValueKind.Null)
return null;
else
return e.GetProperty(property).EnumerateArray().Select(e => new T().Convert(e)!);
return e.GetProperty(property).EnumerateArray().Select(T.Convert);
}
}
public interface IFromJsonElement<T> {
T Convert(JsonElement e);
static abstract T Convert(JsonElement e);
}
public class BooleanResult : IFromJsonElement<BooleanResult> {
JsonElement _e;
public BooleanResult() { }
readonly JsonElement _e;
public BooleanResult(JsonElement e) => _e = e;
public bool IsError => Error.IsError(_e);
public Error? Error => new Error(_e);
public Error? Error => new(_e);
public bool Result => _e.GetProperty("result").GetBoolean();
public BooleanResult Convert(JsonElement e) => new BooleanResult(e);
public static BooleanResult Convert(JsonElement e) => new(e);
}
public abstract class ApiBase {
@ -200,12 +200,12 @@ public abstract class ApiBase {
return (await JsonDocument.ParseAsync(r.Content.ReadAsStream())).RootElement;
}
public static Result<IEnumerable<T>, Error> IEnumerableResult<T>(JsonElement res) where T : IFromJsonElement<T>, new() {
public static Result<IEnumerable<T>, Error> IEnumerableResult<T>(JsonElement res) where T : IFromJsonElement<T> {
if (Error.IsError(res)) {
return Result<IEnumerable<T>, Error>.Error(new Error(res));
} else {
if (res.ValueKind == JsonValueKind.Array)
return Result<IEnumerable<T>, Error>.Ok(res.EnumerateArray().Select(e => (new T()).Convert(e)));
return Result<IEnumerable<T>, Error>.Ok(res.EnumerateArray().Select(T.Convert));
else {
var r = Result<T>(res);
if (r.IsOk)
@ -215,17 +215,17 @@ public abstract class ApiBase {
}
}
}
public static Result<T, Error> Result<T>(JsonElement res) where T : IFromJsonElement<T>, new() {
public static Result<T, Error> Result<T>(JsonElement res) where T : IFromJsonElement<T> {
if (Error.IsError(res)) {
return Result<T, Error>.Error(new Error(res));
} else {
Assert.True(res.ValueKind != JsonValueKind.Array);
return Result<T, Error>.Ok((new T()).Convert(res));
return Result<T, Error>.Ok(T.Convert(res));
}
}
public static T Return<T>(JsonElement res) where T : IFromJsonElement<T>, new() {
return (new T()).Convert(res);
public static T Return<T>(JsonElement res) where T : IFromJsonElement<T> {
return T.Convert(res);
}
}

View File

@ -3,9 +3,8 @@
namespace FunctionalTests;
public class Authentication : IFromJsonElement<Authentication> {
JsonElement _e;
readonly JsonElement _e;
public Authentication() { }
public Authentication(JsonElement e) => _e = e;
public string Password => _e.GetStringProperty("password");
@ -13,6 +12,5 @@ public class Authentication : IFromJsonElement<Authentication> {
public string PublicKey => _e.GetStringProperty("public_key");
public string PrivateKey => _e.GetStringProperty("private_key");
public Authentication Convert(JsonElement e) => new Authentication(e);
public static Authentication Convert(JsonElement e) => new(e);
}

View File

@ -5,10 +5,9 @@ using Xunit.Abstractions;
namespace FunctionalTests {
public class ContainerInfo : IFromJsonElement<ContainerInfo> {
JsonElement _e;
public ContainerInfo() { }
readonly JsonElement _e;
public ContainerInfo(JsonElement e) => _e = e;
public ContainerInfo Convert(JsonElement e) => new ContainerInfo(e);
public static ContainerInfo Convert(JsonElement e) => new(e);
public string Name => _e.GetStringProperty("name");
public IDictionary<string, string>? Metadata => _e.GetNullableStringDictProperty("metadata");
public Uri SasUrl => new Uri(_e.GetStringProperty("sas_url"));

View File

@ -1,26 +1,26 @@
using System.Text.Json;
using Xunit;
namespace FunctionalTests;
public class Error : IComparable<Error>, IFromJsonElement<Error> {
JsonElement _e;
public Error() { }
private readonly JsonElement _e;
public Error(JsonElement e) {
_e = e;
Assert.True(_e.EnumerateObject().Count() == 2);
}
public int Code => _e.GetIntProperty("code");
public int StatusCode => _e.GetIntProperty("status");
public IEnumerable<string> Errors => _e.GetEnumerableStringProperty("errors");
public string Title => _e.GetStringProperty("title");
public Error Convert(JsonElement e) => new Error(e);
public string Detail => _e.GetStringProperty("detail");
public static Error Convert(JsonElement e) => new(e);
public static bool IsError(JsonElement res) {
return res.ValueKind == JsonValueKind.Object && res.TryGetProperty("code", out _) && res.TryGetProperty("errors", out _);
return res.ValueKind == JsonValueKind.Object
&& res.TryGetProperty("title", out _)
&& res.TryGetProperty("detail", out _);
}
public int CompareTo(Error? other) {
@ -28,16 +28,22 @@ public class Error : IComparable<Error>, IFromJsonElement<Error> {
return -1;
}
var sameErrorMessages = Errors.Count() == other.Errors.Count();
foreach (var s in other.Errors) {
if (!sameErrorMessages) break;
sameErrorMessages = Errors.Contains(s);
var statusCompare = StatusCode.CompareTo(other.StatusCode);
if (statusCompare != 0) {
return statusCompare;
}
if (other.Code == this.Code && sameErrorMessages) {
return 0;
} else
return 1;
var titleCompare = Title.CompareTo(other.Title);
if (titleCompare != 0) {
return titleCompare;
}
var detailCompare = Detail.CompareTo(other.Detail);
if (detailCompare != 0) {
return detailCompare;
}
return 0;
}
public override string ToString() {
@ -45,15 +51,15 @@ public class Error : IComparable<Error>, IFromJsonElement<Error> {
}
public bool IsWrongSizeError =>
Code == 450 && Errors.First() == "The field Size must be between 1 and 9.223372036854776E+18.";
Title == "INVALID_REQUEST" && Detail.Contains("The field Size must be between 1 and 9.223372036854776E+18.");
public bool UnableToFindPoolError => Code == 450 && Errors.First() == "unable to find pool";
public bool UnableToFindPoolError => Title == "INVALID_REQUEST" && Detail.Contains("unable to find pool");
public bool UnableToFindScalesetError => Code == 450 && Errors.First() == "unable to find scaleset";
public bool UnableToFindScalesetError => Title == "INVALID_REQUEST" && Detail.Contains("unable to find scaleset");
public bool UnableToFindNode => Code == 467 && Errors.First() == "unable to find node";
public bool UnableToFindNode => Title == "UNABLE_TO_FIND" && Detail.Contains("unable to find node");
public bool ShouldBeProvided(string p) => Code == 450 && Errors.First() == $"'{p}' query parameter must be provided";
public bool ShouldBeProvided(string p) => Title == "INVALID_REQUEST" && Detail.Contains($"'{p}' query parameter must be provided");
public bool UnableToFindTask => Code == 450 && Errors.First() == "unable to find task";
public bool UnableToFindTask => Title == "INVALID_REQUEST" && Detail.Contains("unable to find task");
}

View File

@ -5,11 +5,10 @@ using Xunit.Abstractions;
namespace FunctionalTests {
public class InfoVersion : IFromJsonElement<InfoVersion> {
JsonElement _e;
readonly JsonElement _e;
public InfoVersion() { }
public InfoVersion(JsonElement e) => _e = e;
public InfoVersion Convert(JsonElement e) => new InfoVersion(e);
public static InfoVersion Convert(JsonElement e) => new(e);
public string Git => _e.GetProperty("git").GetString()!;
public string Build => _e.GetProperty("build").GetString()!;
@ -18,13 +17,11 @@ namespace FunctionalTests {
public class InfoResponse : IFromJsonElement<InfoResponse> {
JsonElement _e;
public InfoResponse() { }
readonly JsonElement _e;
public InfoResponse(JsonElement e) => _e = e;
public InfoResponse Convert(JsonElement e) => new InfoResponse(e);
public static InfoResponse Convert(JsonElement e) => new(e);
public string ResourceGroup => _e.GetStringProperty("resource_group")!;
public string Region => _e.GetStringProperty("region")!;

View File

@ -5,13 +5,11 @@ using Xunit.Abstractions;
namespace FunctionalTests;
public class JobTaskInfo : IFromJsonElement<JobTaskInfo> {
JsonElement _e;
public JobTaskInfo() { }
readonly JsonElement _e;
public JobTaskInfo(JsonElement e) => _e = e;
public JobTaskInfo Convert(JsonElement e) => new JobTaskInfo(e);
public static JobTaskInfo Convert(JsonElement e) => new(e);
public Guid TaskId => _e.GetGuidProperty("task_id");
@ -22,12 +20,11 @@ public class JobTaskInfo : IFromJsonElement<JobTaskInfo> {
public class Job : IFromJsonElement<Job> {
JsonElement _e;
readonly JsonElement _e;
public Job() { }
public Job(JsonElement e) => _e = e;
public Job Convert(JsonElement e) => new Job(e);
public static Job Convert(JsonElement e) => new(e);
public Guid JobId => _e.GetGuidProperty("job_id");

View File

@ -6,9 +6,7 @@ using Xunit.Abstractions;
namespace FunctionalTests;
public class Node : IFromJsonElement<Node> {
JsonElement _e;
public Node() { }
readonly JsonElement _e;
public Node(JsonElement e) => _e = e;
@ -31,7 +29,7 @@ public class Node : IFromJsonElement<Node> {
public bool DeleteRequested => _e.GetBoolProperty("delete_requested");
public bool DebugKeepNode => _e.GetBoolProperty("debug_keep_node");
public Node Convert(JsonElement e) => new Node(e);
public static Node Convert(JsonElement e) => new(e);
}

View File

@ -5,13 +5,11 @@ using Xunit.Abstractions;
namespace FunctionalTests;
public class Notification : IFromJsonElement<Notification> {
JsonElement _e;
public Notification() { }
readonly JsonElement _e;
public Notification(JsonElement e) => _e = e;
public Notification Convert(JsonElement e) => new Notification(e);
public static Notification Convert(JsonElement e) => new(e);
public Guid NotificationId => _e.GetGuidProperty("notification_id");

View File

@ -7,11 +7,10 @@ namespace FunctionalTests;
public class Pool : IFromJsonElement<Pool> {
JsonElement _e;
readonly JsonElement _e;
public Pool() { }
public Pool(JsonElement e) => _e = e;
public Pool Convert(JsonElement e) => new Pool(e);
public static Pool Convert(JsonElement e) => new(e);
public string Name => _e.GetStringProperty("name");
public string PoolId => _e.GetStringProperty("pool_id");

View File

@ -4,8 +4,7 @@ using Xunit.Abstractions;
namespace FunctionalTests;
public class Proxy : IFromJsonElement<Proxy> {
JsonElement _e;
readonly JsonElement _e;
public Proxy() { }
public Proxy(JsonElement e) => _e = e;
@ -14,11 +13,11 @@ public class Proxy : IFromJsonElement<Proxy> {
public string VmState => _e.GetStringProperty("state");
public Proxy Convert(JsonElement e) => new Proxy(e);
public static Proxy Convert(JsonElement e) => new(e);
}
public class Forward : IFromJsonElement<Forward>, IComparable<Forward> {
JsonElement _e;
readonly JsonElement _e;
public Forward() { }
public Forward(JsonElement e) => _e = e;
@ -27,7 +26,7 @@ public class Forward : IFromJsonElement<Forward>, IComparable<Forward> {
public string DstIp => _e.GetStringProperty("dst_ip");
public Forward Convert(JsonElement e) => new Forward(e);
public static Forward Convert(JsonElement e) => new(e);
public int CompareTo(Forward? other) {
if (other == null) return 1;
@ -41,7 +40,7 @@ public class Forward : IFromJsonElement<Forward>, IComparable<Forward> {
}
public class ProxyGetResult : IFromJsonElement<ProxyGetResult>, IComparable<ProxyGetResult> {
JsonElement _e;
readonly JsonElement _e;
public ProxyGetResult() { }
@ -62,9 +61,9 @@ public class ProxyGetResult : IFromJsonElement<ProxyGetResult>, IComparable<Prox
}
}
public Forward Forward => new Forward(_e.GetProperty("forward"));
public Forward Forward => new(_e.GetProperty("forward"));
public ProxyGetResult Convert(JsonElement e) => new ProxyGetResult(e);
public static ProxyGetResult Convert(JsonElement e) => new(e);
public int CompareTo(ProxyGetResult? other) {

View File

@ -5,12 +5,11 @@ using Xunit.Abstractions;
namespace FunctionalTests;
public class ReproConfig : IFromJsonElement<ReproConfig> {
JsonElement _e;
readonly JsonElement _e;
public ReproConfig() { }
public ReproConfig(JsonElement e) => _e = e;
public ReproConfig Convert(JsonElement e) => new ReproConfig(e);
public static ReproConfig Convert(JsonElement e) => new(e);
public string Container => _e.GetStringProperty("container");
public string Path => _e.GetStringProperty("path");
@ -20,12 +19,11 @@ public class ReproConfig : IFromJsonElement<ReproConfig> {
public class Repro : IFromJsonElement<Repro> {
JsonElement _e;
readonly JsonElement _e;
public Repro() { }
public Repro(JsonElement e) => _e = e;
public Repro Convert(JsonElement e) => new Repro(e);
public static Repro Convert(JsonElement e) => new(e);
public Guid VmId => _e.GetGuidProperty("vm_id");

View File

@ -6,12 +6,11 @@ using Xunit.Abstractions;
namespace FunctionalTests;
public class ScalesetNodeState : IFromJsonElement<ScalesetNodeState> {
JsonElement _e;
readonly JsonElement _e;
public ScalesetNodeState() { }
public ScalesetNodeState(JsonElement e) => _e = e;
public ScalesetNodeState Convert(JsonElement e) => new ScalesetNodeState(e);
public static ScalesetNodeState Convert(JsonElement e) => new(e);
public Guid MachineId => _e.GetGuidProperty("machine_id");
public string InstanceId => _e.GetStringProperty("instance_id");
@ -20,10 +19,9 @@ public class ScalesetNodeState : IFromJsonElement<ScalesetNodeState> {
}
public class Scaleset : IFromJsonElement<Scaleset> {
JsonElement _e;
public Scaleset() { }
readonly JsonElement _e;
public Scaleset(JsonElement e) => _e = e;
public Scaleset Convert(JsonElement e) => new Scaleset(e);
public static Scaleset Convert(JsonElement e) => new(e);
public Guid ScalesetId => _e.GetGuidProperty("scaleset_id");
public string PoolName => _e.GetStringProperty("pool_name");

View File

@ -6,8 +6,7 @@ namespace FunctionalTests;
public class TaskDetails {
JsonElement _e;
public TaskDetails() { }
readonly JsonElement _e;
public TaskDetails(JsonElement e) => _e = e;
public string Type => _e.GetStringProperty("type");
@ -77,22 +76,20 @@ public class TaskDetails {
public class TaskConfig : IFromJsonElement<TaskConfig> {
JsonElement _e;
public TaskConfig() { }
readonly JsonElement _e;
public TaskConfig(JsonElement e) => _e = e;
public TaskConfig Convert(JsonElement e) => new TaskConfig(e);
public static TaskConfig Convert(JsonElement e) => new(e);
public Guid JobId => _e.GetGuidProperty("job_id");
public IEnumerable<Guid>? PrereqTasks => _e.GetEnumerableGuidProperty("prereq_tasks");
}
public class OneFuzzTask : IFromJsonElement<OneFuzzTask> {
JsonElement _e;
readonly JsonElement _e;
public OneFuzzTask() { }
public OneFuzzTask(JsonElement e) => _e = e;
public OneFuzzTask Convert(JsonElement e) => new OneFuzzTask(e);
public static OneFuzzTask Convert(JsonElement e) => new(e);
public Guid JobId => _e.GetGuidProperty("job_id");
public Guid TaskId => _e.GetGuidProperty("task_id");

View File

@ -4,13 +4,11 @@ namespace FunctionalTests;
public class UserInfo : IFromJsonElement<UserInfo> {
JsonElement _e;
public UserInfo() { }
readonly JsonElement _e;
public UserInfo(JsonElement e) => _e = e;
public UserInfo Convert(JsonElement e) => new UserInfo(e);
public static UserInfo Convert(JsonElement e) => new(e);
public Guid? ApplicationId => _e.GetNullableGuidProperty("application_id");
public Guid? ObjectId => _e.GetNullableGuidProperty("object_id");
public string? Upn => _e.GetNullableStringProperty("upn");
}

View File

@ -69,9 +69,9 @@ public abstract class AgentRegistrationTestsBase : FunctionTestBase {
var result = await func.Run(req);
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var body = BodyAs<Error>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST, body.Code);
Assert.Equal("'machine_id' query parameter must be provided", body.Errors?.Single());
var body = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST.ToString(), body.Title);
Assert.Equal("'machine_id' query parameter must be provided", body.Detail);
}
[Fact]
@ -85,9 +85,9 @@ public abstract class AgentRegistrationTestsBase : FunctionTestBase {
var result = await func.Run(req);
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var body = BodyAs<Error>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST, body.Code);
Assert.Contains("unable to find a registration", body.Errors?.Single());
var body = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST.ToString(), body.Title);
Assert.Contains("unable to find a registration", body.Detail);
}
[Fact]
@ -104,9 +104,9 @@ public abstract class AgentRegistrationTestsBase : FunctionTestBase {
var result = await func.Run(req);
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var body = BodyAs<Error>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST, body.Code);
Assert.Contains("unable to find a pool", body.Errors?.Single());
var body = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST.ToString(), body.Title);
Assert.Contains("unable to find a pool", body.Detail);
}
[Fact]
@ -220,8 +220,8 @@ public abstract class AgentRegistrationTestsBase : FunctionTestBase {
var result = await func.Run(req);
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST, err.Code);
Assert.Equal($"'{parameterToSkip}' query parameter must be provided", err.Errors?.Single());
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST.ToString(), err.Title);
Assert.Equal($"'{parameterToSkip}' query parameter must be provided", err.Detail);
}
}

View File

@ -41,8 +41,8 @@ public abstract class ContainersTestBase : FunctionTestBase {
var result = await func.Run(TestHttpRequestData.Empty(method));
Assert.Equal(HttpStatusCode.Unauthorized, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED, err.Code);
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED.ToString(), err.Title);
}
@ -174,4 +174,19 @@ public abstract class ContainersTestBase : FunctionTestBase {
}
using var r = await client.DeleteBlobAsync("blob"); // delete
}
[Fact]
public async Async.Task BadContainerNameProducesGoodErrorMessage() {
// use anonymous type so we can send an invalid name
var msg = TestHttpRequestData.FromJson("POST", new { Name = "AbCd" });
var auth = new TestEndpointAuthorization(RequestType.User, Logger, Context);
var func = new ContainersFunction(Logger, auth, Context);
var result = await func.Run(msg);
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var details = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST.ToString(), details.Title);
Assert.StartsWith("Unable to parse 'AbCd' as a Container: Container name must", details.Detail);
}
}

View File

@ -34,8 +34,8 @@ public abstract class DownloadTestBase : FunctionTestBase {
var result = await func.Run(TestHttpRequestData.Empty("GET"));
Assert.Equal(HttpStatusCode.Unauthorized, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED, err.Code);
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED.ToString(), err.Title);
}
[Fact]
@ -49,8 +49,8 @@ public abstract class DownloadTestBase : FunctionTestBase {
var result = await func.Run(req);
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST, err.Code);
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST.ToString(), err.Title);
}
[Fact]
@ -65,8 +65,8 @@ public abstract class DownloadTestBase : FunctionTestBase {
var result = await func.Run(req);
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST, err.Code);
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.INVALID_REQUEST.ToString(), err.Title);
}
[Fact]

View File

@ -40,8 +40,8 @@ public abstract class JobsTestBase : FunctionTestBase {
var result = await func.Run(TestHttpRequestData.Empty(method));
Assert.Equal(HttpStatusCode.Unauthorized, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED, err.Code);
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED.ToString(), err.Title);
}
[Fact]
@ -52,8 +52,8 @@ public abstract class JobsTestBase : FunctionTestBase {
var result = await func.Run(TestHttpRequestData.FromJson("DELETE", new JobGet(_jobId)));
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.INVALID_JOB, err.Code);
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.INVALID_JOB.ToString(), err.Title);
}
[Fact]

View File

@ -1,7 +1,6 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using IntegrationTests.Fakes;
using Microsoft.OneFuzz.Service;
@ -171,9 +170,9 @@ public abstract class NodeTestBase : FunctionTestBase {
var result = await func.Run(TestHttpRequestData.FromJson(method, req));
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED, err.Code);
Assert.Contains("pool modification disabled", err.Errors?.Single());
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED.ToString(), err.Title);
Assert.Contains("pool modification disabled", err.Detail);
}
[Theory]
@ -201,7 +200,7 @@ public abstract class NodeTestBase : FunctionTestBase {
// we will fail with BadRequest but due to not being able to find the Node,
// not because of UNAUTHORIZED
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
Assert.Equal(ErrorCode.UNABLE_TO_FIND, BodyAs<Error>(result).Code);
Assert.Equal(ErrorCode.UNABLE_TO_FIND.ToString(), BodyAs<ProblemDetails>(result).Title);
}
[Theory]
@ -231,7 +230,7 @@ public abstract class NodeTestBase : FunctionTestBase {
// we will fail with BadRequest but due to not being able to find the Node,
// not because of UNAUTHORIZED
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
Assert.Equal(ErrorCode.UNABLE_TO_FIND, BodyAs<Error>(result).Code);
Assert.Equal(ErrorCode.UNABLE_TO_FIND.ToString(), BodyAs<ProblemDetails>(result).Title);
}
[Theory]
@ -260,9 +259,9 @@ public abstract class NodeTestBase : FunctionTestBase {
var result = await func.Run(TestHttpRequestData.FromJson(method, req));
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED, err.Code);
Assert.Contains("not authorized to manage instance", err.Errors?.Single());
var err = BodyAs<ProblemDetails>(result);
Assert.Equal(ErrorCode.UNAUTHORIZED.ToString(), err.Title);
Assert.Contains("not authorized to manage instance", err.Detail);
}
[Theory]

View File

@ -252,7 +252,7 @@ public abstract class PoolTestBase : FunctionTestBase {
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
// should get an error back
var returnedPool = BodyAs<Error>(result);
Assert.Contains(returnedPool.Errors, c => c == "pool with that name already exists");
var returnedPool = BodyAs<ProblemDetails>(result);
Assert.Contains("pool with that name already exists", returnedPool.Detail);
}
}

View File

@ -1,6 +1,5 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using IntegrationTests.Fakes;
using Microsoft.OneFuzz.Service;
@ -51,8 +50,8 @@ public abstract class ScalesetTestBase : FunctionTestBase {
var result = await func.Run(TestHttpRequestData.FromJson("GET", req));
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal("unable to find scaleset", err.Errors?.Single());
var err = BodyAs<ProblemDetails>(result);
Assert.Equal("unable to find scaleset", err.Detail);
}
[Fact]

View File

@ -45,8 +45,8 @@ public abstract class TasksTestBase : FunctionTestBase {
var testData = new TestHttpRequestData("POST", new BinaryData(JsonSerializer.SerializeToUtf8Bytes(serialized, EntityConverter.GetJsonSerializerOptions())));
var result = await func.Run(testData);
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(new[] { "Unexpected property: \"vm\"" }, err.Errors);
var err = BodyAs<ProblemDetails>(result);
Assert.Equal("Unexpected property: \"vm\"", err.Detail);
}
[Fact]
@ -66,8 +66,8 @@ public abstract class TasksTestBase : FunctionTestBase {
var result = await func.Run(TestHttpRequestData.FromJson("POST", req));
Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode);
var err = BodyAs<Error>(result);
Assert.Equal(new[] { "The Pool field is required." }, err.Errors);
var err = BodyAs<ProblemDetails>(result);
Assert.Equal("The Pool field is required.", err.Detail);
}
[Fact]

View File

@ -88,12 +88,12 @@ namespace Tests {
public static Gen<PoolName> PoolNameGen { get; }
= from name in Arb.Generate<NonEmptyString>()
where PoolName.TryParse(name.Get, out _)
where PoolName.IsValid(name.Get)
select PoolName.Parse(name.Get);
public static Gen<Region> RegionGen { get; }
= from name in Arb.Generate<NonEmptyString>()
where Region.TryParse(name.Get, out _)
where Region.IsValid(name.Get)
select Region.Parse(name.Get);
public static Gen<Node> Node { get; }
@ -365,7 +365,7 @@ namespace Tests {
from len in Gen.Choose(3, 63)
from name in Gen.ArrayOf(len, Gen.Elements<char>("abcdefghijklmnopqrstuvwxyz0123456789-"))
let nameString = new string(name)
where Container.TryParse(nameString, out var _)
where Container.IsValid(nameString)
select Container.Parse(nameString);
public static Gen<ADODuplicateTemplate> AdoDuplicateTemplate() {

View File

@ -31,7 +31,7 @@ public class ValidatedStringTests {
[InlineData("container-Name", false)] // can't have capitals
[InlineData("container-name-09", true)] // can have numbers
public void ContainerNames(string name, bool valid) {
Assert.Equal(valid, Container.TryParse(name, out var _));
Assert.Equal(valid, Container.IsValid(name));
}
[Theory(Skip = "Validation is disabled for now")]
@ -40,6 +40,6 @@ public class ValidatedStringTests {
[InlineData("Default-Ubuntu20.04-Standard_D2", true)]
[InlineData("Default!", false)]
public void PoolNames(string name, bool valid) {
Assert.Equal(valid, PoolName.TryParse(name, out var _));
Assert.Equal(valid, PoolName.IsValid(name));
}
}

View File

@ -82,9 +82,9 @@ def check_application_error(response: requests.Response) -> None:
if response.status_code == 401:
try:
as_json = json.loads(response.content)
if isinstance(as_json, dict) and "code" in as_json and "errors" in as_json:
if isinstance(as_json, dict) and "title" in as_json and "detail" in as_json:
raise Exception(
f"request failed: application error - {as_json['code']} {as_json['errors']}"
f"request failed: application error (401: {as_json['title']}): {as_json['detail']}"
)
except json.decoder.JSONDecodeError:
pass
@ -351,7 +351,20 @@ class Backend:
if response is None:
raise Exception("request failed: %s %s" % (method, url))
if response.status_code / 100 != 2:
if response.status_code // 100 != 2:
try:
json = response.json()
except requests.exceptions.JSONDecodeError:
pass
# attempt to read as https://www.rfc-editor.org/rfc/rfc7807
if isinstance(json, Dict):
title = json.get("title")
details = json.get("detail")
raise Exception(
f"request did not succeed ({response.status_code}: {title}): {details}"
)
error_text = str(
response.content, encoding="utf-8", errors="backslashreplace"
)
@ -359,6 +372,7 @@ class Backend:
"request did not succeed: HTTP %s - %s"
% (response.status_code, error_text)
)
return response