Merge pull request #35 from bbockelm/s3_ls

Implement directory listing

.github/workflows/linter.yml

@@ -9,7 +9,7 @@ name: Lint
 
 # If the linter fails, the PR can still be completed, but none of the linter changes will be made.
 
-on:         
+on:
   push:
     branches:
       - main
@@ -31,7 +31,7 @@ jobs:
       - name: Check out repository (push)
         if: ${{ github.event_name == 'push' }}
         uses: actions/checkout@v3
-      
+
       - name: Check out repository (pull_request_target)
         if: ${{ github.event_name == 'pull_request_target' }}
         uses: actions/checkout@v3
@@ -57,4 +57,3 @@ jobs:
         uses: reviewdog/action-suggester@v1
         with:
           tool_name: lint
-

.github/workflows/test.yml

@@ -2,10 +2,10 @@ name: Test
 
 on:
   pull_request:
-    branches: 
+    branches:
       - main
   push:
-    branches: 
+    branches:
       - main
 
 env:
@@ -29,7 +29,7 @@ jobs:
     - name: install deps
       run: |
         sudo apt update && sudo apt-get install -y cmake libcurl4-openssl-dev libcurl4 pkg-config libssl-dev libxrootd-dev libxrootd-server-dev libgtest-dev
-      
+
     - name: Create Build Environment
       # Some projects don't allow in-source building, so create a separate build directory
       # We'll use this as our working directory for all subsequent commands
@@ -40,8 +40,8 @@ jobs:
       # access regardless of the host operating system
       shell: bash
       working-directory: ${{runner.workspace}}/build
-      # Note the current convention is to use the -S and -B options here to specify source 
-      # and build directories, but this is only available with CMake 3.13 and higher.  
+      # Note the current convention is to use the -S and -B options here to specify source
+      # and build directories, but this is only available with CMake 3.13 and higher.
       # The CMake binaries on the Github Actions machines are (as of this writing) 3.12
       run: cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DXROOTD_PLUGINS_BUILD_UNITTESTS=yes -DXROOTD_PLUGINS_EXTERNAL_GTEST=${{ matrix.external-gtest }}
 
@@ -54,6 +54,6 @@ jobs:
     - name: Test
       working-directory: ${{runner.workspace}}/build
       shell: bash
-      # Execute tests defined by the CMake configuration.  
+      # Execute tests defined by the CMake configuration.
       # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
       run: ctest -C $BUILD_TYPE --verbose

.gitmodules

@@ -1,3 +1,7 @@
 [submodule "vendor/gtest"]
 	path = vendor/gtest
 	url = https://github.com/google/googletest.git
+
+[submodule "vendor/tinyxml2"]
+	path = vendor/tinyxml2
+	url = https://github.com/leethomason/tinyxml2.git

.pre-commit-config.yaml

@@ -0,0 +1,12 @@
+repos:
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v3.2.0
+    hooks:
+    -   id: trailing-whitespace
+    -   id: end-of-file-fixer
+    -   id: check-yaml
+    -   id: check-added-large-files
+-   repo: https://github.com/pre-commit/mirrors-clang-format
+    rev: v18.1.6
+    hooks:
+    -   id: clang-format

CMakeLists.txt

