Skip to content

Commit

Permalink
Fix ping in SDK and add user agent headers (#3672)
Browse files Browse the repository at this point in the history
* ping should ping a static asset and add user agent headers

* fix: fmt

* Removed frontend headers check for FE1 servers

* Removed another test that relied on this pinging

---------

Co-authored-by: Dimitrie Stefanescu <[email protected]>
Co-authored-by: Jedd Morgan <[email protected]>
  • Loading branch information
3 people authored Jan 13, 2025
1 parent 72fefd5 commit 516caa6
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 52 deletions.
5 changes: 2 additions & 3 deletions Core/Core/Api/GraphQL/Models/ServerInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ public sealed class ServerInfo

/// <remarks>
/// This field is not returned from the GQL API,
/// it should be populated after construction from the response headers.
/// see <see cref="Speckle.Core.Credentials.AccountManager"/>
/// it was previously populated after construction from the response headers, but now FE1 is deprecated, so we should always assume FE2
/// </remarks>
public bool frontend2 { get; set; }
public bool frontend2 { get; set; } = true;

/// <remarks>
/// This field is not returned from the GQL API,
Expand Down
34 changes: 0 additions & 34 deletions Core/Core/Credentials/AccountManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ public static async Task<ServerInfo> GetServerInfo(Uri server, CancellationToken

ServerInfo serverInfo = response.Data.serverInfo;
serverInfo.url = server.ToString().TrimEnd('/');
serverInfo.frontend2 = await IsFrontend2Server(server).ConfigureAwait(false);

return response.Data.serverInfo;
}
Expand Down Expand Up @@ -198,7 +197,6 @@ internal static async Task<ActiveUserServerInfoResponse> GetUserServerInfo(

ServerInfo serverInfo = response.Data.serverInfo;
serverInfo.url = server.ToString().TrimEnd('/');
serverInfo.frontend2 = await IsFrontend2Server(server).ConfigureAwait(false);

return response.Data;
}
Expand Down Expand Up @@ -802,38 +800,6 @@ await response.Content.ReadAsStringAsync().ConfigureAwait(false)
}
}

/// <summary>
/// Sends a simple get request to the <paramref name="server"/>, and checks the response headers for a <c>"x-speckle-frontend-2"</c> <see cref="Boolean"/> value
/// </summary>
/// <param name="server">Server endpoint to get header</param>
/// <returns><see langword="true"/> if response contains FE2 header and the value was <see langword="true"/></returns>
/// <exception cref="SpeckleException">response contained FE2 header, but the value was <see langword="null"/>, empty, or not parseable to a <see cref="Boolean"/></exception>
/// <exception cref="HttpRequestException">Request to <paramref name="server"/> failed to send or response was not successful</exception>
private static async Task<bool> IsFrontend2Server(Uri server)
{
using var httpClient = Http.GetHttpProxyClient();

var response = await Http.HttpPing(server).ConfigureAwait(false);

var headers = response.Headers;
const string HEADER = "x-speckle-frontend-2";
if (!headers.TryGetValues(HEADER, out IEnumerable<string> values))
{
return false;
}

string? headerValue = values.FirstOrDefault();

if (!bool.TryParse(headerValue, out bool value))
{
throw new SpeckleException(
$"Headers contained {HEADER} header, but value {headerValue} could not be parsed to a bool"
);
}

return value;
}

private static string GenerateChallenge()
{
using RNGCryptoServiceProvider rng = new();
Expand Down
10 changes: 9 additions & 1 deletion Core/Core/Helpers/Http.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public static async Task<HttpResponseMessage> HttpPing(Uri uri)
try
{
using var httpClient = GetHttpProxyClient();
HttpResponseMessage response = await httpClient.GetAsync(uri).ConfigureAwait(false);
HttpResponseMessage response = await httpClient.GetAsync(GetPingUrl(uri)).ConfigureAwait(false);
response.EnsureSuccessStatusCode();
SpeckleLog.Logger.Information("Successfully pinged {uri}", uri);
return response;
Expand All @@ -155,6 +155,12 @@ public static async Task<HttpResponseMessage> HttpPing(Uri uri)
}
}

public static Uri GetPingUrl(Uri serverUrl)
{
var server = serverUrl.GetLeftPart(UriPartial.Authority);
return new Uri(new(server), "/favicon.ico");
}

public static HttpClient GetHttpProxyClient(SpeckleHttpClientHandler? speckleHttpClientHandler = null)
{
IWebProxy proxy = WebRequest.GetSystemWebProxy();
Expand All @@ -166,6 +172,8 @@ public static HttpClient GetHttpProxyClient(SpeckleHttpClientHandler? speckleHtt
{
Timeout = Timeout.InfiniteTimeSpan //timeout is configured on the SpeckleHttpClientHandler through policy
};
client.DefaultRequestHeaders.UserAgent.Clear();
client.DefaultRequestHeaders.UserAgent.Add(new("SpeckleSDK", "2.0.0"));
return client;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,6 @@ public async Task IsFrontEnd2True()
Assert.That(result.frontend2, Is.True);
}

/// <remarks>
/// We get ServerInfo from "http://localhost:3000/graphql",
/// Then we mutate the `frontend2` property of ServerInfo by trying to fetch header from "http://localhost:3000/",
/// This is not doable in local server because there is no end-point on this to ping.
/// This is a bad sign for mutation.
/// </remarks>
[Test]
public void GetServerInfo_ExpectFail_CantPing()
{
Uri serverUrl = new(acc.serverInfo.url);

Assert.ThrowsAsync<HttpRequestException>(async () => await AccountManager.GetServerInfo(serverUrl));
}

[Test]
public void GetServerInfo_ExpectFail_NoServer()
{
Expand Down
11 changes: 11 additions & 0 deletions Core/Tests/Speckle.Core.Tests.Unit/Helpers/Path.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ namespace Speckle.Core.Tests.Unit.Helpers;
[TestOf(nameof(SpecklePathProvider))]
public class SpecklePathTests
{
[Test]
[TestCase("https://app.speckle.systems", "https://app.speckle.systems/favicon.ico")]
[TestCase("https://app.speckle.systems/", "https://app.speckle.systems/favicon.ico")]
[TestCase("https://app.speckle.systems/auth/login", "https://app.speckle.systems/favicon.ico")]
[TestCase("https://latest.speckle.systems", "https://latest.speckle.systems/favicon.ico")]
public void TestGetPingUrl(string given, string expected)
{
var x = Http.GetPingUrl(new Uri(given));
Assert.That(x, Is.EqualTo(new Uri(expected)));
}

[Test]
public void TestUserApplicationDataPath()
{
Expand Down

0 comments on commit 516caa6

Please sign in to comment.