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

fix/redirect with no location #498

Open
wants to merge 2 commits into
base: main
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Fixed a bug where the client would fail on 301/302 responses with no location headers. [#272](https://github.com/microsoft/kiota-dotnet/issues/272)

## [1.16.0] - 2024-12-13

### Added
Expand Down
2 changes: 1 addition & 1 deletion src/http/httpClient/HttpClientRequestAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to use for cancelling the request.</param>
/// <returns>The deserialized primitive response model.</returns>
#if NET5_0_OR_GREATER
public async Task<ModelType?> SendPrimitiveAsync<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] ModelType>(RequestInformation requestInfo, Dictionary<string, ParsableFactory<IParsable>>? errorMapping = default, CancellationToken cancellationToken = default)

Check warning on line 228 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 32 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 228 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 32 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
#else
public async Task<ModelType?> SendPrimitiveAsync<ModelType>(RequestInformation requestInfo, Dictionary<string, ParsableFactory<IParsable>>? errorMapping = default, CancellationToken cancellationToken = default)
#endif
Expand Down Expand Up @@ -424,7 +424,7 @@
private async Task ThrowIfFailedResponseAsync(HttpResponseMessage response, Dictionary<string, ParsableFactory<IParsable>>? errorMapping, Activity? activityForAttributes, CancellationToken cancellationToken)
{
using var span = activitySource?.StartActivity(nameof(ThrowIfFailedResponseAsync));
if(response.IsSuccessStatusCode) return;
if(response.IsSuccessStatusCode || response.StatusCode is HttpStatusCode.Moved or HttpStatusCode.MovedPermanently or HttpStatusCode.Found or HttpStatusCode.Redirect) return;

activityForAttributes?.SetStatus(ActivityStatusCode.Error, "received_error_response");

Expand Down Expand Up @@ -487,13 +487,13 @@
if(contentStream == Stream.Null || (contentStream.CanSeek && contentStream.Length == 0))
return null;// ensure a useful stream is passed to the factory
#pragma warning disable CS0618 // Type or member is obsolete
//TODO remove with v2

Check warning on line 490 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)

Check warning on line 490 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)

Check warning on line 490 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var rootNode = pNodeFactory is IAsyncParseNodeFactory asyncParseNodeFactory ? await asyncParseNodeFactory.GetRootParseNodeAsync(responseContentType!, contentStream, cancellationToken).ConfigureAwait(false) : pNodeFactory.GetRootParseNode(responseContentType!, contentStream);
#pragma warning restore CS0618 // Type or member is obsolete
return rootNode;
}
private const string ClaimsKey = "claims";
private const string BearerAuthenticationScheme = "Bearer";

Check warning on line 496 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field 'BearerAuthenticationScheme'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 496 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field 'BearerAuthenticationScheme'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 496 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field 'BearerAuthenticationScheme'. (https://rules.sonarsource.com/csharp/RSPEC-1144)
private async Task<HttpResponseMessage> GetHttpResponseMessageAsync(RequestInformation requestInfo, CancellationToken cancellationToken, Activity? activityForAttributes, string? claims = default, bool isStreamResponse = false)
{
using var span = activitySource?.StartActivity(nameof(GetHttpResponseMessageAsync));
Expand Down Expand Up @@ -575,7 +575,7 @@
else throw new InvalidOperationException($"Could not convert the request information to a {typeof(T).Name}");
}

private HttpRequestMessage GetRequestMessageFromRequestInformation(RequestInformation requestInfo, Activity? activityForAttributes)

Check warning on line 578 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 25 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 578 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 25 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
using var span = activitySource?.StartActivity(nameof(GetRequestMessageFromRequestInformation));
SetBaseUrlForRequestInformation(requestInfo);// this method can also be called from a different context so ensure the baseUrl is added.
Expand Down
41 changes: 33 additions & 8 deletions tests/http/httpClient/RequestAdapterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ namespace Microsoft.Kiota.Http.HttpClientLibrary.Tests
public class HttpClientRequestAdapterTests
{
private readonly IAuthenticationProvider _authenticationProvider;
private readonly HttpClientRequestAdapter requestAdapter;
private readonly HttpClientRequestAdapter _requestAdapter;

public HttpClientRequestAdapterTests()
{
_authenticationProvider = new Mock<IAuthenticationProvider>().Object;
requestAdapter = new HttpClientRequestAdapter(new AnonymousAuthenticationProvider());
_requestAdapter = new HttpClientRequestAdapter(new AnonymousAuthenticationProvider());
}

[Fact]
Expand Down Expand Up @@ -82,15 +82,15 @@ public void EnablesBackingStore()
public async Task GetRequestMessageFromRequestInformationWithBaseUrlTemplate()
{
// Arrange
requestAdapter.BaseUrl = "http://localhost";
_requestAdapter.BaseUrl = "http://localhost";
var requestInfo = new RequestInformation
{
HttpMethod = Method.GET,
UrlTemplate = "{+baseurl}/me"
};

// Act
var requestMessage = await requestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInfo);
var requestMessage = await _requestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInfo);

// Assert
Assert.NotNull(requestMessage);
Expand All @@ -113,10 +113,10 @@ public async Task GetRequestMessageFromRequestInformationUsesBaseUrlFromAdapter(

};
// Change the baseUrl of the adapter
requestAdapter.BaseUrl = "http://localhost";
_requestAdapter.BaseUrl = "http://localhost";

// Act
var requestMessage = await requestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInfo);
var requestMessage = await _requestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInfo);

// Assert
Assert.NotNull(requestMessage);
Expand All @@ -140,7 +140,7 @@ public async Task GetRequestMessageFromRequestInformationSetsQueryParametersCorr
requestInfo.QueryParameters.Add(queryParam, queryParamObject!);

// Act
var requestMessage = await requestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInfo);
var requestMessage = await _requestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInfo);

// Assert
Assert.NotNull(requestMessage);
Expand All @@ -162,7 +162,7 @@ public async Task GetRequestMessageFromRequestInformationSetsContentHeaders()
requestInfo.SetStreamContent(new MemoryStream(Encoding.UTF8.GetBytes("contents")), "application/octet-stream");

// Act
var requestMessage = await requestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInfo);
var requestMessage = await _requestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInfo);

// Assert
Assert.NotNull(requestMessage);
Expand Down Expand Up @@ -207,6 +207,31 @@ public async Task SendMethodDoesNotThrowWithoutUrlTemplate()
Assert.Equal(4, response.Length);
}

[InlineData(HttpStatusCode.Redirect)]
[InlineData(HttpStatusCode.MovedPermanently)]
[Theory]
public async Task SendMethodDoesNotThrowOn3XXWithNoLocationAsync(HttpStatusCode httpStatusCode)
{
var mockHandler = new Mock<HttpMessageHandler>();
var client = new HttpClient(mockHandler.Object);
mockHandler.Protected()
.Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
.ReturnsAsync(new HttpResponseMessage
{
StatusCode = httpStatusCode
});
var adapter = new HttpClientRequestAdapter(_authenticationProvider, httpClient: client);
var requestInfo = new RequestInformation
{
HttpMethod = Method.GET,
URI = new Uri("https://example.com")
};

var response = await adapter.SendAsync(requestInfo, MockEntity.Factory);

Assert.Null(response);
}

[InlineData(HttpStatusCode.OK)]
[InlineData(HttpStatusCode.Created)]
[InlineData(HttpStatusCode.Accepted)]
Expand Down
Loading