[cpp-sp] branch main updated: CPPSP-13 Move remoting tracing to different output sink

Codeberg noreply at shibboleth.net
Wed Aug 26 15:13:19 UTC 2026


This is an automated email from the git hooks/post-receive script.

codeberg pushed a commit to branch main
in repository cpp-sp.

View the commit online:
https://codeberg.org/Shibboleth/cpp-sp/commit/2c6633182f62faf0d3aa8a3749fdc74969f805ab

The following commit(s) were added to refs/heads/main by this push:
     new 2c663318 CPPSP-13 Move remoting tracing to different output sink
2c663318 is described below

commit 2c6633182f62faf0d3aa8a3749fdc74969f805ab
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Wed Aug 26 16:12:52 2026 +0100

    CPPSP-13 Move remoting tracing to different output sink
    
    https://shibboleth.atlassian.net/browse/CPPSP-13
    
    Align tracing with cUrl, but using the ThreadID as the
    uniquifier.
---
 shibsp/remoting/impl/WinHTTPRemotingService.cpp | 131 ++++++++++++++----------
 1 file changed, 78 insertions(+), 53 deletions(-)

diff --git a/shibsp/remoting/impl/WinHTTPRemotingService.cpp b/shibsp/remoting/impl/WinHTTPRemotingService.cpp
index 5c444c0e..54b41573 100644
--- a/shibsp/remoting/impl/WinHTTPRemotingService.cpp
+++ b/shibsp/remoting/impl/WinHTTPRemotingService.cpp
@@ -35,6 +35,7 @@
 
 #include <stdexcept>
 #include <boost/algorithm/string.hpp>
+#include <boost/lexical_cast.hpp>
 #include <boost/property_tree/ptree.hpp>
 
 using namespace shibsp;
@@ -83,10 +84,8 @@ namespace {
         set<string> m_tls12CipherSet;
         set<string> m_tls13CipherSet;
         string getCertName(PCCERT_CONTEXT certContext) const;
-        void writeTraceHeader(string message) const;
-        HANDLE m_traceHandle;
         bool isTracing() const {
-            return m_traceHandle != INVALID_HANDLE_VALUE && m_traceHandle != nullptr;
+            return !getTraceFileBase().empty();
         }
     };
 
@@ -101,6 +100,22 @@ namespace {
         HINTERNETJanitor& operator=(const HINTERNETJanitor&);
     };
 
+    class TraceHandle
+    {
+    public:
+        TraceHandle(HANDLE handle, Category& log) :m_handle(handle), m_log(log) {};
+        ~TraceHandle() { if (isValid()) CloseHandle(m_handle); }
+
+        void writeHeader(const string message) const;
+        void write(const string what) const;
+        void write(const char* buffer, size_t len) const;
+        bool isValid() const { return (m_handle != INVALID_HANDLE_VALUE) && (m_handle != nullptr); };
+
+    private:
+        const HANDLE m_handle;
+        Category& m_log;
+    };
+
 }
 
 namespace shibsp {
@@ -252,7 +267,7 @@ WinHTTPRemotingService::WinHTTPRemotingService(ptree& pt)
     AbstractHTTPRemotingService(pt),
     m_log(Category::getInstance(SHIBSP_LOGCAT ".RemotingService")),
     m_secure(false), m_caChainEngine(nullptr), m_caStore(nullptr),
-    m_init(false), m_traceHandle(INVALID_HANDLE_VALUE)
+    m_init(false)
 {
     if (getUserAgent() == nullptr) {
         string useragent = string(PACKAGE_NAME) + '/' + PACKAGE_VERSION + '/' + "WINHTTP";
@@ -369,21 +384,6 @@ WinHTTPRemotingService::WinHTTPRemotingService(ptree& pt)
         throw runtime_error("WinHHHTP failed to initialize: Could not connect");
     }
 
-    if (!getTraceFileBase().empty()) {
-        m_traceHandle = CreateFileA(getTraceFileBase().c_str(),
-                                  GENERIC_WRITE,
-                                  FILE_SHARE_READ,
-                                  NULL,
-                                  CREATE_ALWAYS,
-                                  FILE_ATTRIBUTE_NORMAL,
-                                  NULL);
-        if (!isTracing()) {
-             m_log.error("Could not open tracing file %s : %d", getTraceFileBase().c_str(), GetLastError());
-        } else {
-            writeTraceHeader("DDIF Tracing Initialized");
-        }
-    }
-
     m_log.info("WinHTTP RemotingService installed for agent ID (%s), baseURL (%s)", getAgentID(), getBaseURL());
 }
 
@@ -401,9 +401,6 @@ WinHTTPRemotingService::~WinHTTPRemotingService()
     if (m_caStore)
         CertCloseStore(m_caStore, 0);
 
-    if (isTracing()) {
-        CloseHandle(m_traceHandle);
-    }
 }
 
 void WinHTTPRemotingService::send(const char* path, istream& input, ostream& output) const