@@ -39,27 +39,34 @@ endmacro(use_cxx17)
 use_cxx17()
 
 if( CMAKE_COMPILER_IS_GNUCXX )
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror" ) 
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror" )
 endif()
 
 if(NOT APPLE)
   SET( CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-undefined")
   SET( CMAKE_MODULE_LINKER_FLAGS "-Wl,--no-undefined")
 endif()
 
+if( NOT XROOTD_EXTERNAL_TINYXML2 )
+	set(CMAKE_POSITION_INDEPENDENT_CODE ON)
+	add_subdirectory(vendor/tinyxml2)
+else()
+	find_package(tinyxml2::tinyxml2)
+endif()
+
 include_directories(${XROOTD_INCLUDES} ${CURL_INCLUDE_DIRS} ${LIBCRYPTO_INCLUDE_DIRS})
 
-add_library(XrdS3 SHARED src/S3File.cc src/S3AccessInfo.cc src/S3FileSystem.cc src/AWSv4-impl.cc src/S3Commands.cc src/HTTPCommands.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc)
+add_library(XrdS3 SHARED src/S3File.cc src/S3Directory.cc src/S3AccessInfo.cc src/S3FileSystem.cc src/AWSv4-impl.cc src/S3Commands.cc src/HTTPCommands.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc)
 add_library(XrdHTTPServer SHARED src/HTTPFile.cc src/HTTPFileSystem.cc src/HTTPCommands.cc src/stl_string_utils.cc src/shortfile.cc src/logging.cc)
 
-target_link_libraries(XrdS3 -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES})
+target_link_libraries(XrdS3 -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES} tinyxml2::tinyxml2)
 target_link_libraries(XrdHTTPServer -ldl ${XROOTD_UTILS_LIB} ${XROOTD_SERVER_LIB} ${CURL_LIBRARIES} ${LIBCRYPTO_LIBRARIES})
 
 # The CMake documentation strongly advises against using these macros; instead, the pkg_check_modules
 # is supposed to fill out the full path to ${LIBCRYPTO_LIBRARIES}.  As of cmake 3.26.1, this does not
 # appear to be the case on Mac OS X.  Remove once no longer necessary!
-target_link_directories(XrdS3 PRIVATE ${LIBCRYPTO_LIBRARY_DIRS})
-target_link_directories(XrdHTTPServer PRIVATE ${LIBCRYPTO_LIBRARY_DIRS})
+target_link_directories(XrdS3 PUBLIC ${LIBCRYPTO_LIBRARY_DIRS})
+target_link_directories(XrdHTTPServer PUBLIC ${LIBCRYPTO_LIBRARY_DIRS})
 
 if(NOT APPLE)
   set_target_properties(XrdS3 PROPERTIES OUTPUT_NAME "XrdS3-${XROOTD_PLUGIN_VERSION}" SUFFIX ".so" LINK_FLAGS "-Wl,--version-script=${CMAKE_SOURCE_DIR}/configs/export-lib-symbols")
@@ -77,16 +84,17 @@ install(
 )
 
 if( XROOTD_PLUGINS_BUILD_UNITTESTS )
-	if( NOT XROOTD_PLUGINS_EXTERNAL_GTEST )
-include(ExternalProject)
-ExternalProject_Add(gtest
-    PREFIX external/gtest
-    URL ${CMAKE_CURRENT_SOURCE_DIR}/vendor/gtest
-    INSTALL_COMMAND :
-)
-endif()
-enable_testing()
-add_subdirectory(test)
+  if( NOT XROOTD_PLUGINS_EXTERNAL_GTEST )
+    include(ExternalProject)
+    ExternalProject_Add(gtest
+      PREFIX external/gtest
+      URL ${CMAKE_CURRENT_SOURCE_DIR}/vendor/gtest
+      BUILD_BYPRODUCTS external/gtest/src/gtest-build/lib/libgtest.a
+      INSTALL_COMMAND :
+    )
+  endif()
+  enable_testing()
+  add_subdirectory(test)
 endif()
 
 #install(

README.md

