SSPCPP-616 - clean up concatenated string literals
[shibboleth/cpp-sp.git] / isapi_shib / isapi_shib.cpp
index d4d06bd..3872b3c 100644 (file)
@@ -1,23 +1,27 @@
-/*
- *  Copyright 2001-2007 Internet2
+/**
+ * Licensed to the University Corporation for Advanced Internet
+ * Development, Inc. (UCAID) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
  *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
+ * UCAID licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the
+ * License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied. See the License for the specific
+ * language governing permissions and limitations under the License.
  */
 
 /**
  * isapi_shib.cpp
  *
- * Shibboleth ISAPI filter
+ * Shibboleth ISAPI filter.
  */
 
 #define SHIBSP_LITE
 
 #define _CRT_NONSTDC_NO_DEPRECATE 1
 #define _CRT_SECURE_NO_DEPRECATE 1
+#define _CRT_RAND_S
 
+#include <shibsp/exceptions.h>
 #include <shibsp/AbstractSPRequest.h>
 #include <shibsp/SPConfig.h>
 #include <shibsp/ServiceProvider.h>
+
+#include <set>
+#include <fstream>
+#include <stdexcept>
+#include <process.h>
+#include <boost/lexical_cast.hpp>
 #include <xmltooling/unicode.h>
 #include <xmltooling/XMLToolingConfig.h>
 #include <xmltooling/util/NDC.h>
 #include <xercesc/util/Base64.hpp>
 #include <xercesc/util/XMLUniDefs.hpp>
 
-#include <set>
-#include <sstream>
-#include <fstream>
-#include <stdexcept>
-#include <process.h>
-
 #include <windows.h>
 #include <httpfilt.h>
 #include <httpext.h>
@@ -50,6 +56,7 @@
 using namespace shibsp;
 using namespace xmltooling;
 using namespace xercesc;
+using namespace boost;
 using namespace std;
 
 // globals
@@ -66,19 +73,15 @@ namespace {
 
     struct site_t {
         site_t(const DOMElement* e)
+            : m_name(XMLHelper::getAttrString(e, "", name)),
+                m_scheme(XMLHelper::getAttrString(e, "", scheme)),
+                m_port(XMLHelper::getAttrString(e, "", port)),
+                m_sslport(XMLHelper::getAttrString(e, "", sslport))
         {
-            auto_ptr_char n(e->getAttributeNS(NULL,name));
-            auto_ptr_char s(e->getAttributeNS(NULL,scheme));
-            auto_ptr_char p(e->getAttributeNS(NULL,port));
-            auto_ptr_char p2(e->getAttributeNS(NULL,sslport));
-            if (n.get()) m_name=n.get();
-            if (s.get()) m_scheme=s.get();
-            if (p.get()) m_port=p.get();
-            if (p2.get()) m_sslport=p2.get();
             e = XMLHelper::getFirstChildElement(e, Alias);
             while (e) {
                 if (e->hasChildNodes()) {
-                    auto_ptr_char alias(e->getFirstChild()->getNodeValue());
+                    auto_ptr_char alias(e->getTextContent());
                     m_aliases.insert(alias.get());
                 }
                 e = XMLHelper::getNextSiblingElement(e, Alias);
@@ -88,18 +91,14 @@ namespace {
         set<string> m_aliases;
     };
 
-    struct context_t {
-       char* m_user;
-       bool m_checked;
-    };
-
     HINSTANCE g_hinstDLL;
-    SPConfig* g_Config = NULL;
+    SPConfig* g_Config = nullptr;
     map<string,site_t> g_Sites;
     bool g_bNormalizeRequest = true;
-    string g_unsetHeaderValue;
+    string g_unsetHeaderValue,g_spoofKey;
     bool g_checkSpoofing = true;
     bool g_catchAll = false;
+    bool g_bSafeHeaderNames = false;
     vector<string> g_NoCerts;
 }
 
@@ -110,17 +109,28 @@ BOOL LogEvent(
     PSID  lpUserSid,
     LPCSTR  message)
 {
-    LPCSTR  messages[] = {message, NULL};
+    LPCSTR  messages[] = {message, nullptr};
 
     HANDLE hElog = RegisterEventSource(lpUNCServerName, "Shibboleth ISAPI Filter");
-    BOOL res = ReportEvent(hElog, wType, 0, dwEventID, lpUserSid, 1, 0, messages, NULL);
+    BOOL res = ReportEvent(hElog, wType, 0, dwEventID, lpUserSid, 1, 0, messages, nullptr);
     return (DeregisterEventSource(hElog) && res);
 }
 
+void _my_invalid_parameter_handler(
+   const wchar_t * expression,
+   const wchar_t * function,
+   const wchar_t * file,
+   unsigned int line,
+   uintptr_t pReserved
+   )
+{
+    return;
+}
+
 extern "C" __declspec(dllexport) BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID)
 {
-    if (fdwReason==DLL_PROCESS_ATTACH)
-        g_hinstDLL=hinstDLL;
+    if (fdwReason == DLL_PROCESS_ATTACH)
+        g_hinstDLL = hinstDLL;
     return TRUE;
 }
 
@@ -130,12 +140,12 @@ extern "C" BOOL WINAPI GetExtensionVersion(HSE_VERSION_INFO* pVer)
         return FALSE;
 
     if (!g_Config) {
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL,
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr,
                 "Extension mode startup not possible, is the DLL loaded as a filter?");
         return FALSE;
     }
 
-    pVer->dwExtensionVersion=HSE_VERSION;
+    pVer->dwExtensionVersion = HSE_VERSION;
     strncpy(pVer->lpszExtensionDesc,"Shibboleth ISAPI Extension",HSE_MAX_EXT_DLL_NAME_LEN-1);
     return TRUE;
 }
@@ -150,12 +160,12 @@ extern "C" BOOL WINAPI GetFilterVersion(PHTTP_FILTER_VERSION pVer)
     if (!pVer)
         return FALSE;
     else if (g_Config) {
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL,
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr,
                 "Reentrant filter initialization, ignoring...");
         return TRUE;
     }
 
-    g_Config=&SPConfig::getConfig();
+    g_Config = &SPConfig::getConfig();
     g_Config->setFeatures(
         SPConfig::Listener |
         SPConfig::Caching |
@@ -165,60 +175,86 @@ extern "C" BOOL WINAPI GetFilterVersion(PHTTP_FILTER_VERSION pVer)
         SPConfig::Handlers
         );
     if (!g_Config->init()) {
-        g_Config=NULL;
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL,
+        g_Config = nullptr;
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr,
                 "Filter startup failed during library initialization, check native log for help.");
         return FALSE;
     }
 
     try {
-        if (!g_Config->instantiate(NULL, true))
+        if (!g_Config->instantiate(nullptr, true))
             throw runtime_error("unknown error");
     }
