@abh3 commented on this pull request.

Like I said, I'm OK with this though I do have a question that may or may not lead to another change. So, if @amadio is OK with it so am I.


In src/XrdHttp/XrdHttpReq.cc:

> @@ -1798,7 +1798,16 @@ XrdHttpReq::PostProcessChecksum(std::string &digest_header) {
     if (convert_to_base64) {free(digest_value);}
     return 0;
   } else {
-    prot->SendSimpleResp(500, NULL, NULL, "Underlying filesystem failed to calculate checksum.", 0, false);
+    // This is a result of the "Ugly" hack put in place to prevent a failing stat() on a
+    // non-manager server to fail a transfer.
+    // In the case a user issues a GET with a 'Want-Digest' header on a non-existing file, the non-failing
+    // stat() will result in a failed checksum query giving a 500 error back to the user.
+    // Here we just return a 404 instead. httpStatusCode and httpStatusText contain the right reason why the request failed.
+    if(xrderrcode != kXR_NotFound) {

I'm OK with the way it is but you could be a bit more discerning here. For instance, if the error is kXR_Unsupported then the status code should really be 501 with the text as returned by the server. Which leads to the question why override the server supplied error text with a generic one? Is the original error message lost somewhere? One would expect that the server's error message will better explain how to correct the problem.


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <xrootd/xrootd/pull/2019/review/1451649108@github.com>

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/xrootd/xrootd/pull/2019#pullrequestreview-1451649108", "url": "https://github.com/xrootd/xrootd/pull/2019#pullrequestreview-1451649108", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1