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

Fixes format exception thrown when user agent header value is incorrect #283

Open
wants to merge 6 commits into
base: master
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
44 changes: 36 additions & 8 deletions CloudinaryDotNet.Tests/Asset/UrlBuilderTest.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using System;
using System.Collections.Generic;
using System.Net.Http;
using System.Linq;
using System.Net.Http.Headers;
using System.Text.RegularExpressions;
using CloudinaryDotNet.Actions;
using NUnit.Framework;
Expand Down Expand Up @@ -475,23 +475,51 @@ public void TestExcludeEmptyTransformation()
Assert.AreEqual(TestConstants.DefaultImageUpPath + "c_fill,x_100,y_100/test", uri);
}

[Test]
public void TestAgentPlatformHeaders()
private HttpRequestMessage CreateRequest(string userPlatformProduct, string userPlatformVersion = null)
{
var request = new HttpRequestMessage { RequestUri = new Uri("http://dummy.com") };
m_api.UserPlatform = "Test/1.0";
m_api.UserPlatform = new ProductHeaderValue(userPlatformProduct, userPlatformVersion);

m_api.PrepareRequestBody(
request,
HttpMethod.GET,
new SortedDictionary<string, object>(),
new FileDescription(""));
return request;
}

[Test]
public void TestAgentPlatformHeaders()
{
var httpRequestMessage = CreateRequest("UserPlatform", "2.3");

//Can't test the result, so we just verify the UserAgent parameter is sent to the server
StringAssert.AreEqualIgnoringCase($"{m_api.UserPlatform} {ApiShared.USER_AGENT}",
request.Headers.UserAgent.ToString());
StringAssert.IsMatch(@"Test\/1\.0 CloudinaryDotNet\/(\d+)\.(\d+)\.(\d+) \(.*\)",
request.Headers.UserAgent.ToString());
StringAssert.IsMatch(@"CloudinaryDotNet\/(\d+)\.(\d+)\.(\d+) \(.*\) UserPlatform/2\.3",
httpRequestMessage.Headers.UserAgent.ToString());
}

[Test]
[TestCase("Mono 5.11.0 ((HEAD/768f1b247c6)")]
public void TestMalformedFrameworkVersion(string dotnetVersion)
{
var previousFramework = m_api.DotnetVersion;
try
{
m_api.DotnetVersion = dotnetVersion;
Assert.DoesNotThrow(() => CreateRequest("p"));
}
finally
{
m_api.DotnetVersion = previousFramework;
}
}

[Test]
[TestCase("UserPlatform", null)]
[TestCase("UserPlatform", "1.2")]
public void TestUserPlatformCombinations(string userPlatformProduct, string userPlatformVersion)
{
Assert.DoesNotThrow(() => CreateRequest(userPlatformProduct, userPlatformVersion));
}

[Test]
Expand Down
14 changes: 9 additions & 5 deletions CloudinaryDotNet/ApiShared.Internal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -455,12 +455,16 @@ private void PrePrepareRequestBody(

// Add platform information to the USER_AGENT header
// This is intended for platform information and not individual applications!
var userPlatform = string.IsNullOrEmpty(UserPlatform)
? USER_AGENT
: string.Format(CultureInfo.InvariantCulture, "{0} {1}", UserPlatform, USER_AGENT);
request.Headers.Add("User-Agent", userPlatform);
var userAgentHeader = request.Headers.UserAgent;
userAgentHeader.Add(new ProductInfoHeaderValue("CloudinaryDotNet", CloudinaryVersion.Full));
userAgentHeader.Add(new ProductInfoHeaderValue($"({DotnetVersion})"));

byte[] authBytes = Encoding.ASCII.GetBytes(GetApiCredentials());
if (UserPlatform != null)
{
userAgentHeader.Add(new ProductInfoHeaderValue(UserPlatform));
}

var authBytes = Encoding.ASCII.GetBytes(GetApiCredentials());
request.Headers.Add("Authorization", string.Format(CultureInfo.InvariantCulture, "Basic {0}", Convert.ToBase64String(authBytes)));

if (extraHeaders != null)
Expand Down
50 changes: 34 additions & 16 deletions CloudinaryDotNet/ApiShared.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Runtime.Serialization;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
using CloudinaryDotNet.Actions;
Expand Down Expand Up @@ -68,11 +70,6 @@ public partial class ApiShared : ISignProvider
/// </summary>
public const string HTTP_BOUNDARY = "notrandomsequencetouseasboundary";

/// <summary>
/// User agent for cloudinary API requests.
/// </summary>
public static string USER_AGENT = BuildUserAgent();

/// <summary>
/// Whether to use a sub domain.
/// </summary>
Expand Down Expand Up @@ -111,7 +108,7 @@ public partial class ApiShared : ISignProvider
/// <summary>
/// User platform information.
/// </summary>
public string UserPlatform;
public ProductHeaderValue UserPlatform;

/// <summary>
/// Timeout for the API requests, milliseconds.
Expand Down Expand Up @@ -150,6 +147,8 @@ public partial class ApiShared : ISignProvider
private readonly Func<string, HttpRequestMessage> requestBuilder =
(url) => new HttpRequestMessage { RequestUri = new Uri(url) };

private string dotnetVersion;

/// <summary>
/// Initializes a new instance of the <see cref="ApiShared"/> class.
/// Default parameterless constructor.
Expand Down Expand Up @@ -382,6 +381,24 @@ public Url ApiUrlVideoUpV
}
}

/// <summary>
/// Gets or sets the .NET version compatible with Http headers comment specification.
/// </summary>
internal string DotnetVersion
{
get
{
if (dotnetVersion == null)
{
DotnetVersion = System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription;
}

return dotnetVersion;
}

set => dotnetVersion = GetUserAgentCommentFriendlyValue(value);
}

/// <summary>
/// Gets cloudinary parameter from enumeration.
/// </summary>
Expand Down Expand Up @@ -628,12 +645,12 @@ public string SignParameters(IDictionary<string, object> parameters)
StringBuilder signBase = new StringBuilder(string.Join("&", parameters.
Where(pair => pair.Value != null && !excludedSignatureKeys.Any(s => pair.Key.Equals(s, StringComparison.Ordinal)))
.Select(pair =>
{
var value = pair.Value is IEnumerable<string>
? string.Join(",", ((IEnumerable<string>)pair.Value).ToArray())
: pair.Value.ToString();
return string.Format(CultureInfo.InvariantCulture, "{0}={1}", pair.Key, value);
})
{
var value = pair.Value is IEnumerable<string>
? string.Join(",", ((IEnumerable<string>)pair.Value).ToArray())
: pair.Value.ToString();
return string.Format(CultureInfo.InvariantCulture, "{0}={1}", pair.Key, value);
})
.ToArray()));

signBase.Append(Account.ApiSecret);
Expand Down Expand Up @@ -799,9 +816,10 @@ public string BuildUploadFormShared(string field, string resourceType, SortedDic
return builder.ToString();
}

private static string BuildUserAgent()
{
return $"CloudinaryDotNet/{CloudinaryVersion.Full} ({RuntimeInformation.FrameworkDescription})";
}
// Nonmatching braces are not accepted, so we just remove them all.
// See https://github.com/aspnet/HttpAbstractions/blob/master/src/Microsoft.Net.Http.Headers/HttpRuleParser.cs#L250
// for details.
private static string GetUserAgentCommentFriendlyValue(string value)
=> Regex.Replace(value, "\\(|\\)", string.Empty);
}
}