@@ -488,9 +485,34 @@ void WinHTTPRemotingService::send(const char* path, istream& input, ostream& out
         throw RemotingException("Send failed");
     }
 
-    if (isTracing() && !WriteFile(m_traceHandle, startCallMessage.c_str(), static_cast<DWORD>(startCallMessage.length()), NULL, NULL)) {
-            m_log.error("Could not write message to trace file %d", GetLastError());
+    HANDLE handle = INVALID_HANDLE_VALUE;
+    string tracename = getTraceFileBase() + boost::lexical_cast<string>(GetCurrentThreadId()) + ".log";
+    if (isTracing()) {
+        handle = CreateFileA(tracename.c_str(),
+                             GENERIC_WRITE,
+                             FILE_SHARE_READ | FILE_SHARE_DELETE,
+                             NULL,
+                             OPEN_ALWAYS,
+                             FILE_ATTRIBUTE_NORMAL,
+                             NULL);
+        if (handle != INVALID_HANDLE_VALUE) {
+            //
+            // Set up for append
+            //
+            SetFilePointer(handle, 0, NULL, FILE_END);
+        } else {
+            DWORD gle = GetLastError();
+            if (gle == ERROR_SHARING_VIOLATION) {
+                m_log.crit("Sharing error opening %s", tracename.c_str());
+                throw RemotingException("Sharing error opening unshared file");
+            }
+            else {
+                m_log.error("Could not open tracing file %s : %d", getTraceFileBase().c_str(), gle);
+            }
+        }
     }
+    TraceHandle traceHandle(handle, m_log);
+    traceHandle.write(startCallMessage);
 
     //
     // As per CPPSP=73, chunked IO has regressed (when tested against the test harness)
@@ -504,7 +526,7 @@ void WinHTTPRemotingService::send(const char* path, istream& input, ostream& out
             m_log.crit("Send. Failed to send request to %s : %d", path, GetLastError());
             throw RemotingException("Send failed");
         }
-        writeTraceHeader("Chunked DDF request");
+        traceHandle.writeHeader("Chunked DDF request");
 
         DWORD written;
         while (input) {
@@ -534,11 +556,7 @@ void WinHTTPRemotingService::send(const char* path, istream& input, ostream& out
                 m_log.crit("Send. Failed to send chunk trailer to %s : %d", path, GetLastError());
                 throw RemotingException("Send failed");
             }
-            if (isTracing()) {
-                if (!WriteFile(m_traceHandle, buf, transferSize, NULL, NULL) || !WriteFile(m_traceHandle, "\r\n", 2, NULL, NULL)) {
-                    m_log.error("Could not write chunk to trace file %d", GetLastError());
-                }
-            }
+            traceHandle.write(buf, transferSize) ;
         }
         //
         // And when all the data is gone we say that the last chunk is zero long
@@ -565,18 +583,13 @@ void WinHTTPRemotingService::send(const char* path, istream& input, ostream& out
             input.read(buf, sizeof(buf));
             msg.append(buf, input.gcount());
         }
-        writeTraceHeader("Unchunked DDF request");
+        traceHandle.writeHeader("Unchunked DDF request");
         m_log.debug("Sending %d bytes to %s", msg.length(), path);
         if (!WinHttpSendRequest(request, WINHTTP_NO_ADDITIONAL_HEADERS, 0, const_cast<char*>(msg.c_str()), static_cast<DWORD>(msg.length()), static_cast<DWORD>(msg.length()), 0)) {
             m_log.crit("Send. Failed to send request to %s : %d", path, GetLastError());
             throw RemotingException("Send failed");
         }
-        if (isTracing()) {
-            if (!WriteFile(m_traceHandle, msg.c_str(), static_cast<DWORD>(msg.length()), NULL, NULL) ||
-                !WriteFile(m_traceHandle, "\r\n", 2, NULL, NULL)) {
-                m_log.error("Could not write data to trace file %d", GetLastError());
-            }
-        }
+        traceHandle.write(msg);
     }
 
     if (!WinHttpReceiveResponse(request, NULL)) {
@@ -607,7 +620,7 @@ void WinHTTPRemotingService::send(const char* path, istream& input, ostream& out
 
     DWORD bufferSize = 0;
     char *buffer = nullptr;
-    writeTraceHeader("DDF response");
+    traceHandle.writeHeader("DDF response");
     while (true)  {
 
         DWORD bytesAvailable = 0;
@@ -637,16 +650,10 @@ void WinHTTPRemotingService::send(const char* path, istream& input, ostream& out
             throw RemotingException("Send failed");
         }
         m_log.debug("%d bytes read from %s", bytesRead, path);
-        if (isTracing()) {
-            if (!WriteFile(m_traceHandle, buffer, bytesRead, NULL, NULL) || !WriteFile(m_traceHandle, "\r\n", 2, NULL, NULL)) {
-                m_log.error("Could not write message to trace file %d", GetLastError());
-            }
-        }
+        traceHandle.write(buffer, bytesRead);
         output.write(buffer, bytesRead);
     }
-    if (isTracing() && !WriteFile(m_traceHandle, endCallMessage.c_str(), static_cast<DWORD>(endCallMessage.length()), NULL, NULL)) {
-        m_log.error("Could not write message to trace file %d", GetLastError());
-    }
+    traceHandle.write(endCallMessage);
 
     if (buffer) {
         delete[] buffer;
@@ -793,13 +800,31 @@ string WinHTTPRemotingService::getCertName(PCCERT_CONTEXT certContext) const {
 
 }
 
-void WinHTTPRemotingService::writeTraceHeader(string message) const {
-
-    if (!isTracing())
+void TraceHandle::writeHeader(const string message) const {
+    if (!isValid()) {
         return;
+    }
     stringstream sink;
-    sink << message << date::format("  %FT%TZ", date::floor<chrono::milliseconds>(chrono::system_clock::now())) << "\r\n";
-    string msg(sink.str());
-    if (!WriteFile(m_traceHandle, msg.c_str(), static_cast<DWORD>(msg.length()), NULL, NULL))
-        m_log.error("Could not write to travce file %d", GetLastError());
+    sink << message << "\r\n" << date::format("  %FT%TZ", date::floor<chrono::milliseconds>(chrono::system_clock::now())) << "\r\n";
+    write(sink.str());
 }
+
+void TraceHandle::write(const string what) const {
+    if (!isValid()) {
+        return;
+    }
+    write(what.c_str(), what.length());
+}
+
+
+void TraceHandle::write(const char* buffer, size_t len) const
+{
+    if (!isValid()) {
+        return;
+    }
+    if (!WriteFile(m_handle, buffer, static_cast<DWORD>(len), NULL, NULL)) {
+        m_log.error("Could not write to trace file %d", GetLastError());
+    }
+}
+
+

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list