Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't let Uri to escape query #2208

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/RestSharp/BuildUriExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ string EncodeParameter(Parameter parameter)
}

static void DoBuildUriValidations(IRestClient client, RestRequest request) {
if (client.Options.BaseUrl == null && !request.Resource.ToLowerInvariant().StartsWith("http"))
if (client.Options.BaseUrl == null && !request.Resource.StartsWith("http", StringComparison.InvariantCultureIgnoreCase))
throw new ArgumentOutOfRangeException(
nameof(request),
"Request resource doesn't contain a valid scheme for an empty base URL of the client"
Expand Down
60 changes: 60 additions & 0 deletions src/RestSharp/Request/Parsers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright (c) .NET Foundation and Contributors
//
// Licensed 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;
using System.Web;

namespace RestSharp;

static class Parsers {
// ReSharper disable once CognitiveComplexity
public static IEnumerable<KeyValuePair<string, string?>> ParseQueryString(string query, Encoding encoding) {
Ensure.NotNull(query, nameof(query));
Ensure.NotNull(encoding, nameof(encoding));
var length = query.Length;
var startIndex1 = query[0] == '?' ? 1 : 0;

if (length == startIndex1)
yield break;

while (startIndex1 <= length) {
var startIndex2 = -1;
var num = -1;

for (var index = startIndex1; index < length; ++index) {
if (startIndex2 == -1 && query[index] == '=')
startIndex2 = index + 1;
else if (query[index] == '&') {
num = index;
break;
}
}

string? name;

if (startIndex2 == -1) {
name = null;
startIndex2 = startIndex1;
}
else
name = HttpUtility.UrlDecode(query.Substring(startIndex1, startIndex2 - startIndex1 - 1), encoding);

if (num < 0)
num = query.Length;
startIndex1 = num + 1;
var str = HttpUtility.UrlDecode(query.Substring(startIndex2, num - startIndex2), encoding);
yield return new KeyValuePair<string, string?>(name ?? "", str);
}
}
}
51 changes: 24 additions & 27 deletions src/RestSharp/Request/RestRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System.Net;
using System.Net.Http.Headers;
using RestSharp.Authenticators;
using RestSharp.Extensions;
using RestSharp.Interceptors;
using System.Text;
using System.Web;

// ReSharper disable ReplaceSubstringWithRangeIndexer
// ReSharper disable UnusedAutoPropertyAccessor.Global

namespace RestSharp;

using Authenticators;
using Extensions;
using Interceptors;

/// <summary>
/// Container for data used to make requests
/// </summary>
Expand All @@ -39,35 +41,30 @@ public class RestRequest {
/// Constructor for a rest request to a relative resource URL and optional method
/// </summary>
/// <param name="resource">Resource to use</param>
/// <param name="method">Method to use (defaults to Method.Get></param>
/// <param name="method">Method to use. Default is Method.Get.</param>
public RestRequest(string? resource, Method method = Method.Get) : this() {
Resource = resource ?? "";
Method = method;
Method = method;

if (string.IsNullOrWhiteSpace(resource)) return;
if (string.IsNullOrWhiteSpace(resource)) {
Resource = "";
return;
}

var queryStringStart = Resource.IndexOf('?');

if (queryStringStart < 0 || Resource.IndexOf('=') <= queryStringStart) return;
if (queryStringStart < 0 || Resource.IndexOf('=') <= queryStringStart) {
return;
}

var queryParams = ParseQuery(Resource.Substring(queryStringStart + 1));
var queryString = Resource.Substring(queryStringStart + 1);
Resource = Resource.Substring(0, queryStringStart);

foreach (var param in queryParams) this.AddQueryParameter(param.Key, param.Value, false);

return;

static IEnumerable<KeyValuePair<string, string?>> ParseQuery(string query)
=> query.Split(new[] { '&' }, StringSplitOptions.RemoveEmptyEntries)
.Select(
x => {
var position = x.IndexOf('=');
var queryParameters = Parsers.ParseQueryString(queryString, Encoding.UTF8);

return position > 0
? new KeyValuePair<string, string?>(x.Substring(0, position), x.Substring(position + 1))
: new KeyValuePair<string, string?>(x, null);
}
);
foreach (var parameter in queryParameters) {
this.AddQueryParameter(parameter.Key, parameter.Value);
}
}

/// <summary>
Expand All @@ -84,12 +81,12 @@ public RestRequest(Uri resource, Method method = Method.Get)
/// Always send a multipart/form-data request - even when no Files are present.
/// </summary>
public bool AlwaysMultipartFormData { get; set; }

/// <summary>
/// Always send a file as request content without multipart/form-data request - even when the request contains only one file parameter
/// </summary>
public bool AlwaysSingleFileAsContent { get; set; }

/// <summary>
/// When set to true, parameter values in a multipart form data requests will be enclosed in
/// quotation marks. Default is false. Enable it if the remote endpoint requires parameters
Expand Down Expand Up @@ -232,7 +229,7 @@ public Func<HttpResponseMessage, RestRequest, RestResponse>? AdvancedResponseWri
_advancedResponseHandler = value;
}
}

/// <summary>
/// Request-level interceptors. Will be combined with client-level interceptors if set.
/// </summary>
Expand All @@ -255,4 +252,4 @@ public RestRequest RemoveParameter(Parameter parameter) {
}

internal RestRequest AddFile(FileParameter file) => this.With(x => x._files.Add(file));
}
}
52 changes: 44 additions & 8 deletions src/RestSharp/Request/UriExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,65 @@
namespace RestSharp;

