benjreinhart / react-native-aws3

Pure JavaScript React Native library for uploading to AWS S3
MIT License
399 stars 151 forks source link

Allow for time sway #24

Closed chadwilken closed 7 years ago

chadwilken commented 7 years ago

Some users for various reasons may have their device's time set to a different time than the world clocks time. Normally this would get blocked by AWS unless you specify the offset in the policy expiration and x-aws-date header. This PR allows you to pass the timeDelta option to allow device time to sway.

benjreinhart commented 7 years ago

Sweet... looks good to me, but I'll have to pull this down and check it out later today.

Also, do you mind removing the version bump? I'll probably do that in a separate commit.

chadwilken commented 7 years ago

Removed the version bump. If you need anything else, let me know.

chadwilken commented 7 years ago

@benjreinhart have you had a change to test this? Would love to get back to your npm package.

benjreinhart commented 7 years ago

Hey @chadwilken,

Looking back on this, do you mind explaining to me how this helps your situation with users' clocks being different than 'the world clock's time?'

This PR only affects the expiration date sent to Amazon, not the x-amz-date field in the policy. Also, it appears only to shrink the timeframe that is allowable by amazon by setting the expiration date to a time closer to the present (unless you passed negative values in the timeDelta option).

For instance, before this PR, we were taking the current date and adding 5 minutes to it to derive our expiration date, thereby saying 'the request will expire in 5 minutes from now.' After this PR, we do the same thing, except now we're also subtracting the timeDelta option from the 5 minutes, thereby saying 'the request will expire in 5 minutes from now minus the timeDelta option's value (or 0).' So if we gave a value of 60*1000 (1 minute in MS) for the timeDelta option, we'd effectively be saying 'this request expires in 5 minutes minus 1 minute from now.' Our expiration window would be shorter at this point. I'm not sure how this solves the offset from world clock issue you were talking about above.

Do you mind showing me an example of how you're using this option and how it solves your problem?

benjreinhart commented 7 years ago

If you are passing negative values as the offset in order to expand the expiration window, we should probably rephrase this or add some other option to configure the expiration because otherwise the source is a bit confusing (to me anyways). I'd be happy to help sort that out.

chadwilken commented 7 years ago

I am going off of memory here but from what I remember you want to modify the expiration date because it was using 5 minutes from the devices time. If the device's time is in the past more than 5 minutes then the expiration date will have already lapsed.

The code I am using is:

const deviceTime = Math.round(new Date().getTime() / 1000);
        const serverTime = response.current_time;

        dispatch({
          type: DEVICE_TIME_SWAY_CHANGED,
          timeDelta: (deviceTime - serverTime) * 1000,
        });

Where response.current_time is the current UTC time. Then we pass this number into RNS3.put. Some of our users have their phone set in the future and some in the past (for reasons unknown).