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

Add back support for Go 1.14 #51

Closed
wants to merge 4 commits into from

Conversation

zeripath
Copy link

@zeripath zeripath commented Aug 1, 2021

Adds simple reimplementation of fillBytesInt for go 1.14 support

Fix #50

Signed-off-by: Andrew Thornton art27@cantab.net

Adds simple reimplementation of fillBytesInt for go 1.14 support

Fix golang-jwt#50

Signed-off-by: Andrew Thornton <art27@cantab.net>
@mfridman
Copy link
Member

mfridman commented Aug 1, 2021

Can you please add go1.14 to workflow?

https://github.com/golang-jwt/jwt/blob/main/.github/workflows/build.yml#L16

@mfridman
Copy link
Member

mfridman commented Aug 1, 2021

Ah perfect, you added build tags

ecdsa_go1.14.go Outdated
if i >= 0 {
buf[i] = byte(d)
} else if byte(d) != 0 {
panic("math/big: buffer too small to fit value")
Copy link
Collaborator

@oxisto oxisto Aug 1, 2021

Choose a reason for hiding this comment

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

For me this looks like a 1:1 copy out of the Go std lib, even including the particular error here and the comment above. Not quite sure, if this ok from a license point of view. Even though it is BSD, it still needs proper attribution and the LICENSE file/header and unless you are the original author of that line, this is very fishy in my opinion to just copy/paste that here.

Copy link
Author

Choose a reason for hiding this comment

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

Adjusted.

Copy link
Member

Choose a reason for hiding this comment

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

I had the exact same thought (was just about to check if this was a copy/paste from the standard library).

I usually reference this ticket by Ian. golang/go#19893 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

The panic has to be exactly the same otherwise it's not compatible.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath closed this Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Due to the use of fillBytes golang-jwt now requires go1.15
3 participants