[XrdHttp] Use mapXrdErrToHttp to improve WebDAV error reporting

- Map XrdErrors to HTTP status codes
- Remove old error mapping
- Update XrdOssTests to allow mimic failures on read/write independently

Author: dynamic-entropy

src/XrdHttp/XrdHttpReq.cc

@@ -817,124 +817,23 @@ void XrdHttpReq::parseResource(char *res) {
     resourceplusopaque.append('?');
     resourceplusopaque.append(p + 1);
   }
-  
-  
-  
 }
 
-void XrdHttpReq::sendWebdavErrorMessage(
-    XResponseType xrdresp, XErrorCode xrderrcode, XrdHttpReq::ReqType httpVerb,
-    XRequestTypes xrdOperation, std::string etext, const char *desc,
-    const char *header_to_add, bool keepalive) {
-  int code{0};
-  std::string errCode{"Unknown"};
-  std::string statusText;
+void XrdHttpReq::generateWebdavErrMsg() {
 
-  switch (httpVerb) {
-    case XrdHttpReq::rtPUT:
-      if (xrdOperation == kXR_open) {
-        if (xrderrcode == kXR_isDirectory) {
-          code = 409;
-          errCode = "8.1";
-        } else if (xrderrcode == kXR_NoSpace) {
-          code = 507;
-          errCode = "8.3.1";
-        } else if (xrderrcode == kXR_overQuota) {
-          code = 507;
-          errCode = "8.3.2";
-        } else if (xrderrcode == kXR_NotAuthorized) {
-          code = 403;
-          errCode = "9.3";
-        }
-      } else if (xrdOperation == kXR_write) {
-        if (xrderrcode == kXR_NoSpace) {
-          code = 507;
-          errCode = "8.4.1";
-        } else if (xrderrcode == kXR_overQuota) {
-          code = 507;
-          errCode = "8.4.2";
-        }
-      }
-      break;
-    default:
-      break;
+  // This block is only used when sending an "X-Transfer-Status" trailer response.
+  // We set the body to "OK" so that the trailer becomes "X-Transfer-Status: 200 OK",
+  // indicating a successful transfer.
+  if (xrdresp == kXR_ok) {
+    httpStatusCode = 200;
+    httpErrorBody = "OK";
+    return;
   }
 
-  // Remove the if at the end of project completion
-  // Till then status text defaults to as set by mapXrdResponseToHttpStatus
-  if (code != 0) {
-    httpStatusCode = code;
-    httpErrorCode = errCode;
-    httpErrorBody = "ERROR: " + errCode + ": " + etext + "\n";
+  // default error
+  httpStatusCode = mapXrdErrToHttp(xrderrcode);
+  httpErrorBody = etext + "\n";
 
-    prot->SendSimpleResp(httpStatusCode, desc, header_to_add,
-                         httpErrorBody.c_str(), httpErrorBody.length(),
-                         keepalive);
-  }
-}
-
-// Map an XRootD error code to an appropriate HTTP status code and message
-// The variables httpStatusCode and httpErrorBody will be populated
-
-void XrdHttpReq::mapXrdErrorToHttpStatus() {
-  // Set default HTTP status values for an error case
-  httpStatusCode = 500;
-  httpErrorBody = "Unrecognized error";
-
-  // Do error mapping
-  if (xrdresp == kXR_error) {
-    switch (xrderrcode) {
-      case kXR_AuthFailed:
-        httpStatusCode = 401; httpErrorBody = "Unauthorized";
-        break;
-      case kXR_NotAuthorized:
-        httpStatusCode = 403; httpErrorBody = "Operation not permitted";
-        break;
-      case kXR_NotFound:
-        httpStatusCode = 404; httpErrorBody = "File not found";
-        break;
-      case kXR_Unsupported:
-        httpStatusCode = 405; httpErrorBody = "Operation not supported";
-        break;
-      case kXR_FileLocked:
-        httpStatusCode = 423; httpErrorBody = "Resource is a locked";
-        break;
-      case kXR_isDirectory:
-        httpStatusCode = 409; httpErrorBody = "Resource is a directory";
-        break;
-      case kXR_ItExists:
-        if(request != ReqType::rtDELETE) {
-          httpStatusCode = 409; httpErrorBody = "File already exists";
-        } else {
-          // In the case the XRootD layer returns a kXR_ItExists after a deletion
-          // was submitted, we return a 405 status code with the error message set by
-          // the XRootD layer
-          httpStatusCode = 405;
-        }
-        break;
-      case kXR_InvalidRequest:
-        httpStatusCode = 405; httpErrorBody = "Method is not allowed";
-        break;
-      case kXR_noserver:
-        httpStatusCode = 502; httpErrorBody = "Bad Gateway";
-        break;
-      case kXR_TimerExpired:
-        httpStatusCode = 504; httpErrorBody = "Gateway timeout";
-        break;
-      default:
-        break;
-    }
-
-    if (!etext.empty()) httpErrorBody = etext;
-
-    TRACEI(REQ, "PostProcessHTTPReq mapping Xrd error [" << xrderrcode
-                 << "] to status code [" << httpStatusCode << "]");
-
-    httpErrorBody += "\n";
-  } else {
-      httpStatusCode = 200;
-      httpErrorBody = "OK";
-  }
 }
 
 int XrdHttpReq::ProcessHTTPReq() {
@@ -1007,14 +906,9 @@ int XrdHttpReq::ProcessHTTPReq() {
   switch (request) {
     case XrdHttpReq::rtUnset:
     case XrdHttpReq::rtUnknown:
-    {
-      prot->SendSimpleResp(400, NULL, NULL, (char *) "Request unknown", 0, false);
-      reset();
-      return -1;
-    }
-    case XrdHttpReq::rtMalformed:
-    {
-      prot->SendSimpleResp(400, NULL, NULL, (char *) "Request malformed", 0, false);
+    case XrdHttpReq::rtMalformed: {
+      generateWebdavErrMsg();
+      prot->SendSimpleResp(httpStatusCode, NULL, NULL, httpErrorBody.c_str(), httpErrorBody.length(), false);
       reset();
       return -1;
     }
@@ -1035,7 +929,8 @@ int XrdHttpReq::ProcessHTTPReq() {
         m_req_cksum = prot->cksumHandler.getChecksumToRun(m_req_digest);
         if(!m_req_cksum) {
             // No HTTP IANA checksums have been configured by the server admin, return a "METHOD_NOT_ALLOWED" error
-            prot->SendSimpleResp(403, NULL, NULL, (char *) "No HTTP-IANA compatible checksums have been configured.", 0, false);
+            // We should not send body in response to HEAD request
+            prot->SendSimpleResp(HTTP_METHOD_NOT_ALLOWED, NULL, NULL, NULL, 0, false);
             return -1;
         }
         if (!opaque) {
@@ -1157,7 +1052,7 @@ int XrdHttpReq::ProcessHTTPReq() {
             m_req_cksum = prot->cksumHandler.getChecksumToRun(m_req_digest);
             if(!m_req_cksum) {
                 // No HTTP IANA checksums have been configured by the server admin, return a "METHOD_NOT_ALLOWED" error
-                prot->SendSimpleResp(403, NULL, NULL, (char *) "No HTTP-IANA compatible checksums have been configured.", 0, false);
+                prot->SendSimpleResp(HTTP_METHOD_NOT_ALLOWED, NULL, NULL, (char *) "No HTTP-IANA compatible checksums have been configured.", 0, false);
                 return -1;
             }
             m_resource_with_digest = resourceplusopaque;
@@ -1185,7 +1080,7 @@ int XrdHttpReq::ProcessHTTPReq() {
             memcpy(xrdreq.close.fhandle, fhandle, 4);
 
             if (!prot->Bridge->Run((char *) &xrdreq, 0, 0)) {
-              mapXrdErrorToHttpStatus();
+              generateWebdavErrMsg();
               return sendFooterError("Could not run close request on the bridge");
             }
             return 0;
@@ -1225,7 +1120,7 @@ int XrdHttpReq::ProcessHTTPReq() {
             xrdreq.dirlist.dlen = htonl(l);
 
             if (!prot->Bridge->Run((char *) &xrdreq, (char *) res.c_str(), l)) {
-              mapXrdErrorToHttpStatus();
+              generateWebdavErrMsg();
               prot->SendSimpleResp(httpStatusCode, NULL, NULL, httpErrorBody.c_str(), httpErrorBody.length(), false);
               sendFooterError("Could not run listing request on the bridge");
               return -1;
@@ -1327,7 +1222,7 @@ int XrdHttpReq::ProcessHTTPReq() {
             }
 
             if (!prot->Bridge->Run((char *) &xrdreq, 0, 0)) {
-              mapXrdErrorToHttpStatus();
+              generateWebdavErrMsg();
               return sendFooterError("Could not run read request on the bridge");
             }
           } else {
@@ -1336,7 +1231,7 @@ int XrdHttpReq::ProcessHTTPReq() {
             length = ReqReadV(readChunkList);
 
             if (!prot->Bridge->Run((char *) &xrdreq, (char *) &ralist[0], length)) {
-              mapXrdErrorToHttpStatus();
+              generateWebdavErrMsg();
               return sendFooterError("Could not run ReadV request on the bridge");
             }
 
@@ -1473,7 +1368,7 @@ int XrdHttpReq::ProcessHTTPReq() {
 
             TRACEI(REQ, "XrdHTTP PUT: Writing chunk of size " << bytes_to_write << " starting with '" << *(prot->myBuffStart) << "'" << " with " << chunk_bytes_remaining << " bytes remaining in the chunk");
             if (!prot->Bridge->Run((char *) &xrdreq, prot->myBuffStart, bytes_to_write)) {
-              mapXrdErrorToHttpStatus();
+              generateWebdavErrMsg();
               return sendFooterError("Could not run write request on the bridge");
             }
             // If there are more bytes in the buffer, then immediately call us after the
@@ -1496,7 +1391,7 @@ int XrdHttpReq::ProcessHTTPReq() {
 
           TRACEI(REQ, "Writing " << bytes_to_read);
           if (!prot->Bridge->Run((char *) &xrdreq, prot->myBuffStart, bytes_to_read)) {
-            mapXrdErrorToHttpStatus();
+            generateWebdavErrMsg();
             return sendFooterError("Could not run write request on the bridge");
           }
 
@@ -1519,7 +1414,7 @@ int XrdHttpReq::ProcessHTTPReq() {
 
 
           if (!prot->Bridge->Run((char *) &xrdreq, 0, 0)) {
-            mapXrdErrorToHttpStatus();
+            generateWebdavErrMsg();
             return sendFooterError("Could not run close request on the bridge");
           }
 
@@ -2134,7 +2029,7 @@ void XrdHttpReq::setTransferStatusHeader(std::string &header) {
 int XrdHttpReq::PostProcessHTTPReq(bool final_) {
 
   TRACEI(REQ, "PostProcessHTTPReq req: " << request << " reqstate: " << reqstate << " final_:" << final_);
-  mapXrdErrorToHttpStatus();
+  generateWebdavErrMsg();
 
   if(xrdreq.set.requestid == htons(kXR_set)) {
     // We have set the user agent, if it fails we return a 500 error, otherwise the callback is successful --> we continue
@@ -2370,8 +2265,7 @@ int XrdHttpReq::PostProcessHTTPReq(bool final_) {
     {
       if (!fopened) {
         if (xrdresp != kXR_ok) {
-          sendWebdavErrorMessage(xrdresp, xrderrcode, XrdHttpReq::rtPUT,
-                                 kXR_open, etext, NULL, NULL, keepalive);
+          prot->SendSimpleResp(httpStatusCode, NULL, NULL, httpErrorBody.c_str(), httpErrorBody.length(), keepalive);
           return -1;
         }
 
@@ -2389,9 +2283,13 @@ int XrdHttpReq::PostProcessHTTPReq(bool final_) {
         break;
       } else {
 
-
         // If we are here it's too late to send a proper error message...
-        if (xrdresp == kXR_error) return -1;
+        // However, we decide to send a response anyway before we close the connection
+        // We are not sure if sending a final response before reading the entire request
+        if (xrdresp == kXR_error) {
+          prot->SendSimpleResp(httpStatusCode, NULL, NULL, httpErrorBody.c_str(), httpErrorBody.length(), keepalive);
+          return -1;
+        }
 
         if (ntohs(xrdreq.header.requestid) == kXR_write) {
           int l = ntohl(xrdreq.write.dlen);
@@ -2416,8 +2314,7 @@ int XrdHttpReq::PostProcessHTTPReq(bool final_) {
             prot->SendSimpleResp(201, NULL, NULL, (char *)":-)", 0, keepalive);
             return keepalive ? 1 : -1;
           } else {
-            sendWebdavErrorMessage(xrdresp, xrderrcode, XrdHttpReq::rtPUT,
-                                   kXR_close, etext, NULL, NULL, keepalive);
+            prot->SendSimpleResp(httpStatusCode, NULL, NULL, httpErrorBody.c_str(), httpErrorBody.length(), keepalive);
             return -1;
           }
         }

src/XrdHttp/XrdHttpReq.hh

@@ -162,14 +162,9 @@ private:
 
   // Parse a resource string, typically a filename, setting the resource field and the opaque data
   void parseResource(char *url);
-  // Map an XRootD error code to an appropriate HTTP status code and message
-  void mapXrdErrorToHttpStatus();
 
   // Set Webdav Error messages
-  void sendWebdavErrorMessage(XResponseType xrdresp, XErrorCode xrderrcode,
-                              ReqType httpVerb, XRequestTypes xrdOperation,
-                              std::string etext, const char *desc,
-                              const char *header_to_add, bool keepalive);
+  void generateWebdavErrMsg();
 
   // Sanitize the resource from http[s]://[host]/ questionable prefix
   void sanitizeResourcePfx();