Skip to content

Commit 4ddda24

Browse files
alexeyzimarevalexeyzimarev
authored
Header value null check (#2241)
* Throw an exception when adding a header with `null` value * Add multiple header values properly * Default headers should allow multiple values
1 parent dd52ff6 commit 4ddda24

22 files changed

+172
-94
lines changed

‎Directory.Packages.props

+7-7
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
<PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
1717
<PackageVersion Include="CsvHelper" Version="33.0.1" />
1818
<PackageVersion Include="PolySharp" Version="1.14.1" />
19-
<PackageVersion Include="System.Text.Json" Version="8.0.3" />
20-
<PackageVersion Include="WireMock.Net" Version="1.5.51" />
19+
<PackageVersion Include="System.Text.Json" Version="8.0.4" />
20+
<PackageVersion Include="WireMock.Net" Version="1.5.60" />
2121
<PackageVersion Include="WireMock.Net.FluentAssertions" Version="1.5.51" />
2222
</ItemGroup>
2323
<ItemGroup Label="Compile dependencies">
@@ -28,22 +28,22 @@
2828
<PackageVersion Include="Nullable" Version="1.3.1" />
2929
<PackageVersion Include="Microsoft.NETFramework.ReferenceAssemblies.net472" Version="1.0.3" />
3030
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
31-
<PackageVersion Include="JetBrains.Annotations" Version="2023.3.0" />
31+
<PackageVersion Include="JetBrains.Annotations" Version="2024.2.0" />
3232
</ItemGroup>
3333
<ItemGroup Label="Testing dependencies">
3434
<PackageVersion Include="AutoFixture" Version="4.18.1" />
3535
<PackageVersion Include="coverlet.collector" Version="6.0.2" />
3636
<PackageVersion Include="FluentAssertions" Version="6.12.0" />
3737
<PackageVersion Include="HttpTracer" Version="2.1.1" />
3838
<PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="$(MicrosoftTestHostVer)" />
39-
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
39+
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
4040
<PackageVersion Include="Moq" Version="4.20.70" />
41-
<PackageVersion Include="Polly" Version="8.3.1" />
41+
<PackageVersion Include="Polly" Version="8.4.1" />
4242
<PackageVersion Include="rest-mock-core" Version="0.7.12" />
4343
<PackageVersion Include="RichardSzalay.MockHttp" Version="7.0.0" />
4444
<PackageVersion Include="System.Net.Http.Json" Version="8.0.0" />
4545
<PackageVersion Include="Xunit.Extensions.Logging" Version="1.1.0" />
46-
<PackageVersion Include="xunit.runner.visualstudio" Version="2.5.7" PrivateAssets="All" />
47-
<PackageVersion Include="xunit" Version="2.8.1" />
46+
<PackageVersion Include="xunit.runner.visualstudio" Version="2.8.2" PrivateAssets="All" />
47+
<PackageVersion Include="xunit" Version="2.9.0" />
4848
</ItemGroup>
4949
</Project>

‎gen/SourceGenerator/ImmutableGenerator.cs

+14-11
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,24 @@ public void Execute(GeneratorExecutionContext context) {
3232

3333
static string GenerateImmutableClass(TypeDeclarationSyntax mutableClass, Compilation compilation) {
3434
var containingNamespace = compilation.GetSemanticModel(mutableClass.SyntaxTree).GetDeclaredSymbol(mutableClass)!.ContainingNamespace;
35-
36-
var namespaceName = containingNamespace.ToDisplayString();
37-
38-
var className = mutableClass.Identifier.Text;
39-
40-
var usings = mutableClass.SyntaxTree.GetCompilationUnitRoot().Usings.Select(u => u.ToString());
35+
var namespaceName = containingNamespace.ToDisplayString();
36+
var className = mutableClass.Identifier.Text;
37+
var usings = mutableClass.SyntaxTree.GetCompilationUnitRoot().Usings.Select(u => u.ToString());
4138

4239
var properties = GetDefinitions(SyntaxKind.SetKeyword)
43-
.Select(prop => $" public {prop.Type} {prop.Identifier.Text} {{ get; }}")
40+
.Select(
41+
prop => {
42+
var xml = prop.GetLeadingTrivia().FirstOrDefault(x => x.IsKind(SyntaxKind.SingleLineDocumentationCommentTrivia)).GetStructure();
43+
return $"/// {xml} public {prop.Type} {prop.Identifier.Text} {{ get; }}";
44+
}
45+
)
4446
.ToArray();
4547

4648
var props = GetDefinitions(SyntaxKind.SetKeyword).ToArray();
4749

4850
const string argName = "inner";
49-
var mutableProperties = props
50-
.Select(prop => $" {prop.Identifier.Text} = {argName}.{prop.Identifier.Text};");
51+
52+
var mutableProperties = props.Select(prop => $" {prop.Identifier.Text} = {argName}.{prop.Identifier.Text};");
5153

5254
var constructor = $$"""
5355
public ReadOnly{{className}}({{className}} {{argName}}) {
@@ -85,7 +87,8 @@ IEnumerable<PropertyDeclarationSyntax> GetDefinitions(SyntaxKind kind)
8587
.OfType<PropertyDeclarationSyntax>()
8688
.Where(
8789
prop =>
88-
prop.AccessorList!.Accessors.Any(accessor => accessor.Keyword.IsKind(kind)) && prop.AttributeLists.All(list => list.Attributes.All(attr => attr.Name.ToString() != "Exclude"))
90+
prop.AccessorList!.Accessors.Any(accessor => accessor.Keyword.IsKind(kind)) &&
91+
prop.AttributeLists.All(list => list.Attributes.All(attr => attr.Name.ToString() != "Exclude"))
8992
);
9093
}
91-
}
94+
}

‎src/RestSharp/Authenticators/JwtAuthenticator.cs

+3-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
namespace RestSharp.Authenticators;
15+
namespace RestSharp.Authenticators;
1616

1717
/// <summary>
1818
/// JSON WEB TOKEN (JWT) Authenticator class.
@@ -26,7 +26,8 @@ public class JwtAuthenticator(string accessToken) : AuthenticatorBase(GetToken(a
2626
[PublicAPI]
2727
public void SetBearerToken(string accessToken) => Token = GetToken(accessToken);
2828

29-
static string GetToken(string accessToken) => Ensure.NotEmpty(accessToken, nameof(accessToken)).StartsWith("Bearer ") ? accessToken : $"Bearer {accessToken}";
29+
static string GetToken(string accessToken)
30+
=> Ensure.NotEmptyString(accessToken, nameof(accessToken)).StartsWith("Bearer ") ? accessToken : $"Bearer {accessToken}";
3031

3132
protected override ValueTask<Parameter> GetAuthenticationParameter(string accessToken)
3233
=> new(new HeaderParameter(KnownHeaders.Authorization, accessToken));

‎src/RestSharp/Authenticators/OAuth/OAuthWorkflow.cs

+6-6
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ sealed class OAuthWorkflow {
4848
/// <param name="parameters">Any existing, non-OAuth query parameters desired in the request</param>
4949
/// <returns></returns>
5050
public OAuthParameters BuildRequestTokenSignature(string method, WebPairCollection parameters) {
51-
Ensure.NotEmpty(ConsumerKey, nameof(ConsumerKey));
51+
Ensure.NotEmptyString(ConsumerKey, nameof(ConsumerKey));
5252

5353
var allParameters = new WebPairCollection();
5454
allParameters.AddRange(parameters);
@@ -76,8 +76,8 @@ public OAuthParameters BuildRequestTokenSignature(string method, WebPairCollecti
7676
/// <param name="method">The HTTP method for the intended request</param>
7777
/// <param name="parameters">Any existing, non-OAuth query parameters desired in the request</param>
7878
public OAuthParameters BuildAccessTokenSignature(string method, WebPairCollection parameters) {
79-
Ensure.NotEmpty(ConsumerKey, nameof(ConsumerKey));
80-
Ensure.NotEmpty(Token, nameof(Token));
79+
Ensure.NotEmptyString(ConsumerKey, nameof(ConsumerKey));
80+
Ensure.NotEmptyString(Token, nameof(Token));
8181

8282
var allParameters = new WebPairCollection();
8383
allParameters.AddRange(parameters);
@@ -105,8 +105,8 @@ public OAuthParameters BuildAccessTokenSignature(string method, WebPairCollectio
105105
/// <param name="method">The HTTP method for the intended request</param>
106106
/// <param name="parameters">Any existing, non-OAuth query parameters desired in the request</param>
107107
public OAuthParameters BuildClientAuthAccessTokenSignature(string method, WebPairCollection parameters) {
108-
Ensure.NotEmpty(ConsumerKey, nameof(ConsumerKey));
109-
Ensure.NotEmpty(ClientUsername, nameof(ClientUsername));
108+
Ensure.NotEmptyString(ConsumerKey, nameof(ConsumerKey));
109+
Ensure.NotEmptyString(ClientUsername, nameof(ClientUsername));
110110

111111
var allParameters = new WebPairCollection();
112112
allParameters.AddRange(parameters);
@@ -127,7 +127,7 @@ public OAuthParameters BuildClientAuthAccessTokenSignature(string method, WebPai
127127
}
128128

129129
public OAuthParameters BuildProtectedResourceSignature(string method, WebPairCollection parameters) {
130-
Ensure.NotEmpty(ConsumerKey, nameof(ConsumerKey));
130+
Ensure.NotEmptyString(ConsumerKey, nameof(ConsumerKey));
131131

132132
var allParameters = new WebPairCollection();
133133
allParameters.AddRange(parameters);

‎src/RestSharp/Ensure.cs

+3-4
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,10 @@ namespace RestSharp;
1717
static class Ensure {
1818
public static T NotNull<T>(T? value, [InvokerParameterName] string name) => value ?? throw new ArgumentNullException(name);
1919

20-
public static string NotEmpty(string? value, [InvokerParameterName] string name)
21-
=> string.IsNullOrWhiteSpace(value) ? throw new ArgumentNullException(name) : value!;
22-
2320
public static string NotEmptyString(object? value, [InvokerParameterName] string name) {
2421
var s = value as string ?? value?.ToString();
25-
return string.IsNullOrWhiteSpace(s) ? throw new ArgumentNullException(name) : s!;
22+
if (s == null) throw new ArgumentNullException(name);
23+
24+
return string.IsNullOrWhiteSpace(s) ? throw new ArgumentException("Parameter cannot be an empty string", name) : s;
2625
}
2726
}

‎src/RestSharp/Options/RestClientOptions.cs

+1
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ public int MaxTimeout {
220220

221221
/// <summary>
222222
/// Set to true to allow multiple default parameters with the same name. Default is false.
223+
/// This setting doesn't apply to headers as multiple header values for the same key is allowed.
223224
/// </summary>
224225
public bool AllowMultipleDefaultParametersWithSameName { get; set; }
225226

‎src/RestSharp/Parameters/DefaultParameters.cs

+4-5
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,11 @@ public sealed class DefaultParameters(ReadOnlyRestClientOptions options) : Param
2828
[MethodImpl(MethodImplOptions.Synchronized)]
2929
public DefaultParameters AddParameter(Parameter parameter) {
3030
if (parameter.Type == ParameterType.RequestBody)
31-
throw new NotSupportedException(
32-
"Cannot set request body using default parameters. Use Request.AddBody() instead."
33-
);
31+
throw new NotSupportedException("Cannot set request body using default parameters. Use Request.AddBody() instead.");
3432

3533
if (!options.AllowMultipleDefaultParametersWithSameName &&
36-
!MultiParameterTypes.Contains(parameter.Type) &&
34+
parameter.Type != ParameterType.HttpHeader &&
35+
!MultiParameterTypes.Contains(parameter.Type) &&
3736
this.Any(x => x.Name == parameter.Name)) {
3837
throw new ArgumentException("A default parameters with the same name has already been added", nameof(parameter));
3938
}
@@ -70,4 +69,4 @@ public DefaultParameters ReplaceParameter(Parameter parameter)
7069
.AddParameter(parameter);
7170

7271
static readonly ParameterType[] MultiParameterTypes = [ParameterType.QueryString, ParameterType.GetOrPost];
73-
}
72+
}

‎src/RestSharp/Parameters/HeaderParameter.cs

+10-1
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,14 @@ public record HeaderParameter : Parameter {
2121
/// </summary>
2222
/// <param name="name">Parameter name</param>
2323
/// <param name="value">Parameter value</param>
24-
public HeaderParameter(string? name, string? value) : base(name, value, ParameterType.HttpHeader, false) { }
24+
public HeaderParameter(string name, string value)
25+
: base(
26+
Ensure.NotEmptyString(name, nameof(name)),
27+
Ensure.NotNull(value, nameof(value)),
28+
ParameterType.HttpHeader,
29+
false
30+
) { }
31+
32+
public new string Name => base.Name!;
33+
public new string Value => (string)base.Value!;
2534
}

‎src/RestSharp/Parameters/Parameter.cs

+37-3
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,66 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
using System.Diagnostics;
16+
1517
namespace RestSharp;
1618

1719
/// <summary>
1820
/// Parameter container for REST requests
1921
/// </summary>
20-
public abstract record Parameter(string? Name, object? Value, ParameterType Type, bool Encode) {
22+
[DebuggerDisplay($"{{{nameof(DebuggerDisplay)}()}}")]
23+
public abstract record Parameter {
24+
/// <summary>
25+
/// Parameter container for REST requests
26+
/// </summary>
27+
protected Parameter(string? name, object? value, ParameterType type, bool encode) {
28+
Name = name;
29+
Value = value;
30+
Type = type;
31+
Encode = encode;
32+
}
33+
2134
/// <summary>
2235
/// MIME content type of the parameter
2336
/// </summary>
2437
public ContentType ContentType { get; protected init; } = ContentType.Undefined;
38+
public string? Name { get; }
39+
public object? Value { get; }
40+
public ParameterType Type { get; }
41+
public bool Encode { get; }
2542

2643
/// <summary>
2744
/// Return a human-readable representation of this parameter
2845
/// </summary>
2946
/// <returns>String</returns>
30-
public sealed override string ToString() => Value == null ? $"{Name}" : $"{Name}={Value}";
47+
public sealed override string ToString() => Value == null ? $"{Name}" : $"{Name}={ValueString}";
48+
49+
protected virtual string ValueString => Value?.ToString() ?? "null";
3150

3251
public static Parameter CreateParameter(string? name, object? value, ParameterType type, bool encode = true)
3352
// ReSharper disable once SwitchExpressionHandlesSomeKnownEnumValuesWithExceptionInDefault
3453
=> type switch {
3554
ParameterType.GetOrPost => new GetOrPostParameter(Ensure.NotEmptyString(name, nameof(name)), value?.ToString(), encode),
3655
ParameterType.UrlSegment => new UrlSegmentParameter(Ensure.NotEmptyString(name, nameof(name)), value?.ToString()!, encode),
37-
ParameterType.HttpHeader => new HeaderParameter(name, value?.ToString()),
56+
ParameterType.HttpHeader => new HeaderParameter(name!, value?.ToString()!),
3857
ParameterType.QueryString => new QueryParameter(Ensure.NotEmptyString(name, nameof(name)), value?.ToString(), encode),
3958
_ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
4059
};
60+
61+
[PublicAPI]
62+
public void Deconstruct(out string? name, out object? value, out ParameterType type, out bool encode) {
63+
name = Name;
64+
value = Value;
65+
type = Type;
66+
encode = Encode;
67+
}
68+
69+
/// <summary>
70+
/// Assists with debugging by displaying in the debugger output
71+
/// </summary>
72+
/// <returns></returns>
73+
[UsedImplicitly]
74+
protected string DebuggerDisplay() => $"{GetType().Name.Replace("Parameter", "")} {ToString()}";
4175
}
4276

4377
public record NamedParameter : Parameter {

‎src/RestSharp/Parameters/ParametersCollection.cs

+11-9
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,27 @@
1717

1818
namespace RestSharp;
1919

20-
public abstract class ParametersCollection : IReadOnlyCollection<Parameter> {
21-
protected readonly List<Parameter> Parameters = [];
20+
public abstract class ParametersCollection<T> : IReadOnlyCollection<T> where T : Parameter {
21+
protected readonly List<T> Parameters = [];
2222

2323
// public ParametersCollection(IEnumerable<Parameter> parameters) => _parameters.AddRange(parameters);
2424

25-
static readonly Func<Parameter, string?, bool> SearchPredicate = (p, name)
25+
static readonly Func<T, string?, bool> SearchPredicate = (p, name)
2626
=> p.Name != null && p.Name.Equals(name, StringComparison.InvariantCultureIgnoreCase);
2727

28-
public bool Exists(Parameter parameter) => Parameters.Any(p => SearchPredicate(p, parameter.Name) && p.Type == parameter.Type);
28+
public bool Exists(T parameter) => Parameters.Any(p => SearchPredicate(p, parameter.Name) && p.Type == parameter.Type);
2929

30-
public Parameter? TryFind(string parameterName) => Parameters.FirstOrDefault(x => SearchPredicate(x, parameterName));
30+
public T? TryFind(string parameterName) => Parameters.FirstOrDefault(x => SearchPredicate(x, parameterName));
3131

32-
public IEnumerable<Parameter> GetParameters(ParameterType parameterType) => Parameters.Where(x => x.Type == parameterType);
33-
34-
public IEnumerable<T> GetParameters<T>() where T : class => Parameters.OfType<T>();
32+
public IEnumerable<TParameter> GetParameters<TParameter>() where TParameter : class, T => Parameters.OfType<TParameter>();
3533

36-
public IEnumerator<Parameter> GetEnumerator() => Parameters.GetEnumerator();
34+
public IEnumerator<T> GetEnumerator() => Parameters.GetEnumerator();
3735

3836
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
3937

4038
public int Count => Parameters.Count;
39+
}
40+
41+
public abstract class ParametersCollection : ParametersCollection<Parameter> {
42+
public IEnumerable<Parameter> GetParameters(ParameterType parameterType) => Parameters.Where(x => x.Type == parameterType);
4143
}

‎src/RestSharp/Parameters/RequestParameters.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public ParametersCollection AddParameters(IEnumerable<Parameter> parameters) {
4242
}
4343

4444
/// <summary>
45-
/// Add parameters from another parameters collection
45+
/// Add parameters from another parameter collection
4646
/// </summary>
4747
/// <param name="parameters"></param>
4848
/// <returns></returns>

‎src/RestSharp/Parameters/UrlSegmentParameter.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public partial record UrlSegmentParameter : NamedParameter {
2929
public UrlSegmentParameter(string name, string value, bool encode = true)
3030
: base(
3131
name,
32-
RegexPattern.Replace(Ensure.NotEmpty(value, nameof(value)), "/"),
32+
RegexPattern.Replace(Ensure.NotEmptyString(value, nameof(value)), "/"),
3333
ParameterType.UrlSegment,
3434
encode
3535
) { }

‎src/RestSharp/Request/HttpRequestMessageExtensions.cs

+6-6
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,16 @@ namespace RestSharp;
2020

2121
static class HttpRequestMessageExtensions {
2222
public static void AddHeaders(this HttpRequestMessage message, RequestHeaders headers) {
23-
var headerParameters = headers.Parameters.Where(x => !KnownHeaders.IsContentHeader(x.Name!));
23+
var headerParameters = headers.Where(x => !KnownHeaders.IsContentHeader(x.Name));
2424

25-
headerParameters.ForEach(x => AddHeader(x, message.Headers));
25+
headerParameters.GroupBy(x => x.Name).ForEach(x => AddHeader(x, message.Headers));
2626
return;
2727

28-
void AddHeader(Parameter parameter, HttpHeaders httpHeaders) {
29-
var parameterStringValue = parameter.Value!.ToString();
28+
void AddHeader(IGrouping<string, HeaderParameter> group, HttpHeaders httpHeaders) {
29+
var parameterStringValues = group.Select(x => x.Value);
3030

31-
httpHeaders.Remove(parameter.Name!);
32-
httpHeaders.TryAddWithoutValidation(parameter.Name!, parameterStringValue);
31+
httpHeaders.Remove(group.Key);
32+
httpHeaders.TryAddWithoutValidation(group.Key, parameterStringValues);
3333
}
3434
}
3535
}

‎src/RestSharp/Request/RequestHeaders.cs

+7-9
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,17 @@
1919

2020
namespace RestSharp;
2121

22-
class RequestHeaders {
23-
public RequestParameters Parameters { get; } = new();
24-
22+
class RequestHeaders : ParametersCollection<HeaderParameter> {
2523
public RequestHeaders AddHeaders(ParametersCollection parameters) {
26-
Parameters.AddParameters(parameters.GetParameters<HeaderParameter>());
24+
Parameters.AddRange(parameters.GetParameters<HeaderParameter>());
2725
return this;
2826
}
2927

3028
// Add Accept header based on registered deserializers if the caller has set none.
3129
public RequestHeaders AddAcceptHeader(string[] acceptedContentTypes) {
32-
if (Parameters.TryFind(KnownHeaders.Accept) == null) {
30+
if (TryFind(KnownHeaders.Accept) == null) {
3331
var accepts = acceptedContentTypes.JoinToString(", ");
34-
Parameters.AddParameter(new HeaderParameter(KnownHeaders.Accept, accepts));
32+
Parameters.Add(new(KnownHeaders.Accept, accepts));
3533
}
3634

3735
return this;
@@ -46,13 +44,13 @@ public RequestHeaders AddCookieHeaders(Uri uri, CookieContainer? cookieContainer
4644
if (string.IsNullOrWhiteSpace(cookies)) return this;
4745

4846
var newCookies = SplitHeader(cookies);
49-
var existing = Parameters.GetParameters<HeaderParameter>().FirstOrDefault(x => x.Name == KnownHeaders.Cookie);
47+
var existing = GetParameters<HeaderParameter>().FirstOrDefault(x => x.Name == KnownHeaders.Cookie);
5048

5149
if (existing?.Value != null) {
52-
newCookies = newCookies.Union(SplitHeader(existing.Value.ToString()!));
50+
newCookies = newCookies.Union(SplitHeader(existing.Value!));
5351
}
5452

55-
Parameters.AddParameter(new HeaderParameter(KnownHeaders.Cookie, string.Join("; ", newCookies)));
53+
Parameters.Add(new(KnownHeaders.Cookie, string.Join("; ", newCookies)));
5654

5755
return this;
5856

0 commit comments

Comments
 (0)