Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

makePrivate not working? #763

Closed
MKHenson opened this issue Jul 30, 2015 · 6 comments
Closed

makePrivate not working? #763

MKHenson opened this issue Jul 30, 2015 · 6 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API.

Comments

@MKHenson
Copy link

Hi guys

I think I might have stumbled on a bug. I am running a nodejs server using the gcloud version 0.16.0. When I upload a file and access said file through the URL:

https://storage.googleapis.com/bucketID/fileID

I get a 403 status saying I dont have permission. Which is correct. So in node, I call

file.makePublic(function(err, api){})

and when I access the URL now it gives me a 200 code - so far so good.

If however I want to revert the file back to private:

file.makePrivate(function(err){}) 

The function goes through fine - there are no errors - however the object remains publicly available. The http code is still 200. Is this a bug or have I done something wrong here?

@stephenplusplus
Copy link
Contributor

Thanks for reporting!

I get a 302 status saying I dont have permission.

Are you sure it's not a 403?

I've been getting the expected results with the following code:

var file = bucket.file('file.txt');

var writeStream = file.createWriteStream();
writeStream.end('hi');

writeStream.on('complete', function() {
  // 403
  file.makePublic(function() {
    // 200
    file.makePrivate(function() {
      // 403
    });
  });
});

We also have some tests that seem to cover the case well: https://github.com/GoogleCloudPlatform/gcloud-node/blob/e4fdb2d118ba5098731ccdff74d75c5aa2f69908/system-test/storage.js#L358-382

Pinging @jgeewax in case he can think of anything.

@stephenplusplus stephenplusplus added the api: storage Issues related to the Cloud Storage API. label Jul 30, 2015
@MKHenson
Copy link
Author

Hi stephenplusplus, sorry yes I meant 403. Weird... I pretty much have the exact same code. Gonna see if I can re-create it some other way

@MKHenson
Copy link
Author

It seems that the file does eventually become private - but takes some time before the change is propagated. In my code I have 2 functions that are called to make the file public and private:

// In the make private function
var bucket = that._gcs.bucket(file.bucketId);
var rawFile = bucket.file(file.identifier);
rawFile.makePrivate({ strict: true }, function (err)
{
    if (err)
        return reject(err);

    // other code here...
});


// In the make pubic function
var bucket = that._gcs.bucket(file.bucketId);
var rawFile = bucket.file(file.identifier);
rawFile.makePublic( function (err, api)
{
    if (err)
        return reject(err);

    // other code here...
});

The file is just a basic image which was uploaded like so

var stream = rawFile.createWriteStream();

// Pipe the file to the bucket
stream.on("error", function (err: Error)
{
      // reject upload

}).on('complete', function ()
{
      // Updates to DB
})

So is it perhaps just that it takes time before the changes are aparent? (I was testing the raw file in chrome with cache disabled)

@jgeewax
Copy link
Contributor

jgeewax commented Jul 30, 2015

It's definitely possible that the ACLs do not propagate immediately. If that's the case, you'd have some lag after the makePrivate() call and getting the expected 403.

@stephenplusplus
Copy link
Contributor

I've been playing with this for a bit, and at first it seemed to work (when I made my first comment here), but now I'm struggling:

var file = bucket.file('file.txt');
var url = 'https://storage.googleapis.com/' + bucket.name + '/' + file.name;

var writeStream = file.createWriteStream();
writeStream.end('hi');

writeStream.on('complete', function() {
  async.series([
    confirmFileIsPrivate,

    wait(1500),
    file.makePublic.bind(file),
    confirmFileIsPublic,

    wait(1500),
    file.makePrivate.bind(file),

    wait(1500),
    confirmFileIsPrivate // fails. statusCode is still 200
  ], function(err) {
    assert.ifError(err);
    file.delete(done);
  });
});

function wait(ms) {
  return function(next) {
    setTimeout(next, ms);
  };
}

function confirmFileIsPublic(callback) {
  request(url, function(err, resp) {
    assert.ifError(err);
    assert.strictEqual(resp.statusCode, 200);
    callback();
  });
}

function confirmFileIsPrivate(callback) {
  request(url, function(err, resp) {
    assert.ifError(err);
    assert.strictEqual(resp.statusCode, 403);
    callback();
  });
}

I've tried different delays as well as confirming the statusCode with curl.

FWIW, at one point during testing, I was convinced it was a backend race condition of sorts, where the first request to make public or private might still be processing while the second request is made. The API returns 200 for both requests, but then it's a race to see which request completes first. Anyway, I haven't been able to reproduce any success conditions now, so I'm not sure what the issue might be.

If anyone wants to play around with the test code above, please feel free and let me know if you can plug in the right variables to make it work, or catch something wrong in how I'm testing.

@stephenplusplus
Copy link
Contributor

My research was hitting anomalies thanks to getting a cached response from the storage server. This updated code succeeds:

var file = bucket.file('file.txt.' + Date.now());

var writeStream = file.createWriteStream();
writeStream.end('hi');

writeStream.on('finish', function() {
  async.series([
    confirmStatus(403),

    file.makePublic.bind(file),
    confirmStatus(200),

    file.makePrivate.bind(file),
    confirmStatus(403)
  ], function(err) {
    assert.ifError(err);
    file.delete(console.log);
  });
});

function confirmStatus(expectedStatus) {
  return function(next) {
    request({
      uri: 'https://storage.googleapis.com/' + bucket.name + '/' + file.name,
      qs: {
        cacheBuster: Date.now()
      }
    }, function(err, resp) {
      if (err || resp.statusCode !== expectedStatus) {
        next(err || new Error('Got status code: ' + resp.statusCode));
        return;
      }

      next();
    });
  };
}

Based on your last comment @MKHenson, I think this is resolved-- the backend can just take a bit to process sometimes? I'm going to close but feel to re-open if I'm mistaken.

sofisl added a commit that referenced this issue Nov 9, 2022
* chore(main): release 4.1.1

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
sofisl pushed a commit that referenced this issue Jan 17, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [null-loader](https://github.com/webpack-contrib/null-loader) | devDependencies | major | [`^3.0.0` -> `^4.0.0`](https://renovatebot.com/diffs/npm/null-loader/3.0.0/4.0.0) |

---

### Release Notes

<details>
<summary>webpack-contrib/null-loader</summary>

### [`v4.0.0`](https://github.com/webpack-contrib/null-loader/blob/master/CHANGELOG.md#&#8203;400-httpsgithubcomwebpack-contribnull-loadercomparev300v400-2020-04-15)

[Compare Source](https://github.com/webpack-contrib/null-loader/compare/v3.0.0...v4.0.0)

##### Bug Fixes

-   support `webpack@5`

##### ⚠ BREAKING CHANGES

-   minimum required Nodejs version is `10.13`

</details>

---

### Renovate configuration

:date: **Schedule**: "after 9am and before 3pm" (UTC).

:vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

:recycle: **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#googleapis/nodejs-vision).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API.
Projects
None yet
Development

No branches or pull requests

3 participants