Skip to content

Conversation

smilkuri
Copy link
Contributor

@smilkuri smilkuri commented Mar 11, 2025

Issue

#6403

Description

This PR adds changes in the parseDate function to use date constructor instead of Date.parse() to support timestamps in milliseconds.

Testing

Locally by testing it with milliseconds and date format string inputs.


✓ src/sign.spec.ts (25)
  ✓ getSignedUrl (11)
    ✓ should maintain query params after signing a URL
    ✓ should include url path in policy of signed URL
    ✓ should sign a URL with a canned policy
    ✓ should sign a URL with a custom policy containing a start date
    ✓ should sign a URL with a custom policy containing an ip address
    ✓ should sign a URL with a custom policy containing a start date and ip address
    ✓ should allow an ip address with and without a mask
    ✓ should throw an error when the ip address is invalid
    ✓ should sign a RTMP URL
    ✓ should sign a URL with a policy provided by the user
    ✓ should sign a URL automatically extracted from a policy provided by the user
  ✓ getSignedCookies (8)
    ✓ should allow an ip address with and without a mask
    ✓ should throw an error when the ip address is invalid
    ✓ should be able sign cookies that contain a URL with wildcards
    ✓ should sign cookies with a canned policy
    ✓ should sign cookies with a custom policy containing a start date
    ✓ should sign cookies with a custom policy containing an ip address
    ✓ should sign cookies with a custom policy containing a start date and ip address
    ✓ should sign cookies with a policy provided by the user without a url
  ✓ getSignedUrl- when signing a URL with a date range (6)
    ✓ allows string input compatible with Date constructor
    ✓ allows number input in milliseconds compatible with Date constructor
    ✓ allows Date object input
    ✓ allows string input for date range
    ✓ allows number input for date range
    ✓ allows Date object input for date range

Test Files  1 passed (1)
     Tests  25 passed (25)
  Start at  14:58:42
  Duration  507ms (transform 133ms, setup 0ms, collect 135ms, tests 96ms, environment 0ms, prepare 87ms)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@smilkuri smilkuri requested a review from a team as a code owner March 11, 2025 17:38
@smilkuri smilkuri changed the title chore(cloudfront-signer): fix date parsing by changing to date constructor chore(cloudfront-signer): fix date parsing by using date constructor Mar 11, 2025
@smilkuri smilkuri changed the title chore(cloudfront-signer): fix date parsing by using date constructor fix(cloudfront-signer): fix date parsing by using date constructor Mar 11, 2025
const resultUrl = getSignedUrl({
url,
keyPairId,
dateLessThan: dateString,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are two ends to the date range options, right? this needs to test the other end too.

@smilkuri smilkuri force-pushed the cloudfront-signer-dateConstructor branch from 781e721 to 8c6ce7b Compare March 18, 2025 14:51
@smilkuri smilkuri force-pushed the cloudfront-signer-dateConstructor branch from 1f71486 to c4f8c7e Compare March 18, 2025 14:56
private parseDateWindow(expiration: string | number | Date, start?: string | number | Date): PolicyDates {
const dateLessThan = this.parseDate(expiration);
if (!dateLessThan) {
throw new Error("dateLessThan is invalid. Ensure the date string is compatible with the Date constructor.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should we change this message to "....Ensure the date value is compatible with the Date constructor." instead of the "date string"

@trivikr trivikr changed the title fix(cloudfront-signer): fix date parsing by using date constructor fix(cloudfront-signer): parse date using Date constructor Mar 20, 2025
@smilkuri smilkuri merged commit a07b850 into main Mar 20, 2025
2 of 3 checks passed
@smilkuri smilkuri deleted the cloudfront-signer-dateConstructor branch March 20, 2025 16:34
Copy link

github-actions bot commented Apr 4, 2025

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants