Merge pull request #66 from bbockelm/etag_case_insensitive

Use case-insensitive search for ETag header
PelicanPlatform · Dec 17, 2024 · 91b7196 · 91b7196
2 parents 21b3e77 + 74da355
commit 91b7196
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 23 deletions.
diff --git a/src/HTTPCommands.cc b/src/HTTPCommands.cc
@@ -705,6 +705,7 @@ bool HTTPRequest::Fail(const std::string &ecode, const std::string &emsg) {
 void HTTPRequest::Notify() {
 	std::lock_guard<std::mutex> lk(m_mtx);
 	m_result_ready = true;
+	modifyResponse(m_result);
 	m_cv.notify_one();
 }
 

diff --git a/src/HTTPCommands.hh b/src/HTTPCommands.hh
@@ -162,6 +162,10 @@ class HTTPRequest {
 	// call to send more data.
 	bool Timeout() const { return m_timeout; }
 
+	// Function that can be overridden by test cases, allowing modification
+	// of the server response
+	virtual void modifyResponse(std::string &) {}
+
 	typedef std::map<std::string, std::string> AttributeValueMap;
 	AttributeValueMap query_parameters;
 	AttributeValueMap headers;

diff --git a/src/S3Commands.cc b/src/S3Commands.cc
@@ -535,6 +535,28 @@ bool AmazonS3SendMultipartPart::SendRequest(const std::string_view payload,
 	return SendS3Request(payload, payloadSize, final, true);
 }
 
+bool AmazonS3SendMultipartPart::GetEtag(std::string &result) {
+	if (!m_etag.empty()) {
+		result = m_etag;
+		return true;
+	}
+	auto resultString = getResultString();
+	static const std::string etag = "etag: \"";
+	auto iter = std::search(
+		resultString.begin(), resultString.end(), etag.begin(), etag.end(),
+		[](char a, char b) { return std::tolower(a) == std::tolower(b); });
+	if (iter == resultString.end()) {
+		return false;
+	}
+	std::size_t startPos = std::distance(resultString.begin(), iter);
+	std::size_t endPos = resultString.find("\"", startPos + 7);
+	if (endPos == std::string::npos) {
+		return false;
+	}
+	m_etag = result = resultString.substr(startPos + 7, endPos - startPos - 7);
+	return true;
+}
+
 // ---------------------------------------------------------------------------
 
 AmazonS3Download::~AmazonS3Download() {}

diff --git a/src/S3Commands.hh b/src/S3Commands.hh
@@ -217,7 +217,7 @@ class AmazonS3CompleteMultipartUpload : public AmazonRequest {
   protected:
 };
 
-class AmazonS3SendMultipartPart final : public AmazonRequest {
+class AmazonS3SendMultipartPart : public AmazonRequest {
 	using AmazonRequest::SendRequest;
 
   public:
@@ -247,7 +247,11 @@ class AmazonS3SendMultipartPart final : public AmazonRequest {
 					 const std::string &partNumber, const std::string &uploadId,
 					 size_t payloadSize, bool final);
 
-  protected:
+	// Retrieve the ETag header from the returned headers;
+	bool GetEtag(std::string &result);
+
+  private:
+	std::string m_etag;
 };
 
 class AmazonS3Download : public AmazonRequest {

diff --git a/src/S3File.cc b/src/S3File.cc
@@ -289,11 +289,14 @@ ssize_t S3File::SendPartStreaming() {
 		return -EIO;
 	} else {
 		m_log.Log(LogMask::Debug, "SendPart", "upload.SendRequest() succeeded");
-		std::string resultString = upload_part_request.getResultString();
-		std::size_t startPos = resultString.find("ETag:");
-		std::size_t endPos = resultString.find("\"", startPos + 7);
-		eTags.push_back(
-			resultString.substr(startPos + 7, endPos - startPos - 7));
+		std::string etag;
+		if (!upload_part_request.GetEtag(etag)) {
+			m_log.Log(
+				LogMask::Debug, "SendPart",
+				"upload.SendRequest() response missing an eTag in response");
+			return -EIO;
+		}
+		eTags.push_back(etag);
 		partNumber++;
 		m_streaming_buffer.clear();
 	}
@@ -333,26 +336,14 @@ ssize_t S3File::ContinueSendPart(const void *buffer, size_t size) {
 	if (is_final) {
 		m_part_written = 0;
 		m_part_size = 0;
-		auto &resultString = m_write_op->getResultString();
-		std::size_t startPos = resultString.find("ETag:");
-		if (startPos == std::string::npos) {
-			m_log.Emsg("Write", "Result from S3 does not include ETag:",
-					   resultString.c_str());
-			m_write_op.reset();
-			m_write_offset = -1;
-			return -EIO;
-		}
-		std::size_t endPos = resultString.find('"', startPos + 7);
-		if (startPos == std::string::npos) {
-			m_log.Emsg("Write",
-					   "Result from S3 does not include ETag end-character:",
-					   resultString.c_str());
+		std::string etag;
+		if (!m_write_op->GetEtag(etag)) {
+			m_log.Emsg("Write", "Result from S3 does not include ETag");
 			m_write_op.reset();
 			m_write_offset = -1;
 			return -EIO;
 		}
-		eTags.push_back(
-			resultString.substr(startPos + 7, endPos - startPos - 7));
+		eTags.push_back(etag);
 		m_write_op.reset();
 		partNumber++;
 	}

diff --git a/test/s3_unit_tests.cc b/test/s3_unit_tests.cc
@@ -112,6 +112,11 @@ s3.end
 	}
 
   public:
+	std::unique_ptr<S3FileSystem> GetFS() {
+		return std::unique_ptr<S3FileSystem>(
+			new S3FileSystem(&m_log, m_configfn.c_str(), nullptr));
+	}
+
 	void WritePattern(const std::string &name, const off_t writeSize,
 					  const unsigned char chunkByte, const size_t chunkSize,
 					  bool known_size) {
@@ -260,6 +265,8 @@ s3.end
 		rv = fh->Close();
 		ASSERT_EQ(rv, 0);
 	}
+
+	XrdSysLogger m_log;
 };
 
 // Upload a single byte into S3
@@ -576,6 +583,50 @@ TEST_F(FileSystemS3Fixture, StressGet) {
 	}
 }
 
+class AmazonS3SendMultipartPartLowercase : public AmazonS3SendMultipartPart {
+  protected:
+	virtual void modifyResponse(std::string &resp) override {
+		std::transform(resp.begin(), resp.end(), resp.begin(),
+					   [](unsigned char c) { return std::tolower(c); });
+	}
+};
+
+TEST_F(FileSystemS3Fixture, Etag) {
+	// Determine the S3 info.
+	auto oss = GetFS();
+	std::string exposedPath, object;
+	std::string path = "/test/etag_casesensitive_test";
+	ASSERT_EQ(oss->parsePath(path.c_str(), exposedPath, object), 0);
+
+	auto ai = oss->getS3AccessInfo(exposedPath, object);
+	ASSERT_NE(ai.get(), nullptr);
+	ASSERT_NE(ai->getS3BucketName(), "");
+	ASSERT_NE(object, "");
+
+	// Start an upload.
+	XrdSysLogger log;
+	XrdSysError err(&log, "test");
+	AmazonS3CreateMultipartUpload startUpload(*ai, object, err);
+	ASSERT_TRUE(startUpload.SendRequest());
+	std::string uploadId, errMsg;
+	startUpload.Results(uploadId, errMsg);
+
+	// Upload an etag.
+	AmazonS3SendMultipartPart upload_part_request(*ai, object, err);
+	std::string streaming_buffer = "aaaa";
+	ASSERT_TRUE(upload_part_request.SendRequest(streaming_buffer,
+												std::to_string(1), uploadId,
+												streaming_buffer.size(), true));
+	std::string etag;
+	ASSERT_TRUE(upload_part_request.GetEtag(etag));
+	std::vector<std::string> eTags;
+	eTags.push_back(etag);
+
+	// Finalize the object
+	AmazonS3CompleteMultipartUpload complete_upload_request(*ai, object, err);
+	ASSERT_TRUE(complete_upload_request.SendRequest(eTags, 2, uploadId));
+}
+
 int main(int argc, char **argv) {
 	::testing::InitGoogleTest(&argc, argv);
 	return RUN_ALL_TESTS();