-    catch (exception& ex) {
+    catch (std::exception& ex) {
         g_Config->term();
-        g_Config=NULL;
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, ex.what());
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL,
+        g_Config=nullptr;
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, ex.what());
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr,
                 "Filter startup failed to load configuration, check native log for details.");
         return FALSE;
     }
 
     // Access implementation-specifics and site mappings.
-    ServiceProvider* sp=g_Config->getServiceProvider();
+    ServiceProvider* sp = g_Config->getServiceProvider();
     Locker locker(sp);
-    const PropertySet* props=sp->getPropertySet("InProcess");
+    const PropertySet* props = sp->getPropertySet("InProcess");
     if (props) {
-        pair<bool,const char*> unsetValue=props->getString("unsetHeaderValue");
-        if (unsetValue.first)
-            g_unsetHeaderValue = unsetValue.second;
-        pair<bool,bool> flag=props->getBool("checkSpoofing");
+        pair<bool,bool> flag = props->getBool("checkSpoofing");
         g_checkSpoofing = !flag.first || flag.second;
-        flag=props->getBool("catchAll");
+        flag = props->getBool("catchAll");
         g_catchAll = flag.first && flag.second;
 
+        pair<bool,const char*> unsetValue = props->getString("unsetHeaderValue");
+        if (unsetValue.first)
+            g_unsetHeaderValue = unsetValue.second;
+        if (g_checkSpoofing) {
+            unsetValue = props->getString("spoofKey");
+            if (unsetValue.first)
+                g_spoofKey = unsetValue.second;
+            else {
+                _invalid_parameter_handler old = _set_invalid_parameter_handler(_my_invalid_parameter_handler);
+                unsigned int randkey=0,randkey2=0,randkey3=0,randkey4=0;
+                if (rand_s(&randkey) == 0 && rand_s(&randkey2) == 0 && rand_s(&randkey3) == 0 && rand_s(&randkey4) == 0) {
+                    _set_invalid_parameter_handler(old);
+                    g_spoofKey = lexical_cast<string>(randkey) + lexical_cast<string>(randkey2) +
+                        lexical_cast<string>(randkey3) + lexical_cast<string>(randkey4);
+                }
+                else {
+                    _set_invalid_parameter_handler(old);
+                    LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr,
+                            "Filter failed to generate a random anti-spoofing key (if this is Windows 2000 set one manually).");
+                    locker.assign();    // pops lock on SP config
+                    g_Config->term();
+                    g_Config = nullptr;
+                    return FALSE;
+                }
+            }
+        }
+
         props = props->getPropertySet("ISAPI");
         if (props) {
             flag = props->getBool("normalizeRequest");
             g_bNormalizeRequest = !flag.first || flag.second;
-            const DOMElement* child = XMLHelper::getFirstChildElement(props->getElement(),Site);
+            flag = props->getBool("safeHeaderNames");
+            g_bSafeHeaderNames = flag.first && flag.second;
+            const DOMElement* child = XMLHelper::getFirstChildElement(props->getElement(), Site);
             while (child) {
-                auto_ptr_char id(child->getAttributeNS(NULL,id));
-                if (id.get())
-                    g_Sites.insert(pair<string,site_t>(id.get(),site_t(child)));
-                child=XMLHelper::getNextSiblingElement(child,Site);
+                string id(XMLHelper::getAttrString(child, "", id));
+                if (!id.empty())
+                    g_Sites.insert(make_pair(id, site_t(child)));
+                child = XMLHelper::getNextSiblingElement(child, Site);
             }
         }
     }
 
-    pVer->dwFilterVersion=HTTP_FILTER_REVISION;
-    strncpy(pVer->lpszFilterDesc,"Shibboleth ISAPI Filter",SF_MAX_FILTER_DESC_LEN);
+    pVer->dwFilterVersion = HTTP_FILTER_REVISION;
+    strncpy(pVer->lpszFilterDesc, "Shibboleth ISAPI Filter", SF_MAX_FILTER_DESC_LEN);
     pVer->dwFlags=(SF_NOTIFY_ORDER_HIGH |
                    SF_NOTIFY_SECURE_PORT |
                    SF_NOTIFY_NONSECURE_PORT |
                    SF_NOTIFY_PREPROC_HEADERS |
                    SF_NOTIFY_LOG);
-    LogEvent(NULL, EVENTLOG_INFORMATION_TYPE, 7701, NULL, "Filter initialized...");
+    LogEvent(nullptr, EVENTLOG_INFORMATION_TYPE, 7701, nullptr, "Filter initialized...");
     return TRUE;
 }
 
@@ -226,8 +262,8 @@ extern "C" BOOL WINAPI TerminateFilter(DWORD)
 {
     if (g_Config)
         g_Config->term();
-    g_Config = NULL;
-    LogEvent(NULL, EVENTLOG_INFORMATION_TYPE, 7701, NULL, "Filter shut down...");
+    g_Config = nullptr;
+    LogEvent(nullptr, EVENTLOG_INFORMATION_TYPE, 7701, nullptr, "Filter shut down...");
     return TRUE;
 }
 
