From 47235fb9f11656669e0d8f3d4edcea4e106c36dd Mon Sep 17 00:00:00 2001
From: Steven Lizano <130484280+sfc-gh-erojaslizano@users.noreply.github.com>
Date: Wed, 17 Apr 2024 13:58:01 -0600
Subject: [PATCH] [SNOW-1313929] Add https to endpoints without it (#914)
### Description
- Add https to endpoints without it
- Add unit test for SetCommonClientConfig
### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
---
.../UnitTests/SFS3ClientTest.cs | 31 +++++++++++++++++++
.../FileTransfer/StorageClient/SFS3Client.cs | 29 ++++++++---------
2 files changed, 46 insertions(+), 14 deletions(-)
diff --git a/Snowflake.Data.Tests/UnitTests/SFS3ClientTest.cs b/Snowflake.Data.Tests/UnitTests/SFS3ClientTest.cs
index 561819623..da3baf531 100644
--- a/Snowflake.Data.Tests/UnitTests/SFS3ClientTest.cs
+++ b/Snowflake.Data.Tests/UnitTests/SFS3ClientTest.cs
@@ -3,6 +3,7 @@
*/
using System;
+using Amazon.S3.Encryption;
namespace Snowflake.Data.Tests.UnitTests
{
@@ -219,6 +220,36 @@ public void TestUploadFile(string requestKey, ResultStatus expectedResultStatus)
AssertForUploadFileTests(expectedResultStatus);
}
+ [Test]
+ public void TestAppendHttpsToEndpoint()
+ {
+ // Arrange
+ var amazonS3Client = new AmazonS3Config();
+ var endpoint = "endpointWithNoHttps.com";
+ var expectedEndpoint = "https://endpointWithNoHttps.com";
+
+ // ACT
+ SFS3Client.SetCommonClientConfig(amazonS3Client, string.Empty, endpoint, 1, 0);
+
+ // Assert
+ Assert.That(amazonS3Client.ServiceURL, Is.EqualTo(expectedEndpoint));
+ }
+
+ [Test]
+ public void TestAppendHttpsToEndpointWithBrackets()
+ {
+ // Arrange
+ var amazonS3Client = new AmazonS3Config();
+ var endpoint = "[endpointWithNoHttps.com]";
+ var expectedEndpoint = "https://endpointWithNoHttps.com";
+
+ // ACT
+ SFS3Client.SetCommonClientConfig(amazonS3Client, string.Empty, endpoint, 1, 0);
+
+ // Assert
+ Assert.That(amazonS3Client.ServiceURL, Is.EqualTo(expectedEndpoint));
+ }
+
[Test]
[TestCase(MockS3Client.AwsStatusOk, ResultStatus.UPLOADED)]
[TestCase(SFS3Client.EXPIRED_TOKEN, ResultStatus.RENEW_TOKEN)]
diff --git a/Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs b/Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs
index e68fbbd3e..88b20c1d5 100644
--- a/Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs
+++ b/Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs
@@ -116,7 +116,7 @@ public SFS3Client(
stageInfo.endPoint,
maxRetry,
parallel);
-
+
// Get the AWS token value and create the S3 client
if (stageInfo.stageCredentials.TryGetValue(AWS_TOKEN, out string awsSessionToken))
{
@@ -164,7 +164,7 @@ public RemoteLocation ExtractBucketNameAndPath(string stageLocation)
{
bucketName = stageLocation.Substring(0, stageLocation.IndexOf('/'));
- s3path = stageLocation.Substring(stageLocation.IndexOf('/') + 1,
+ s3path = stageLocation.Substring(stageLocation.IndexOf('/') + 1,
stageLocation.Length - stageLocation.IndexOf('/') - 1);
if (s3path != null && !s3path.EndsWith("/"))
{
@@ -287,13 +287,13 @@ private FileHeader HandleFileHeaderResponse(ref SFFileMetadata fileMetadata, Get
}
///
- /// Set the client configuration common to both client with and without client-side
+ /// Set the client configuration common to both client with and without client-side
/// encryption.
///
/// The client config to update.
/// The region if any.
/// The endpoint if any.
- private static void SetCommonClientConfig(
+ internal static void SetCommonClientConfig(
AmazonS3Config clientConfig,
string region,
string endpoint,
@@ -309,23 +309,25 @@ private static void SetCommonClientConfig(
}
// If a specific endpoint is specified use this
- if ((null != endpoint) && (0 != endpoint.Length))
+ if (!string.IsNullOrEmpty(endpoint))
{
var start = endpoint.IndexOf('[');
var end = endpoint.IndexOf(']');
- if(start > -1 && end > -1 && end > start)
+ if (start > -1 && end > -1 && end > start)
{
endpoint = endpoint.Substring(start + 1, end - start - 1);
- if(!endpoint.Contains("https"))
- {
- endpoint = "https://" + endpoint;
- }
}
+
+ if (!endpoint.StartsWith("https://", StringComparison.OrdinalIgnoreCase))
+ {
+ endpoint = "https://" + endpoint;
+ }
+
clientConfig.ServiceURL = endpoint;
}
// The region information used to determine the endpoint for the service.
- // RegionEndpoint and ServiceURL are mutually exclusive properties.
+ // RegionEndpoint and ServiceURL are mutually exclusive properties.
// If both stageInfo.endPoint and stageInfo.region have a value, stageInfo.region takes
// precedence and ServiceUrl will be reset to null.
if ((null != region) && (0 != region.Length))
@@ -337,7 +339,6 @@ private static void SetCommonClientConfig(
// Unavailable for .net framework 4.6
//clientConfig.MaxConnectionsPerServer = parallel;
clientConfig.MaxErrorRetry = maxRetry;
-
}
///
@@ -410,7 +411,7 @@ private PutObjectRequest GetPutObjectRequest(ref AmazonS3Client client, SFFileMe
{
PutGetStageInfo stageInfo = fileMetadata.stageInfo;
RemoteLocation location = ExtractBucketNameAndPath(stageInfo.location);
-
+
// Create S3 PUT request
fileBytesStream.Position = 0;
PutObjectRequest putObjectRequest = new PutObjectRequest
@@ -585,4 +586,4 @@ private SFFileMetadata HandleDownloadFileErr(Exception ex, SFFileMetadata fileMe
return fileMetadata;
}
}
-}
\ No newline at end of file
+}