@@ -161,4 +161,4 @@ In a separate terminal, run
 
 ```
 curl -v http://localhost:1094/<path name>/<object name>
-```
+```

rpm/xrootd-s3-http.spec

@@ -52,4 +52,3 @@ make install DESTDIR=$RPM_BUILD_ROOT
 
 * Tue Dec 06 2022 Brian Bockelman <[email protected]> - 0.0.1-1
 - Initial, "Hello world" version of the S3 filesystem plugin
-

src/AWSv4-impl.cc

@@ -201,7 +201,9 @@ std::string canonicalizeQueryString(
 	}
 
 	// We'll always have a superflous trailing ampersand.
-	canonicalQueryString.erase(canonicalQueryString.end() - 1);
+	if (!canonicalQueryString.empty()) {
+		canonicalQueryString.erase(canonicalQueryString.end() - 1);
+	}
 	return canonicalQueryString;
 }
 

src/HTTPCommands.cc

@@ -67,8 +67,6 @@ HTTPRequest::~HTTPRequest() {}
 		if (rv##B != CURLE_OK) {                                               \
 			this->errorCode = "E_CURL_LIB";                                    \
 			this->errorMessage = "curl_easy_setopt( " #B " ) failed.";         \
-			/* dprintf( D_ALWAYS, "curl_easy_setopt( %s ) failed (%d): '%s',   \
-			   failing.\n", #B, rv##B, curl_easy_strerror( rv##B ) ); */       \
 			return false;                                                      \
 		}                                                                      \
 	}
@@ -168,6 +166,8 @@ bool HTTPRequest::sendPreparedRequest(const std::string &protocol,
 									  const std::string &uri,
 									  const std::string &payload) {
 
+	m_log.Log(XrdHTTPServer::Debug, "SendRequest", "Sending HTTP request",
+			  uri.c_str());
 	CURLcode rv = curl_global_init(CURL_GLOBAL_ALL);
 	if (rv != 0) {
 		this->errorCode = "E_CURL_LIB";
@@ -317,20 +317,6 @@ bool HTTPRequest::sendPreparedRequest(const std::string &protocol,
 		CAFile = x509_ca_file;
 	}
 
-	if (CAPath.empty()) {
-		char *soap_ssl_ca_dir = getenv("GAHP_SSL_CADIR");
-		if (soap_ssl_ca_dir != NULL) {
-			CAPath = soap_ssl_ca_dir;
-		}
-	}
-
-	if (CAFile.empty()) {
-		char *soap_ssl_ca_file = getenv("GAHP_SSL_CAFILE");
-		if (soap_ssl_ca_file != NULL) {
-			CAFile = soap_ssl_ca_file;
-		}
-	}
-
 	if (!CAPath.empty()) {
 		SET_CURL_SECURITY_OPTION(curl.get(), CURLOPT_CAPATH, CAPath.c_str());
 	}

src/HTTPCommands.hh

@@ -47,6 +47,8 @@ class HTTPRequest {
 	virtual bool SendHTTPRequest(const std::string &payload);
 
 	unsigned long getResponseCode() const { return responseCode; }
+	const std::string &getErrorCode() const { return errorCode; }
+	const std::string &getErrorMessage() const { return errorMessage; }
 	const std::string &getResultString() const { return resultString; }
 
 	// Currently only used in PUTS, but potentially useful elsewhere
@@ -74,7 +76,7 @@ class HTTPRequest {
 	std::string errorCode;
 
 	std::string resultString;
-	unsigned long responseCode;
+	unsigned long responseCode{0};
 	unsigned long expectedResponseCode = 200;
 	bool includeResponseHeader;
 

src/S3AccessInfo.cc

@@ -49,3 +49,9 @@ const std::string &S3AccessInfo::getS3SecretKeyFile() const {
 void S3AccessInfo::setS3SecretKeyFile(const std::string &s3SecretKeyFile) {
 	s3_secret_key_file = s3SecretKeyFile;
 }
+
+const std::string &S3AccessInfo::getS3UrlStyle() const { return s3_url_style; }
+
+void S3AccessInfo::setS3UrlStyle(const std::string &s3UrlStyle) {
+	s3_url_style = s3UrlStyle;
+}

src/S3AccessInfo.hh

@@ -2,8 +2,7 @@
 // Created by Rich Wellner on 2/29/24.
 //
 
-#ifndef XROOTD_S3_HTTP_S3ACCESSINFO_HH
-#define XROOTD_S3_HTTP_S3ACCESSINFO_HH
+#pragma once
 
 #include <string>
 
@@ -33,13 +32,18 @@ class S3AccessInfo {
 
 	void setS3SecretKeyFile(const std::string &s3SecretKeyFile);
 
+	const std::string &getS3UrlStyle() const;
+
+	void setS3UrlStyle(const std::string &s3UrlStyle);
+
+	const int getS3SignatureVersion() const { return 4; }
+
   private:
 	std::string s3_bucket_name;
 	std::string s3_service_name;
 	std::string s3_region;
 	std::string s3_service_url;
 	std::string s3_access_key_file;
 	std::string s3_secret_key_file;
+	std::string s3_url_style;
 };
-
-#endif // XROOTD_S3_HTTP_S3ACCESSINFO_HH