static class UriExtensions {
#if NET6_0_OR_GREATER
internal static UriCreationOptions UriOptions = new() { DangerousDisablePathAndQueryCanonicalization = true };
#endif

public static Uri MergeBaseUrlAndResource(this Uri? baseUrl, string? resource) {
var assembled = resource;

#if NET6_0_OR_GREATER
if (assembled.IsNotEmpty() && assembled.StartsWith('/')) assembled = assembled[1..];
#else
if (assembled.IsNotEmpty() && assembled.StartsWith("/")) assembled = assembled.Substring(1);
#endif

if (baseUrl == null || baseUrl.AbsoluteUri.IsEmpty()) {
return assembled.IsNotEmpty()
? new Uri(assembled)
#if NET6_0_OR_GREATER
? new Uri(assembled, UriOptions)
#else
#pragma warning disable CS0618 // Type or member is obsolete
? new Uri(assembled, true)
#pragma warning restore CS0618 // Type or member is obsolete
#endif
: throw new ArgumentException("Both BaseUrl and Resource are empty", nameof(resource));
}

var usingBaseUri = baseUrl.AbsoluteUri.EndsWith("/") || assembled.IsEmpty() ? baseUrl : new Uri(baseUrl.AbsoluteUri + "/");
#if NET6_0_OR_GREATER
var usingBaseUri = baseUrl.AbsoluteUri.EndsWith('/') || assembled.IsEmpty() ? baseUrl : new Uri($"{baseUrl.AbsoluteUri}/", UriOptions);

var isResourceAbsolute = false;
// ReSharper disable once InvertIf
if (assembled != null) {
var resourceUri = new Uri(assembled, UriKind.RelativeOrAbsolute);
isResourceAbsolute = resourceUri.IsAbsoluteUri;
}

return assembled != null ? new Uri(usingBaseUri, assembled) : baseUrl;
return assembled != null ? new Uri(isResourceAbsolute ? assembled : $"{usingBaseUri.AbsoluteUri}{assembled}", UriOptions) : baseUrl;
#else
#pragma warning disable CS0618 // Type or member is obsolete
var usingBaseUri = baseUrl.AbsoluteUri.EndsWith("/") || assembled.IsEmpty() ? baseUrl : new Uri($"{baseUrl.AbsoluteUri}/", true);
return assembled != null ? new Uri(usingBaseUri, assembled, true) : baseUrl;
#pragma warning restore CS0618 // Type or member is obsolete
#endif
}

public static Uri AddQueryString(this Uri uri, string? query) {
if (query == null) return uri;

var absoluteUri = uri.AbsoluteUri;
var separator = absoluteUri.Contains('?') ? "&" : "?";

return new Uri($"{absoluteUri}{separator}{query}");
var absoluteUri = uri.AbsoluteUri;
var separator = absoluteUri.Contains('?') ? "&" : "?";

var result =
#if NET6_0_OR_GREATER
new Uri($"{absoluteUri}{separator}{query}", UriOptions);
#else
#pragma warning disable CS0618 // Type or member is obsolete
new Uri($"{absoluteUri}{separator}{query}", true);
#pragma warning restore CS0618 // Type or member is obsolete
#endif
return result;
}

public static UrlSegmentParamsValues GetUrlSegmentParamsValues(
Expand Down Expand Up @@ -72,4 +108,4 @@ params ParametersCollection[] parametersCollections
}
}

