fix(cloudfront-signer): url and policy shouldn't be passed together

[nayabatir1]

Issue

issue "#5836" related #4516

Description

In getSignedCookies or getSignedUrl url and policy shouldn't passed together. As in policy url provided.

Testing

Test cases updated and tested locally

Additional context

Checklist

• [x] If you wrote E2E tests, are they resilient to concurrent I/O?
• [x] If adding new public functions, did you add the @public tag and enable doc generation on the package?

---

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

[NamesMT]

IMO the url shouldn't be never and should not always be overwritten if (policy), in some current complex use cases (my company internal included), the signedUrl could contain a policy that allows a flexible (wildcard) resource target, and we still could pass in the url param to get what we want as an initial URL, i.e ignoring url when used with policy would be a breaking change.

[nayabatir1]

Policy also has url, I'm extracting url from policy if it exists. You don't have to provide url as an additional param.

I think setting different initial url is completely business requirement.

[NamesMT]

I know the policy has url (Resource), but there is cases where the extracted Resource would introduce a breaking change, assuming the policy's Resource is: https://example.org/subFolder/*, currently we're using the url to set the initial URL to be https://example.org/subFolder/mainFile.js, by migrating this PR that would broke our app. I do like your implementation. and the initial URL can very easily be changed by the user's requirement but breaking change to the SDK isn't good.

There's also another issue that would throw an error internally at new URL(), with wildcard protocol Resource: *://example.com/file.ext

[nayabatir1]

As per AWS doc protocal must be http:// or https://. It also mentions that url, DateLessThan, DateGreaterThan, IpAddress can be provided in policy. It no where mentions to pass url along with policy.

Incase this is merged existing version won't be modified. Same version won't be breaking for you.

Let see what maintainers have to say about this.

[NamesMT]

That's fun, the AWS doc for signed URL do allow *://, and one of our service is using this protocol, however the service is using AWS-SDK v2:

Incase this is merged existing version won't be modified. Same version won't be breaking for you.

Would it not be modified?, I thought the aws-sdk version on Lambda would be up-to-date, still, if the SDK version on Lambda do follows the lockfile, a not thoroughly tested dependencies update would then break things.

BTW I added you as co-author on new some updates in #5926

[github-actions[bot]]

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.