Skip to content

base64::encode API incompatibility with arduino-esp32 #6604

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

Closed
dirkmueller opened this issue Oct 5, 2019 · 2 comments · Fixed by #7910
Closed

base64::encode API incompatibility with arduino-esp32 #6604

dirkmueller opened this issue Oct 5, 2019 · 2 comments · Fixed by #7910

Comments

@dirkmueller
Copy link
Contributor

There is an unfortunate behavior difference between ESP32's base64::encode and ESP8266 base64::encode. The signature of ESP8266 is:

        // is to add a newline every 72 (encoded) characters output.
        // This may 'break' longer uris and json variables
        String encode(const uint8_t * data, size_t length, bool doNewLines = true);

in esp32, the signature is:

static String encode(const uint8_t * data, size_t length);

and it doesn't introduce newlines. This is actually a much nicer behavior because the
primary purpose of these functions (which can only deal with small input sizes as they
don't implement a Stream interface) seems to be JSON APIs and http authentication headers.

Both must not have newlines. The only case where you want newlines is PEM (which
is already provided by the embedded BearSSL, so you can just use that api, which is
both better performing and more convenient) and mime64 encoding (which I think you won't be able to do as you have no stream interface).

I suggest to align the signature (remove the capability to inject newlines), which reduces code size, or at least flip the default.

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 5, 2019

ref for discussion: #3208

@devyte
Copy link
Collaborator

devyte commented Oct 5, 2019

it's hard to guarantee that removal of newlines will not break applications which depend on them

That's the main reason this is still there.

Possibilities that come to mind:

  1. Remove the default value from the last arg in the current signature, add an overload without the last arg that forwards with doNewLines = false. This is a breaking change. Apps that use new lines and call without the 3rd arg will need to be updated with a 3rd arg set to true.
  2. Remove the doNewLines functionality altogether. This is a breaking change. Apps that use new lines won't be able to access the functionality anymore.
  3. Change the default value of doNewLines to false. This is a breaking change, similar to point 1. above.

In all three cases, the changes would need to be targeted for release 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants