dropbox / dropbox-sdk-js

The Official Dropbox API V2 SDK for Javascript
https://www.dropbox.com/developers
MIT License
942 stars 354 forks source link

V10.34.0 _utils.isBrowserEnv not working in all cases. #1134

Open spydmobile opened 8 months ago

spydmobile commented 8 months ago

Sorry but I cant reproduce this outside of my code yet,but i am reporting it anyway.

Using "dropbox": "^10.34.0", in Node 21, in javascript, on mac m2 studio on the backend only not in the client. I use the api perfectly fine until I have to reject an error back to the webUI, after which _utils.isBrowserEnv stops working correctly and causes:

/Users/xxx/localcode/dropbox_service/node_modules/dropbox/cjs/src/auth.js:58
      fetch = window.fetch.bind(window);
                           ^

So: this is the code from the current version of dropbox:

   if ((0, _utils.isBrowserEnv)()) {
      fetch = window.fetch.bind(window);
      crypto = window.crypto || window.msCrypto; // for IE11
    } else if ((0, _utils.isWorkerEnv)()) {
      /* eslint-disable no-restricted-globals */
      fetch = self.fetch.bind(self);
      crypto = self.crypto;
      /* eslint-enable no-restricted-globals */
    } else {
      fetch = require('node-fetch'); // eslint-disable-line global-require

      crypto = require('crypto'); // eslint-disable-line global-require
    }

after i reject and send my error to the client, this logic no longer works correctly, and mistakenly thinks its in the browser environment instead so tries to use browser fetch, from the browsers global window object, which does not exist in NodeJS.

so to fix this, I altered the dropbox code to this:

   if ((typeof window == 'undefined')&&(0, _utils.isBrowserEnv)()) {
      fetch = window.fetch.bind(window);
      crypto = window.crypto || window.msCrypto; // for IE11
    } else if ((0, _utils.isWorkerEnv)()) {
      /* eslint-disable no-restricted-globals */
      fetch = self.fetch.bind(self);
      crypto = self.crypto;
      /* eslint-enable no-restricted-globals */
    } else {
      fetch = require('node-fetch'); // eslint-disable-line global-require

      crypto = require('crypto'); // eslint-disable-line global-require
    }

I have added a safety check, so that the dropbox utility function "isBrowserEnv" which is failing to work properly, must pass, but also, the existence of the window object must pass before it can use the browser window version of fetch, so now it falls back to loading node-fetch and works correctly!!!!!!!

greg-db commented 8 months ago

Thanks for the report. I'll be happy to send this along to the team to see if they can look into if/how we can address this in the SDK, but I expect they'll want to be able to reproduce the issue to make sure they understand it and can address it properly. That being the case, can you share some steps to reproduce the issue when possible? Thanks!