Skip to content

http response from google may not always has 'x-goog-hash' header #423

Description

@teddybearz

The http response from google may not always has 'x-goog-hash' header. When it happens, the node process will exit.

Need fix like:

diff --git a/node_modules/gcloud/lib/storage/file.js b/node_modules/gcloud/lib/storage/file.js
index 8e1a26e..4aaec72 100644
--- a/node_modules/gcloud/lib/storage/file.js
+++ b/node_modules/gcloud/lib/storage/file.js
@@ -361,10 +361,12 @@
             var md5Fail = true;

             var hashes = {};
-            res.headers['x-goog-hash'].split(',').forEach(function(hash) {
-              var hashType = hash.split('=')[0];
-              hashes[hashType] = hash.substr(hash.indexOf('=') + 1);
-            });
+            if (res.headers && res.headers['x-goog-hash']) {
+              res.headers['x-goog-hash'].split(',').forEach(function(hash) {
+                var hashType = hash.split('=')[0];
+                hashes[hashType] = hash.substr(hash.indexOf('=') + 1);
+              });
+            }

             var remoteMd5 = hashes.md5;
             var remoteCrc = hashes.crc32c && hashes.crc32c.substr(4);

Activity

  1. stephenplusplus commented on Mar 5, 2015

    @stephenplusplus
    Contributor

    Did you run into a situation where it wasn't returned?

    Google Cloud Storage stores MD5 hashes for all non-composite objects. CRC32Cs are available for all objects.

  2. ryanseys commented on Mar 5, 2015

    @ryanseys
    Contributor

    Seems like a reasonable request for a sanity check.

  3. stephenplusplus commented on Mar 5, 2015

    @stephenplusplus
    Contributor

    Still would like to know if this can come up. Would affect more of our code, if we can't always trust that a CRC32C hash is available.

  4. teddybearz commented on Mar 5, 2015

    @teddybearz
    Author

    It happens reliably if you try to fetch an object right before the upload of the object is done.

  5. ryanseys commented on Mar 10, 2015

    @ryanseys
    Contributor

    Why would you try to fetch an object before it was uploaded? Can you provide a snippet of code that causes this issue?

  6. teddybearz commented on Mar 10, 2015

    @teddybearz
    Author

    It was not intentionally. The bug was triggered because of in my old code, I registered on the "finish" event instead of "complete" event. A normal writeablestream will emit 'finish' when the data has been flushed to the underlying system, apparently 'finish' and 'complete' have different semantic in your lib. 'finish' event doesn't mean the object is committed. There is no document about this 'complete" event and I have to dig into the lib code to find out that.

    Nevertheless, the lib's robustness should not entirely depend on the well-behave of the backend service. A not-well-behaved service should not cause the lib user's process exits.

  7. stephenplusplus commented on Mar 10, 2015

    @stephenplusplus
    Contributor

    Regarding finish vs complete, it's just kind of hard to work with Node streams. You'll see various libraries using different terms for different reasons. We use complete to be consistent with the request library, which is a familiar term for many developers. That at least makes the transition a little less painful for most.

    Regarding the header, it should still always be there. I don't think we need to defensively program against that, since that the code pasted in your initial post executes after we've received a response from an upload. I'm still uncertain how our library executed that code before the response came back. Code that you used showing that happening may help me understand better.

  8. teddybearz commented on Mar 10, 2015

    @teddybearz
    Author

    To reproduce that you just need to fetch the object right after get the 'finish' event during a stream upload.

    fsStream.pipe(bucket.file(blobname).createWriteStream({ metadata: metadata} )).
    .on('error', function(err) {
    handleError(err, 'write');
    })
    .on('finish', function(ex) {
    if (!fdCbCalled) {
    fdCbCalled = true;
    future.return();
    }
    });
    future.wait();
    fetchBlob(blobName);

    50% of the time you will get a response without the 'x-goog-hash' header the lib code expects.

  9. stephenplusplus commented on Mar 11, 2015

    @stephenplusplus
    Contributor

    We merged a PR that will hopefully help with explaining we use complete and not finish. I believe that was one part of this issue, and the other was: in parallel, upload a file and read from it. @ryanseys did some testing and observed the API responds with a 404 error, however, our code doesn't account for this and continues on as if it's a successful response.

    Specifically, we need to implement util.handleResp here to help determine the quality of the API response.

    Thanks for catching this @teddybearz and for tracking down the problem @ryanseys! PR incoming :)

  10. added a commit that references this issue on Sep 15, 2022
  11. 22 remaining items

  12. added a commit that references this issue on Jan 28, 2026
  13. added a commit that references this issue on Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

🚨This issue needs some love.api: storageIssues related to the Cloud Storage API.triage meI really want to be triaged.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions