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

Move the Base64 encode/decode functions from g/c/storage/internal -> g/c/internal #2796

Closed
devjgm opened this issue Jun 21, 2019 · 9 comments
Assignees
Labels
type: cleanup An internal cleanup or hygiene concern.

Comments

@devjgm
Copy link
Contributor

devjgm commented Jun 21, 2019

We need to use the Base64Encode() and Base64Decode() functions in google-cloud-cpp-spanner. It would be best to use these from google/cloud/common/ rather than reaching into google/cloud/storage/.

@devjgm devjgm added the type: cleanup An internal cleanup or hygiene concern. label Jun 21, 2019
@ghost
Copy link

ghost commented Jun 21, 2019

openssl_util.h currently includes google/cloud/storage/oauth2/credential_constants.h for storage::oauth2::JwtSigningAlgorithms. Should we:

  1. Move openssl_util.h over and factor out SignStringWithPem, or:

  2. Move only the Base64Encode() and Base64Decode() functions to their own file, and if so, what should this file be called?

@devjgm
Copy link
Contributor Author

devjgm commented Jun 21, 2019

Fwiw, the Base64 functions are all I need at this point.

@ghost
Copy link

ghost commented Jun 21, 2019

FYI I started a PR but ran into issues. google_cloud_cpp_common needs OpenSSL as a dependency, but doing that seems to cause problems elsewhere.

@devjgm devjgm changed the title Move the g/c/s/internal/openssl_util.h into g/c/common/internal Move the Base64 encode/decode functions from g/c/storage/internal -> g/c/internal Jun 21, 2019
@devjgm
Copy link
Contributor Author

devjgm commented Jun 21, 2019

Ack. That makes sense.

I renamed this issue to make it clear that we only need the base64 functions. The other SSL utilities are not needed (at least, not yet). Note that while we currently use an SSL library to implement the base64 encoding, we don't need to. See https://github.com/abseil/abseil-cpp/blob/master/absl/strings/escaping.cc#L453 for example.

So one solution might be to implement these functions w/o using SSL.

... Not that we actually want to do this. I'm not actually arguing for this, just that it would be one way to break a dep on SSL.

@ghost
Copy link

ghost commented Jun 21, 2019

Indeed, my CMake-fu is currently lacking. @coryan I am stumped. Do you have any ideas?

@coryan
Copy link
Contributor

coryan commented Jun 21, 2019

If this is not urgent (and I think it is not) I would like to wait for #2802. Implementing our own Base64 encoding/decoding is also a good option.

@devjgm
Copy link
Contributor Author

devjgm commented Jun 24, 2019

I filed googleapis/google-cloud-cpp-spanner#123 to clarify the need/urgency for this.TL;DR: We need base64 encode/decode functions to suport Spanner BYTES values, which would be needed for an Alpha launch.

@devbww
Copy link
Contributor

devbww commented Jul 26, 2019

FTR: googleapis/google-cloud-cpp-spanner#123 added standalone base64 encode/decode functions to the spanner repo.

@devjgm
Copy link
Contributor Author

devjgm commented Mar 27, 2020

This issue was closed in @devbww 's last comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants