-
Notifications
You must be signed in to change notification settings - Fork 81
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
Experiment with worker redirection #606
base: main
Are you sure you want to change the base?
Conversation
ec59b61
to
afa8e96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are a few comments that we can address early. I didn't touch parts of the code that are not ready, like GetMediaById
.
panic(err) // it wouldn't have worked anyways | ||
} | ||
qs := parsedUrl.Query() | ||
qs.Set(key, val) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to url-encode val
or does Set()
do it for us?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set
does this for us
@@ -1,9 +1,58 @@ | |||
package _responses | |||
|
|||
import ( | |||
"crypto/hmac" | |||
"crypto/sha256" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for not using sha512 for the HMACs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read "use sha512" as "use sha256" for some reason - let's just use sha512.
This is not ready for usage, and makes a ton of assumptions. It's at best a proof of concept.