record UrlSegmentParamsValues(Uri Uri, string Resource);
record UrlSegmentParamsValues(Uri Uri, string Resource);
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ public async Task Should_keep_to_parameters_with_the_same_name() {
using var client = new RestClient(_server.Url!);
var request = new RestRequest(parameters);

var uri = client.BuildUri(request);

await client.GetAsync(request);

var query = new Uri(url).Query;
Expand Down
2 changes: 1 addition & 1 deletion test/RestSharp.Tests/ParametersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void AddUrlSegmentModifiesUrlSegmentWithInt() {
using var client = new RestClient(BaseUrl);
var actual = client.BuildUri(request).AbsolutePath;

expected.Should().BeEquivalentTo(actual);
actual.Should().Be(expected);
}

[Fact]
Expand Down
32 changes: 0 additions & 32 deletions test/RestSharp.Tests/RestClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,38 +32,6 @@ public async Task ConfigureHttp_will_set_proxy_to_null_with_no_exceptions_When_n
await client.ExecuteAsync(req);
}

[Fact]
public void BuildUri_should_build_with_passing_link_as_Uri() {
// arrange
var relative = new Uri("/foo/bar/baz", UriKind.Relative);
var absoluteUri = new Uri(new Uri(BaseUrl), relative);
var req = new RestRequest(absoluteUri);

// act
using var client = new RestClient();

var builtUri = client.BuildUri(req);

// assert
absoluteUri.Should().Be(builtUri);
}

[Fact]
public void BuildUri_should_build_with_passing_link_as_Uri_with_set_BaseUrl() {
// arrange
var baseUrl = new Uri(BaseUrl);
var relative = new Uri("/foo/bar/baz", UriKind.Relative);
var req = new RestRequest(relative);

// act
using var client = new RestClient(baseUrl);

var builtUri = client.BuildUri(req);

// assert
new Uri(baseUrl, relative).Should().Be(builtUri);
}

[Fact]
public void UseJson_leaves_only_json_serializer() {
// arrange
Expand Down
17 changes: 13 additions & 4 deletions test/RestSharp.Tests/RestRequestTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,26 @@ public void RestRequest_Request_Property() {

[Fact]
public void RestRequest_Test_Already_Encoded() {
var request = new RestRequest("/api/get?query=Id%3d198&another=notencoded");
const string resource = "/api/get?query=Id%3d198&another=notencoded&novalue=";
const string baseUrl = "https://example.com";

var request = new RestRequest(resource);
var parameters = request.Parameters.ToArray();

request.Resource.Should().Be("/api/get");
parameters.Length.Should().Be(2);
parameters.Length.Should().Be(3);

var expected = new[] {
new { Name = "query", Value = "Id%3d198", Type = ParameterType.QueryString, Encode = false },
new { Name = "another", Value = "notencoded", Type = ParameterType.QueryString, Encode = false }
new { Name = "another", Value = "notencoded", Type = ParameterType.QueryString, Encode = false },
new { Name = "novalue", Value = "", Type = ParameterType.QueryString, Encode = false }
};
parameters.Should().BeEquivalentTo(expected, options => options.ExcludingMissingMembers());
// parameters.Should().BeEquivalentTo(expected, options => options.ExcludingMissingMembers());

using var client = new RestClient(baseUrl);

var actual = client.BuildUri(request);
actual.AbsoluteUri.Should().Be($"{baseUrl}{resource}");
}

[Fact]
Expand Down
Loading
Loading