Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nevermind #6243

Closed
wants to merge 1 commit into from
Closed

nevermind #6243

wants to merge 1 commit into from

Conversation

andrew0
Copy link

@andrew0 andrew0 commented Feb 11, 2024

edit: nevermind, consider allowing people to sign CLA without a google account

The `constructSegmentUris` util already returns an array, so the call to `makeRequest` was being made with an array of arrays. This wasn't causing errors for requests that use the `fetch` plugin, since `fetch` will stringify the first argument if its an array. But the data URI plugin expects to receive a string and calls `.split()` on it, so keys using data URIs throw an error if the URI is wrapped in an array.
Copy link

google-cla bot commented Feb 11, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@andrew0 andrew0 closed this Feb 11, 2024
@andrew0 andrew0 changed the title fix: don't double wrap URIs for HLS key requests nevermind Feb 11, 2024
@andrew0 andrew0 deleted the patch-1 branch February 11, 2024 16:57
@joeyparrish
Copy link
Member

@andrew0, I don't have any control over Google's CLA policy. I respect your decision not to sign it, and I apologize for any inconvenience.

joeyparrish pushed a commit that referenced this pull request Feb 14, 2024
The `constructSegmentUris` util already returns an array, so the call to `makeRequest` was being made with an array of arrays. This wasn't causing errors for requests that use the `fetch` plugin, since `fetch` will stringify the first argument if it's an array. But the data URI plugin expects to receive a string and calls `.split()` on it, so keys using data URIs throw an error if the URI is wrapped in an array.

Thanks to #6243 and @andrew0
@andrew0
Copy link
Author

andrew0 commented Feb 14, 2024

@joeyparrish thanks, sorry if I came off a bit cranky, I just don't use google and didn't want to jump through a bunch of hoops for such a small change

@joeyparrish
Copy link
Member

It's all good. I hope you are okay with @avelad continuing your PR and crediting you with a mention in the commit.

@andrew0
Copy link
Author

andrew0 commented Feb 16, 2024

yup, totally fine 👍

joeyparrish pushed a commit that referenced this pull request Feb 20, 2024
The `constructSegmentUris` util already returns an array, so the call to `makeRequest` was being made with an array of arrays. This wasn't causing errors for requests that use the `fetch` plugin, since `fetch` will stringify the first argument if it's an array. But the data URI plugin expects to receive a string and calls `.split()` on it, so keys using data URIs throw an error if the URI is wrapped in an array.

Thanks to #6243 and @andrew0

Backported to v4.7.x
joeyparrish pushed a commit that referenced this pull request Feb 20, 2024
The `constructSegmentUris` util already returns an array, so the call to `makeRequest` was being made with an array of arrays. This wasn't causing errors for requests that use the `fetch` plugin, since `fetch` will stringify the first argument if it's an array. But the data URI plugin expects to receive a string and calls `.split()` on it, so keys using data URIs throw an error if the URI is wrapped in an array.

Thanks to #6243 and @andrew0

Backported to v4.7.x
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 11, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants