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
crypto: Remove redundant internal struct x509.rsaPublicKey and replace references with rsa.PublicKey #19355
Comments
I don't think it does any harm keep it as-is. In fact, it's defensive: it's documented as being the asn.1 structure, whereas the other one could have fields added or reordered safely and is the one for users. |
Perhaps true. I had thought that its use was somewhat idiosyncratic, but it appears to be used only twice, in calls to asn1.Marshal and asn1.Unmarshal. So perhaps moving it into the asn1 package and possibly renaming it would better indicate its intended use? |
We don't put the http2 encoder and decoder in the bytes package just because it's eventually encoded as bytes. I don't see why the asn1 package should contain anything about RSA or x509 or anything else that uses asn.1. The layering seems good as it is. Closing, but feel free to argue otherwise. |
Good point RE not including higher-level structs in codec packages. Wasn't thinking clearly on that. I'd suggest finally then that the struct in question be renamed One of the reasons this was confusing to me was probably that it was just hanging out all alone down there at the bottom of the file. Not going to argue too hard for this though. Handle as you see fit and thanks for taking the time to discuss. Easy enough for me to submit the CL if so desired. |
CL https://golang.org/cl/37885 mentions this issue. |
For consistency with the other named types in this package, this change renames the unexported rsaPublicKey struct to pkcs1PublicKey and positions the declaration up with the other similarly-named types in pkcs1.go. See the final comment of #19355 for discussion. Change-Id: I1fa0366a8efa01602b81bc69287ef747abce84f5 Reviewed-on: https://go-review.googlesource.com/37885 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
Just proposing a simple change to remove the apparently redundant
x509.rsaPublicKey
internal struct that's used in two places, preventing future accidental use.I have a simple patch to submit and all tests pass.
The text was updated successfully, but these errors were encountered: