fix: validate action of getSignedUrl() function

[AVaksman]

Fixes #671

• Tests and linter pass
• Code coverage does not decrease

add validation for action parameter.

[codecov]

Codecov Report

Merging #684 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #684      +/-   ##
==========================================
+ Coverage   98.04%   98.04%   +<.01%     
==========================================
  Files           9        9              
  Lines         921      922       +1     
  Branches      100      100              
==========================================
+ Hits          903      904       +1     
  Misses          9        9              
  Partials        9        9

Impacted Files	Coverage Δ
src/file.ts	98.8% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b40e6a...7d27223. Read the comment docs.

[bcoe]

What was the behavior of the old API, if I were to pass in an invalid action? This looks great, my only concern is it might be a breaking change:

• if the old logic would have failed with a more cryptic error, this seems reasonable as a non-breaking change.
• if prior to this we would have succeeded, this feels like a breaking change.

Assuming we would have previously been throwing an exception, I'm 👍 on this change.

[bcoe]

This looks reasonable to me, might be worth considering using an Enum for TypeScript, does this translate cleanly to the JavaScript API?

[stephenplusplus]

There's actually already this enum:

nodejs-storage/src/file.ts

Line 2341 in 7b1ef2c

const method = ActionToHTTPMethod[cfg.action];

We could just validate that method is something.

[AVaksman]

Thanks, done

[AVaksman]

What was the behavior of the old API, if I were to pass in an invalid action? This looks great, my only concern is it might be a breaking change:

• if the old logic would have failed with a more cryptic error, this seems reasonable as a non-breaking change.
• if prior to this we would have succeeded, this feels like a breaking change.

Assuming we would have previously been throwing an exception, I'm 👍 on this change.

Previously getSignedUrl with invalid action parameter would still successfully return an invalid URL (a URL with permissions for non existing action so to speak) and any request via this URL would result with SignatureDoesNotMatch response. The error is cryptic since it points user in completely different direction.

[bcoe]

@AVaksman feels like a fix to me, I continue to be a 👍