@@ -244,7 +280,7 @@ extern "C" BOOL WINAPI TerminateFilter(DWORD)
 class dynabuf
 {
 public:
-    dynabuf() { bufptr=NULL; buflen=0; }
+    dynabuf() { bufptr=nullptr; buflen=0; }
     dynabuf(size_t s) { bufptr=new char[buflen=s]; *bufptr=0; }
     ~dynabuf() { delete[] bufptr; }
     size_t length() const { return bufptr ? strlen(bufptr) : 0; }
@@ -275,67 +311,12 @@ void dynabuf::reserve(size_t s, bool keep)
 
 bool dynabuf::operator==(const char* s) const
 {
-    if (buflen==NULL || s==NULL)
-        return (buflen==NULL && s==NULL);
+    if (buflen==0 || s==nullptr)
+        return (buflen==0 && s==nullptr);
     else
         return strcmp(bufptr,s)==0;
 }
 
-void GetServerVariable(PHTTP_FILTER_CONTEXT pfc, LPSTR lpszVariable, dynabuf& s, DWORD size=80, bool bRequired=true)
-{
-    s.reserve(size);
-    s.erase();
-    size=s.size();
-
-    while (!pfc->GetServerVariable(pfc,lpszVariable,s,&size)) {
-        // Grumble. Check the error.
-        DWORD e=GetLastError();
-        if (e==ERROR_INSUFFICIENT_BUFFER)
-            s.reserve(size);
-        else
-            break;
-    }
-    if (bRequired && s.empty())
-        throw ERROR_NO_DATA;
-}
-
-void GetServerVariable(LPEXTENSION_CONTROL_BLOCK lpECB, LPSTR lpszVariable, dynabuf& s, DWORD size=80, bool bRequired=true)
-{
-    s.reserve(size);
-    s.erase();
-    size=s.size();
-
-    while (!lpECB->GetServerVariable(lpECB->ConnID,lpszVariable,s,&size)) {
-        // Grumble. Check the error.
-        DWORD e=GetLastError();
-        if (e==ERROR_INSUFFICIENT_BUFFER)
-            s.reserve(size);
-        else
-            break;
-    }
-    if (bRequired && s.empty())
-        throw ERROR_NO_DATA;
-}
-
-void GetHeader(PHTTP_FILTER_PREPROC_HEADERS pn, PHTTP_FILTER_CONTEXT pfc,
-               LPSTR lpszName, dynabuf& s, DWORD size=80, bool bRequired=true)
-{
-    s.reserve(size);
-    s.erase();
-    size=s.size();
-
-    while (!pn->GetHeader(pfc,lpszName,s,&size)) {
-        // Grumble. Check the error.
-        DWORD e=GetLastError();
-        if (e==ERROR_INSUFFICIENT_BUFFER)
-            s.reserve(size);
-        else
-            break;
-    }
-    if (bRequired && s.empty())
-        throw ERROR_NO_DATA;
-}
-
 /****************************************************************************/
 // ISAPI Filter
 
@@ -348,20 +329,26 @@ class ShibTargetIsapiF : public AbstractSPRequest
   string m_scheme,m_hostname;
   mutable string m_remote_addr,m_content_type,m_method;
   dynabuf m_allhttp;
+  bool m_firsttime;
 
 public:
   ShibTargetIsapiF(PHTTP_FILTER_CONTEXT pfc, PHTTP_FILTER_PREPROC_HEADERS pn, const site_t& site)
-      : AbstractSPRequest(SHIBSP_LOGCAT".ISAPI"), m_pfc(pfc), m_pn(pn), m_allhttp(4096) {
+      : AbstractSPRequest(SHIBSP_LOGCAT ".ISAPI"), m_pfc(pfc), m_pn(pn), m_allhttp(4096), m_firsttime(true) {
 
     // URL path always come from IIS.
     dynabuf var(256);
-    GetHeader(pn,pfc,"url",var,256,false);
+    GetHeader("url",var,256,false);
     setRequestURI(var);
 
     // Port may come from IIS or from site def.
     if (!g_bNormalizeRequest || (pfc->fIsSecurePort && site.m_sslport.empty()) || (!pfc->fIsSecurePort && site.m_port.empty())) {
-        GetServerVariable(pfc,"SERVER_PORT",var,10);
-        m_port = atoi(var);
+        GetServerVariable("SERVER_PORT",var,10);
+        if (var.empty()) {
+            m_port = pfc->fIsSecurePort ? 443 : 80;
+        }
+        else {
+            m_port = atoi(var);
+        }
     }
     else if (pfc->fIsSecurePort) {
         m_port = atoi(site.m_sslport.c_str());
@@ -375,20 +362,26 @@ public:
     if (m_scheme.empty() || !g_bNormalizeRequest)
         m_scheme=pfc->fIsSecurePort ? "https" : "http";
 
-    GetServerVariable(pfc,"SERVER_NAME",var,32);
+    GetServerVariable("SERVER_NAME",var,32);
 
-    // Make sure SERVER_NAME is "authorized" for use on this site. If not, set to canonical name.
-    m_hostname = var;
-    if (site.m_name!=m_hostname && site.m_aliases.find(m_hostname)==site.m_aliases.end())
-        m_hostname=site.m_name;
+    // Make sure SERVER_NAME is "authorized" for use on this site. If not, or empty, set to canonical name.
+    if (var.empty()) {
+        m_hostname = site.m_name;
+    }
+    else {
+        m_hostname = var;
+        if (site.m_name != m_hostname && site.m_aliases.find(m_hostname) == site.m_aliases.end())
+            m_hostname = site.m_name;
+    }
 
-    if (!pfc->pFilterContext) {
-        pfc->pFilterContext = pfc->AllocMem(pfc, sizeof(context_t), NULL);
-        if (static_cast<context_t*>(pfc->pFilterContext)) {
-            static_cast<context_t*>(pfc->pFilterContext)->m_user = NULL;
-            static_cast<context_t*>(pfc->pFilterContext)->m_checked = false;
-        }
+    if (!g_spoofKey.empty()) {
+        GetHeader("ShibSpoofCheck:", var, 32, false);
+        if (!var.empty() && g_spoofKey == (char*)var)
+            m_firsttime = false;
     }
+
+    if (!m_firsttime)
+        log(SPDebug, "ISAPI filter running more than once");
   }
   ~ShibTargetIsapiF() { }
 
@@ -403,13 +396,13 @@ public:
   }
   const char* getQueryString() const {
       const char* uri = getRequestURI();
-      uri = (uri ? strchr(uri, '?') : NULL);
-      return uri ? (uri + 1) : NULL;
+      uri = (uri ? strchr(uri, '?') : nullptr);
+      return uri ? (uri + 1) : nullptr;
   }
   const char* getMethod() const {
     if (m_method.empty()) {
         dynabuf var(5);
-        GetServerVariable(m_pfc,"HTTP_METHOD",var,5,false);
+        GetServerVariable("HTTP_METHOD",var,5,false);
         if (!var.empty())
             m_method = var;
     }
@@ -418,7 +411,7 @@ public:
   string getContentType() const {
     if (m_content_type.empty()) {
         dynabuf var(32);
-        GetServerVariable(m_pfc,"HTTP_CONTENT_TYPE",var,32,false);
+        GetServerVariable("HTTP_CONTENT_TYPE",var,32,false);
         if (!var.empty())
             m_content_type = var;
     }
@@ -428,72 +421,99 @@ public:
     m_remote_addr = AbstractSPRequest::getRemoteAddr();
     if (m_remote_addr.empty()) {
         dynabuf var(16);
-        GetServerVariable(m_pfc,"REMOTE_ADDR",var,16,false);
+        GetServerVariable("REMOTE_ADDR",var,16,false);
         if (!var.empty())
             m_remote_addr = var;
     }
     return m_remote_addr;
   }
-  void log(SPLogLevel level, const string& msg) {
+  void log(SPLogLevel level, const string& msg) const {
     AbstractSPRequest::log(level,msg);
-    if (level >= SPError)
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, msg.c_str());
+    if (level >= SPCrit)
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, msg.c_str());
+  }
+  string makeSafeHeader(const char* rawname) const {
+      string hdr;
+      for (; *rawname; ++rawname) {
+          if (isalnum(*rawname))
+              hdr += *rawname;
+      }
+      return (hdr + ':');
   }
   void clearHeader(const char* rawname, const char* cginame) {
-       if (g_checkSpoofing && m_pfc->pFilterContext && !static_cast<context_t*>(m_pfc->pFilterContext)->m_checked) {
+    if (g_checkSpoofing && m_firsttime) {
         if (m_allhttp.empty())
-               GetServerVariable(m_pfc,"ALL_HTTP",m_allhttp,4096);
-        if (strstr(m_allhttp, cginame))
-            throw opensaml::SecurityPolicyException("Attempt to spoof header ($1) was detected.", params(1, rawname));
+               GetServerVariable( "ALL_HTTP", m_allhttp, 4096, false);
+        if (!m_allhttp.empty()) {
+            string hdr = g_bSafeHeaderNames ? ("HTTP_" + makeSafeHeader(cginame + 5)) : (string(cginame) + ':');
+            if (strstr(m_allhttp, hdr.c_str()))
+                throw opensaml::SecurityPolicyException("Attempt to spoof header ($1) was detected.", params(1, hdr.c_str()));
+        }
     }
-    string hdr(!strcmp(rawname,"REMOTE_USER") ? "remote-user" : rawname);
-    hdr += ':';
-    m_pn->SetHeader(m_pfc, const_cast<char*>(hdr.c_str()), const_cast<char*>(g_unsetHeaderValue.c_str()));
+    if (g_bSafeHeaderNames) {
+        string hdr = makeSafeHeader(rawname);
+        m_pn->SetHeader(m_pfc, const_cast<char*>(hdr.c_str()), const_cast<char*>(g_unsetHeaderValue.c_str()));
+    }
+    else if (!strcmp(rawname,"REMOTE_USER")) {
+           m_pn->SetHeader(m_pfc, "remote-user:", const_cast<char*>(g_unsetHeaderValue.c_str()));
+        m_pn->SetHeader(m_pfc, "remote_user:", const_cast<char*>(g_unsetHeaderValue.c_str()));
+       }
+       else {
+           string hdr = string(rawname) + ':';
+           m_pn->SetHeader(m_pfc, const_cast<char*>(hdr.c_str()), const_cast<char*>(g_unsetHeaderValue.c_str()));
+       }
   }
   void setHeader(const char* name, const char* value) {
-    string hdr(name);
-    hdr += ':';
+    string hdr = g_bSafeHeaderNames ? makeSafeHeader(name) : (string(name) + ':');
     m_pn->SetHeader(m_pfc, const_cast<char*>(hdr.c_str()), const_cast<char*>(value));
   }
+  string getSecureHeader(const char* name) const {
+    string hdr = g_bSafeHeaderNames ? makeSafeHeader(name) : (string(name) + ':');
+    dynabuf buf(256);
+    GetHeader(const_cast<char*>(hdr.c_str()), buf, 256, false);
+    return string(buf.empty() ? "" : buf);
+  }
   string getHeader(const char* name) const {
     string hdr(name);
     hdr += ':';
     dynabuf buf(256);
-    GetHeader(m_pn, m_pfc, const_cast<char*>(hdr.c_str()), buf, 256, false);
-    return string(buf);
+    GetHeader(const_cast<char*>(hdr.c_str()), buf, 256, false);
+    return string(buf.empty() ? "" : buf);
   }
   void setRemoteUser(const char* user) {
     setHeader("remote-user", user);
-    if (m_pfc->pFilterContext) {
-        if (!user || !*user)
-            static_cast<context_t*>(m_pfc->pFilterContext)->m_user = NULL;
-        else if (static_cast<context_t*>(m_pfc->pFilterContext)->m_user = (char*)m_pfc->AllocMem(m_pfc, sizeof(char) * (strlen(user) + 1), NULL))
-            strcpy(static_cast<context_t*>(m_pfc->pFilterContext)->m_user, user);
-    }
+    if (!user || !*user)
+        m_pfc->pFilterContext = nullptr;
+    else if (m_pfc->pFilterContext = m_pfc->AllocMem(m_pfc, sizeof(char) * (strlen(user) + 1), 0))
+        strcpy(reinterpret_cast<char*>(m_pfc->pFilterContext), user);
   }
   string getRemoteUser() const {
-    return getHeader("remote-user");
+    return getSecureHeader("remote-user");
   }
   void setResponseHeader(const char* name, const char* value) {
-    // Set for later.
-    if (value)
-        m_headers.insert(make_pair(name,value));
-    else
-        m_headers.erase(name);
+    HTTPResponse::setResponseHeader(name, value);
+    if (name) {
+        // Set for later.
+        if (value)
+            m_headers.insert(make_pair(name,value));
+        else
+            m_headers.erase(name);
+    }
   }
   long sendResponse(istream& in, long status) {
     string hdr = string("Connection: close\r\n");
-    for (multimap<string,string>::const_iterator i=m_headers.begin(); i!=m_headers.end(); ++i)
+    for (multimap<string,string>::const_iterator i = m_headers.begin(); i != m_headers.end(); ++i)
         hdr += i->first + ": " + i->second + "\r\n";
     hdr += "\r\n";
     const char* codestr="200 OK";
     switch (status) {
+        case XMLTOOLING_HTTP_STATUS_NOTMODIFIED:    codestr="304 Not Modified"; break;
         case XMLTOOLING_HTTP_STATUS_UNAUTHORIZED:   codestr="401 Authorization Required"; break;
         case XMLTOOLING_HTTP_STATUS_FORBIDDEN:      codestr="403 Forbidden"; break;
         case XMLTOOLING_HTTP_STATUS_NOTFOUND:       codestr="404 Not Found"; break;
         case XMLTOOLING_HTTP_STATUS_ERROR:          codestr="500 Server Error"; break;
     }
-    m_pfc->ServerSupportFunction(m_pfc, SF_REQ_SEND_RESPONSE_HEADER, (void*)codestr, (DWORD)hdr.c_str(), 0);
+    m_pfc->ServerSupportFunction(m_pfc, SF_REQ_SEND_RESPONSE_HEADER, (void*)codestr, (ULONG_PTR)hdr.c_str(), 0);
     char buf[1024];
     while (in) {
         in.read(buf,1024);
@@ -503,16 +523,16 @@ public:
     return SF_STATUS_REQ_FINISHED;
   }
   long sendRedirect(const char* url) {
-    // XXX: Don't support the httpRedirect option, yet.
+    HTTPResponse::sendRedirect(url);
     string hdr=string("Location: ") + url + "\r\n"
       "Content-Type: text/html\r\n"
       "Content-Length: 40\r\n"
-      "Expires: 01-Jan-1997 12:00:00 GMT\r\n"
-      "Cache-Control: private,no-store,no-cache\r\n";
-    for (multimap<string,string>::const_iterator i=m_headers.begin(); i!=m_headers.end(); ++i)
+      "Expires: Wed, 01 Jan 1997 12:00:00 GMT\r\n"
+      "Cache-Control: private,no-store,no-cache,max-age=0\r\n";
+    for (multimap<string,string>::const_iterator i = m_headers.begin(); i != m_headers.end(); ++i)
         hdr += i->first + ": " + i->second + "\r\n";
     hdr += "\r\n";
-    m_pfc->ServerSupportFunction(m_pfc, SF_REQ_SEND_RESPONSE_HEADER, "302 Please Wait", (DWORD)hdr.c_str(), 0);
+    m_pfc->ServerSupportFunction(m_pfc, SF_REQ_SEND_RESPONSE_HEADER, "302 Please Wait", (ULONG_PTR)hdr.c_str(), 0);
     static const char* redmsg="<HTML><BODY>Redirecting...</BODY></HTML>";
     DWORD resplen=40;
     m_pfc->WriteClient(m_pfc, (LPVOID)redmsg, &resplen, 0);
@@ -532,13 +552,47 @@ public:
   // The filter never processes the POST, so stub these methods.
   long getContentLength() const { throw IOException("The request's Content-Length is not available to an ISAPI filter."); }
   const char* getRequestBody() const { throw IOException("The request body is not available to an ISAPI filter."); }
+
+  void GetServerVariable(LPSTR lpszVariable, dynabuf& s, DWORD size=80, bool bRequired=true) const {
+    s.reserve(size);
+    s.erase();
+    size=s.size();
+
+    while (!m_pfc->GetServerVariable(m_pfc,lpszVariable,s,&size)) {
+        // Grumble. Check the error.
+        DWORD e = GetLastError();
+        if (e == ERROR_INSUFFICIENT_BUFFER)
+            s.reserve(size);
+        else
+            break;
+    }
+    if (bRequired && s.empty())
+        log(SPRequest::SPError, string("missing required server variable: ") + lpszVariable);
+  }
+
+  void GetHeader(LPSTR lpszName, dynabuf& s, DWORD size=80, bool bRequired=true) const {
+    s.reserve(size);
+    s.erase();
+    size=s.size();
+
+    while (!m_pn->GetHeader(m_pfc,lpszName,s,&size)) {
+        // Grumble. Check the error.
+        DWORD e = GetLastError();
+        if (e == ERROR_INSUFFICIENT_BUFFER)
+            s.reserve(size);
+        else
+            break;
+    }
+    if (bRequired && s.empty())
+        log(SPRequest::SPError, string("missing required header: ") + lpszName);
+  }
 };
 
 DWORD WriteClientError(PHTTP_FILTER_CONTEXT pfc, const char* msg)
 {
-    LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, msg);
+    LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, msg);
     static const char* ctype="Connection: close\r\nContent-Type: text/html\r\n\r\n";
-    pfc->ServerSupportFunction(pfc,SF_REQ_SEND_RESPONSE_HEADER,"200 OK",(DWORD)ctype,0);
+    pfc->ServerSupportFunction(pfc,SF_REQ_SEND_RESPONSE_HEADER,"200 OK",(ULONG_PTR)ctype,0);
     static const char* xmsg="<HTML><HEAD><TITLE>Shibboleth Filter Error</TITLE></HEAD><BODY>"
                             "<H1>Shibboleth Filter Error</H1>";
     DWORD resplen=strlen(xmsg);
@@ -551,40 +605,60 @@ DWORD WriteClientError(PHTTP_FILTER_CONTEXT pfc, const char* msg)
     return SF_STATUS_REQ_FINISHED;
 }
 
+void GetServerVariable(PHTTP_FILTER_CONTEXT pfc, LPSTR lpszVariable, dynabuf& s, DWORD size=80, bool bRequired=true)
+{
+    s.reserve(size);
+    s.erase();
+    size=s.size();
+
+    while (!pfc->GetServerVariable(pfc,lpszVariable,s,&size)) {
+        // Grumble. Check the error.
+        DWORD e=GetLastError();
+        if (e==ERROR_INSUFFICIENT_BUFFER)
+            s.reserve(size);
+        else
+            break;
+    }
+    if (bRequired && s.empty()) {
+        string msg = string("Missing required server variable: ") + lpszVariable;
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, msg.c_str());
+    }
+}
+
+
 extern "C" DWORD WINAPI HttpFilterProc(PHTTP_FILTER_CONTEXT pfc, DWORD notificationType, LPVOID pvNotification)
 {
     // Is this a log notification?
-    if (notificationType==SF_NOTIFY_LOG) {
-        if (pfc->pFilterContext && static_cast<context_t*>(pfc->pFilterContext)->m_user)
-               ((PHTTP_FILTER_LOG)pvNotification)->pszClientUserName=static_cast<context_t*>(pfc->pFilterContext)->m_user;
+    if (notificationType == SF_NOTIFY_LOG) {
+        if (pfc->pFilterContext)
+               ((PHTTP_FILTER_LOG)pvNotification)->pszClientUserName = reinterpret_cast<char*>(pfc->pFilterContext);
         return SF_STATUS_REQ_NEXT_NOTIFICATION;
     }
 
     PHTTP_FILTER_PREPROC_HEADERS pn=(PHTTP_FILTER_PREPROC_HEADERS)pvNotification;
-    try
-    {
+    try {
         // Determine web site number. This can't really fail, I don't think.
         dynabuf buf(128);
         GetServerVariable(pfc,"INSTANCE_ID",buf,10);
+        if (buf.empty())
+            return WriteClientError(pfc, "Shibboleth Filter failed to obtain INSTANCE_ID server variable.");
 
         // Match site instance to host name, skip if no match.
-        map<string,site_t>::const_iterator map_i=g_Sites.find(static_cast<char*>(buf));
-        if (map_i==g_Sites.end())
+        map<string,site_t>::const_iterator map_i = g_Sites.find(static_cast<char*>(buf));
+        if (map_i == g_Sites.end())
             return SF_STATUS_REQ_NEXT_NOTIFICATION;
 
-        ostringstream threadid;
-        threadid << "[" << getpid() << "] isapi_shib" << '\0';
-        xmltooling::NDC ndc(threadid.str().c_str());
+        string threadid("[");
+        threadid += lexical_cast<string>(getpid()) + "] isapi_shib";
+        xmltooling::NDC ndc(threadid.c_str());
 
         ShibTargetIsapiF stf(pfc, pn, map_i->second);
 
-        // "false" because we don't override the Shib settings
         pair<bool,long> res = stf.getServiceProvider().doAuthentication(stf);
-        if (pfc->pFilterContext)
-            static_cast<context_t*>(pfc->pFilterContext)->m_checked = true;
+        if (!g_spoofKey.empty())
+            pn->SetHeader(pfc, "ShibSpoofCheck:", const_cast<char*>(g_spoofKey.c_str()));
         if (res.first) return res.second;
 
-        // "false" because we don't override the Shib settings
         res = stf.getServiceProvider().doExport(stf);
         if (res.first) return res.second;
 
@@ -594,26 +668,26 @@ extern "C" DWORD WINAPI HttpFilterProc(PHTTP_FILTER_CONTEXT pfc, DWORD notificat
         return SF_STATUS_REQ_NEXT_NOTIFICATION;
     }
     catch(bad_alloc) {
-        return WriteClientError(pfc,"Out of Memory");
+        return WriteClientError(pfc, "Out of Memory");
     }
     catch(long e) {
         if (e==ERROR_NO_DATA)
-            return WriteClientError(pfc,"A required variable or header was empty.");
+            return WriteClientError(pfc, "A required variable or header was empty.");
         else
-            return WriteClientError(pfc,"Shibboleth Filter detected unexpected IIS error.");
+            return WriteClientError(pfc, "Shibboleth Filter detected unexpected IIS error.");
     }
-    catch (exception& e) {
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, e.what());
-        return WriteClientError(pfc,"Shibboleth Filter caught an exception, check Event Log for details.");
+    catch (std::exception& e) {
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, e.what());
+        return WriteClientError(pfc, "Shibboleth Filter caught an exception, check Event Log for details.");
     }
     catch(...) {
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, "Shibboleth Filter threw an unknown exception.");
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, "Shibboleth Filter threw an unknown exception.");
         if (g_catchAll)
-            return WriteClientError(pfc,"Shibboleth Filter threw an unknown exception.");
+            return WriteClientError(pfc, "Shibboleth Filter threw an unknown exception.");
         throw;
     }
 
-    return WriteClientError(pfc,"Shibboleth Filter reached unreachable code, save my walrus!");
+    return WriteClientError(pfc, "Shibboleth Filter reached unreachable code, save my walrus!");
 }
 
 
@@ -622,7 +696,7 @@ extern "C" DWORD WINAPI HttpFilterProc(PHTTP_FILTER_CONTEXT pfc, DWORD notificat
 
 DWORD WriteClientError(LPEXTENSION_CONTROL_BLOCK lpECB, const char* msg)
 {
-    LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, msg);
+    LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, msg);
     static const char* ctype="Connection: close\r\nContent-Type: text/html\r\n\r\n";
     lpECB->ServerSupportFunction(lpECB->ConnID,HSE_REQ_SEND_RESPONSE_HEADER,"200 OK",0,(LPDWORD)ctype);
     static const char* xmsg="<HTML><HEAD><TITLE>Shibboleth Error</TITLE></HEAD><BODY><H1>Shibboleth Error</H1>";
@@ -650,41 +724,49 @@ class ShibTargetIsapiE : public AbstractSPRequest
 
 public:
   ShibTargetIsapiE(LPEXTENSION_CONTROL_BLOCK lpECB, const site_t& site)
-      : AbstractSPRequest(SHIBSP_LOGCAT".ISAPI"), m_lpECB(lpECB), m_gotBody(false) {
+      : AbstractSPRequest(SHIBSP_LOGCAT ".ISAPI"), m_lpECB(lpECB), m_gotBody(false) {
     dynabuf ssl(5);
-    GetServerVariable(lpECB,"HTTPS",ssl,5);
+    GetServerVariable("HTTPS",ssl,5);
     bool SSL=(ssl=="on" || ssl=="ON");
 
     // Scheme may come from site def or be derived from IIS.
-    m_scheme=site.m_scheme;
+    m_scheme = site.m_scheme;
     if (m_scheme.empty() || !g_bNormalizeRequest)
         m_scheme = SSL ? "https" : "http";
 
     // URL path always come from IIS.
     dynabuf url(256);
-    GetServerVariable(lpECB,"URL",url,255);
+    GetServerVariable("URL",url,255);
 
     // Port may come from IIS or from site def.
-    dynabuf port(11);
-    if (!g_bNormalizeRequest || (SSL && site.m_sslport.empty()) || (!SSL && site.m_port.empty()))
-        GetServerVariable(lpECB,"SERVER_PORT",port,10);
+    if (!g_bNormalizeRequest || (SSL && site.m_sslport.empty()) || (!SSL && site.m_port.empty())) {
+        dynabuf port(11);
+        GetServerVariable("SERVER_PORT",port,10);
+        if (port.empty()) {
+            m_port = SSL ? 443 : 80;
+        }
+        else {
+            m_port = atoi(port);
+        }
+    }
     else if (SSL) {
-        strncpy(port,site.m_sslport.c_str(),10);
-        static_cast<char*>(port)[10]=0;
+        m_port = atoi(site.m_sslport.c_str());
     }
     else {
-        strncpy(port,site.m_port.c_str(),10);
-        static_cast<char*>(port)[10]=0;
+        m_port = atoi(site.m_port.c_str());
     }
-    m_port = atoi(port);
 
     dynabuf var(32);
-    GetServerVariable(lpECB, "SERVER_NAME", var, 32);
-
-    // Make sure SERVER_NAME is "authorized" for use on this site. If not, set to canonical name.
-    m_hostname=var;
-    if (site.m_name!=m_hostname && site.m_aliases.find(m_hostname)==site.m_aliases.end())
-        m_hostname=site.m_name;
+    GetServerVariable("SERVER_NAME", var, 32);
+    if (var.empty()) {
+        m_hostname = site.m_name;
+    }
+    else {
+        // Make sure SERVER_NAME is "authorized" for use on this site. If not, set to canonical name.
+        m_hostname=var;
+        if (site.m_name != m_hostname && site.m_aliases.find(m_hostname) == site.m_aliases.end())
+            m_hostname = site.m_name;
+    }
 
     /*
      * IIS screws us over on PATH_INFO (the hits keep on coming). We need to figure out if
@@ -711,11 +793,12 @@ public:
             // Pretty good chance we're in bad mode, unless the PathInfo repeats the path itself.
             uri = lpECB->lpszPathInfo;
         else {
-            uri = url;
+            if (!url.empty())
+                uri = url;
             uri += lpECB->lpszPathInfo;
         }
     }
-    else {
+    else if (!url.empty()) {
         uri = url;
     }
 
@@ -727,7 +810,7 @@ public:
 
     setRequestURI(uri.c_str());
   }
-  ~ShibTargetIsapiE() { }
+  ~ShibTargetIsapiE() {}
 
   const char* getScheme() const {
     return m_scheme.c_str();
@@ -750,7 +833,7 @@ public:
   string getRemoteUser() const {
     if (m_remote_user.empty()) {
         dynabuf var(16);
-        GetServerVariable(m_lpECB, "REMOTE_USER", var, 32, false);
+        GetServerVariable("REMOTE_USER", var, 32, false);
         if (!var.empty())
             m_remote_user = var;
     }
@@ -760,7 +843,7 @@ public:
     m_remote_addr = AbstractSPRequest::getRemoteAddr();
     if (m_remote_addr.empty()) {
         dynabuf var(16);
-        GetServerVariable(m_lpECB, "REMOTE_ADDR", var, 16, false);
+        GetServerVariable("REMOTE_ADDR", var, 16, false);
         if (!var.empty())
             m_remote_addr = var;
     }
@@ -768,27 +851,30 @@ public:
   }
   void log(SPLogLevel level, const string& msg) const {
       AbstractSPRequest::log(level,msg);
-      if (level >= SPError)
-          LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, msg.c_str());
+      if (level >= SPCrit)
+          LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, msg.c_str());
   }
   string getHeader(const char* name) const {
     string hdr("HTTP_");
     for (; *name; ++name) {
-        if (*name=='-')
+        if (*name == '-')
             hdr += '_';
         else
             hdr += toupper(*name);
     }
     dynabuf buf(128);
-    GetServerVariable(m_lpECB, const_cast<char*>(hdr.c_str()), buf, 128, false);
+    GetServerVariable(const_cast<char*>(hdr.c_str()), buf, 128, false);
     return buf.empty() ? "" : buf;
   }
   void setResponseHeader(const char* name, const char* value) {
-    // Set for later.
-    if (value)
-        m_headers.insert(make_pair(name,value));
-    else
-        m_headers.erase(name);
+    HTTPResponse::setResponseHeader(name, value);
+    if (name) {
+        // Set for later.
+        if (value)
+            m_headers.insert(make_pair(name,value));
+        else
+            m_headers.erase(name);
+    }
   }
   const char* getQueryString() const {
     return m_lpECB->lpszQueryString;
@@ -800,13 +886,20 @@ public:
         throw opensaml::SecurityPolicyException("Size of request body exceeded 1M size limit.");
     else if (m_lpECB->cbTotalBytes > m_lpECB->cbAvailable) {
       m_gotBody=true;
-      char buf[8192];
       DWORD datalen=m_lpECB->cbTotalBytes;
+      if (m_lpECB->cbAvailable > 0) {
+        m_body.assign(reinterpret_cast<char*>(m_lpECB->lpbData),m_lpECB->cbAvailable);
+        datalen-=m_lpECB->cbAvailable;
+      }
+      char buf[8192];
       while (datalen) {
         DWORD buflen=8192;
         BOOL ret = m_lpECB->ReadClient(m_lpECB->ConnID, buf, &buflen);
-        if (!ret)
-            throw IOException("Error reading request body from browser.");
+        if (!ret) {
+            char message[65];
+            _snprintf(message, 64, "Error reading request body from browser (%x).", GetLastError());
+            throw IOException(message);
+        }
         else if (!buflen)
             throw IOException("Socket closed while reading request body from browser.");
         m_body.append(buf, buflen);
@@ -821,11 +914,12 @@ public:
   }
   long sendResponse(istream& in, long status) {
     string hdr = string("Connection: close\r\n");
-    for (multimap<string,string>::const_iterator i=m_headers.begin(); i!=m_headers.end(); ++i)
+    for (multimap<string,string>::const_iterator i = m_headers.begin(); i != m_headers.end(); ++i)
         hdr += i->first + ": " + i->second + "\r\n";
     hdr += "\r\n";
     const char* codestr="200 OK";
     switch (status) {
+        case XMLTOOLING_HTTP_STATUS_NOTMODIFIED:    codestr="304 Not Modified"; break;
         case XMLTOOLING_HTTP_STATUS_UNAUTHORIZED:   codestr="401 Authorization Required"; break;
         case XMLTOOLING_HTTP_STATUS_FORBIDDEN:      codestr="403 Forbidden"; break;
         case XMLTOOLING_HTTP_STATUS_NOTFOUND:       codestr="404 Not Found"; break;
@@ -841,12 +935,13 @@ public:
     return HSE_STATUS_SUCCESS;
   }
   long sendRedirect(const char* url) {
+    HTTPResponse::sendRedirect(url);
     string hdr=string("Location: ") + url + "\r\n"
       "Content-Type: text/html\r\n"
       "Content-Length: 40\r\n"
-      "Expires: 01-Jan-1997 12:00:00 GMT\r\n"
-      "Cache-Control: private,no-store,no-cache\r\n";
-    for (multimap<string,string>::const_iterator i=m_headers.begin(); i!=m_headers.end(); ++i)
+      "Expires: Wed, 01 Jan 1997 12:00:00 GMT\r\n"
+      "Cache-Control: private,no-store,no-cache,max-age=0\r\n";
+    for (multimap<string,string>::const_iterator i = m_headers.begin(); i != m_headers.end(); ++i)
         hdr += i->first + ": " + i->second + "\r\n";
     hdr += "\r\n";
     m_lpECB->ServerSupportFunction(m_lpECB->ConnID, HSE_REQ_SEND_RESPONSE_HEADER, "302 Moved", 0, (LPDWORD)hdr.c_str());
@@ -855,7 +950,7 @@ public:
     m_lpECB->WriteClient(m_lpECB->ConnID, (LPVOID)redmsg, &resplen, HSE_IO_SYNC);
     return HSE_STATUS_SUCCESS;
   }
-  // Decline happens in the POST processor if this isn't the shire url
+  // Decline happens in the POST processor if this isn't the handler url
   // Note that it can also happen with HTAccess, but we don't support that, yet.
   long returnDecline() {
     return WriteClientError(
@@ -876,7 +971,7 @@ public:
         ccex.CertContext.pbCertEncoded = (BYTE*)CertificateBuf;
         DWORD dwSize = sizeof(ccex);
 
-        if (m_lpECB->ServerSupportFunction(m_lpECB->ConnID, HSE_REQ_GET_CERT_INFO_EX, (LPVOID)&ccex, (LPDWORD)dwSize, NULL)) {
+        if (m_lpECB->ServerSupportFunction(m_lpECB->ConnID, HSE_REQ_GET_CERT_INFO_EX, (LPVOID)&ccex, (LPDWORD)dwSize, nullptr)) {
             if (ccex.CertContext.cbCertEncoded) {
                 xsecsize_t outlen;
                 XMLByte* serialized = Base64::encode(reinterpret_cast<XMLByte*>(CertificateBuf), ccex.CertContext.cbCertEncoded, &outlen);
@@ -896,23 +991,62 @@ public:
   void clearHeader(const char* rawname, const char* cginame) { throw runtime_error("clearHeader not implemented"); }
   void setHeader(const char* name, const char* value) { throw runtime_error("setHeader not implemented"); }
   void setRemoteUser(const char* user) { throw runtime_error("setRemoteUser not implemented"); }
+
+  void GetServerVariable(LPSTR lpszVariable, dynabuf& s, DWORD size=80, bool bRequired=true) const {
+    s.reserve(size);
+    s.erase();
+    size=s.size();
+
+    while (!m_lpECB->GetServerVariable(m_lpECB->ConnID,lpszVariable,s,&size)) {
+        // Grumble. Check the error.
+        DWORD e=GetLastError();
+        if (e==ERROR_INSUFFICIENT_BUFFER)
+            s.reserve(size);
+        else
+            break;
+    }
+    if (bRequired && s.empty())
+        log(SPRequest::SPError, string("missing required server variable: ") + lpszVariable);
+  }
 };
 
+void GetServerVariable(LPEXTENSION_CONTROL_BLOCK lpECB, LPSTR lpszVariable, dynabuf& s, DWORD size=80, bool bRequired=true)
+{
+    s.reserve(size);
+    s.erase();
+    size=s.size();
+
+    while (!lpECB->GetServerVariable(lpECB->ConnID,lpszVariable,s,&size)) {
+        // Grumble. Check the error.
+        DWORD e=GetLastError();
+        if (e==ERROR_INSUFFICIENT_BUFFER)
+            s.reserve(size);
+        else
+            break;
+    }
+    if (bRequired && s.empty()) {
+        string msg = string("Missing required server variable: ") + lpszVariable;
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, msg.c_str());
+    }
+}
+
 extern "C" DWORD WINAPI HttpExtensionProc(LPEXTENSION_CONTROL_BLOCK lpECB)
 {
     try {
-        ostringstream threadid;
-        threadid << "[" << getpid() << "] isapi_shib_extension" << '\0';
-        xmltooling::NDC ndc(threadid.str().c_str());
+        string threadid("[");
+        threadid += lexical_cast<string>(getpid()) + "] isapi_shib_extension";
+        xmltooling::NDC ndc(threadid.c_str());
 
         // Determine web site number. This can't really fail, I don't think.
         dynabuf buf(128);
         GetServerVariable(lpECB,"INSTANCE_ID",buf,10);
+        if (buf.empty())
+            return WriteClientError(lpECB, "Shibboleth Extension failed to obtain INSTANCE_ID server variable.");
 
         // Match site instance to host name, skip if no match.
-        map<string,site_t>::const_iterator map_i=g_Sites.find(static_cast<char*>(buf));
-        if (map_i==g_Sites.end())
-            return WriteClientError(lpECB, "Shibboleth Extension not configured for web site (check <ISAPI> mappings in configuration).");
+        map<string,site_t>::const_iterator map_i = g_Sites.find(static_cast<char*>(buf));
+        if (map_i == g_Sites.end())
+            return WriteClientError(lpECB, "Shibboleth Extension not configured for web site (check ISAPI mappings in SP configuration).");
 
         ShibTargetIsapiE ste(lpECB, map_i->second);
         pair<bool,long> res = ste.getServiceProvider().doHandler(ste);
@@ -922,22 +1056,22 @@ extern "C" DWORD WINAPI HttpExtensionProc(LPEXTENSION_CONTROL_BLOCK lpECB)
 
     }
     catch(bad_alloc) {
-        return WriteClientError(lpECB,"Out of Memory");
+        return WriteClientError(lpECB, "Out of Memory");
     }
     catch(long e) {
         if (e==ERROR_NO_DATA)
-            return WriteClientError(lpECB,"A required variable or header was empty.");
+            return WriteClientError(lpECB, "A required variable or header was empty.");
         else
-            return WriteClientError(lpECB,"Server detected unexpected IIS error.");
+            return WriteClientError(lpECB, "Server detected unexpected IIS error.");
     }
-    catch (exception& e) {
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, e.what());
-        return WriteClientError(lpECB,"Shibboleth Extension caught an exception, check Event Log for details.");
+    catch (std::exception& e) {
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, e.what());
+        return WriteClientError(lpECB, "Shibboleth Extension caught an exception, check Event Log for details.");
     }
     catch(...) {
-        LogEvent(NULL, EVENTLOG_ERROR_TYPE, 2100, NULL, "Shibboleth Extension threw an unknown exception.");
+        LogEvent(nullptr, EVENTLOG_ERROR_TYPE, 2100, nullptr, "Shibboleth Extension threw an unknown exception.");
         if (g_catchAll)
-            return WriteClientError(lpECB,"Shibboleth Extension threw an unknown exception.");
+            return WriteClientError(lpECB, "Shibboleth Extension threw an unknown exception.");
         throw;
     }