769521 - Add Base64-URL-encoding and Base64-URL-decoding to XPCOM

Status: RESOLVED WORKSFORME

(Core :: XPCOM, enhancement)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

+++ This bug was initially created as a clone of Bug #769519 +++
+++ This bug was initially created as a clone of Bug #753238 +++

toolkit/identity needs the ability to base64-URL-encode a raw byte array of arbitrary length as a string. For testing it will probably be eventually useful to be able to base64-URL-decode too. toolkit/identity/IdentityCryptoService.cpp already contains a function to do base64-URL-encoding built on top of XPCOM's Base64 encoder; we should move that functionality to XPCOM so it is available for every module.

Note that Identity needs base64-url-encoding to be exposed to JS, so perhaps we should also expose the base64 functionality through Components.utils?

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

What's wrong with atob/btoa?

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #1)
> What's wrong with atob/btoa?

It must be callable from native code and it must be base64-URL-encoding, not regular base64.

Also, I don't understand the "Character out of range" problem mentioned in the MDN documentation for atob and btoa.

Comment 3

[JP Rosevear [:jpr]]

This seems to have carried the blocking basecamp forward - is it really blocking basecamp?

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Sorry, I should really stop using the clone feature of Bugzilla. It is not blocking Basecamp.

Comment 5

[Andrew Overholt [:overholt]]

basecamp-ing based on Brian's comment.  Thanks.

Comment 6

[Axel Nennker]

Is 
  https://mxr.mozilla.org/mozilla-central/source/xpcom/io/Base64.cpp#239
the best place for the new functions Base64UrlEncode and Base64UrlDecode?

Write PL_Base64UrlEncode and PL_Base64UrlDecode first?

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

There's no need to write a PL_Base64Url[En|De]code.  Just implement this in xpcom directly.

Comment 8

[David Dahl :ddahl]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #7)
> There's no need to write a PL_Base64Url[En|De]code.  Just implement this in
> xpcom directly.

I don't follow you. What does "implement directly in xpcom" mean here?

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Put the implementation in xpcom/io/Base64.h/cpp.  There's no need to implement functions in NSPR and then write wrappers around them in xpcom/.

Comment 10

[Axel Nennker]

Attachment: 4 new functions implementing base64url (en|de)coding

Four new functions to base64url encode and decode.

I don't understand how the base64 tests work. So this is untested. Have fun.
Axel

Comment 11

[David Dahl :ddahl]

(In reply to Axel Nennker from comment #10)
> Created attachment 658446 [details] [diff] [review]
> 4 new functions implementing base64url (en|de)coding
> 
> Four new functions to base64url encode and decode.
> 
> I don't understand how the base64 tests work. So this is untested. Have fun.
> Axel

This patch may not get review without a test. Apparently, khuey is the test author. khuey: Do we need a new test function that consumes base64url-encoded strings?  http://mxr.mozilla.org/mozilla-central/source/xpcom/tests/TestBase64.cpp ?

Comment 12

[Axel Nennker]

xpcom/tests/TestBase64.cpp tests the base64 stream encoder only.
I think that TestBase64.cpp is not the right place to test base64url.

Maybe we should add base64url encoding and decoding to
https://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#617 and test it there?

nsresult
nsContentUtils::Base64UrlEncode(const nsAString& aBinaryData,
                     nsAString& aAsciiBase64String)
{
  if (!Is8bit(aBinaryData)) {
    aAsciiBase64String.Truncate();
    return NS_ERROR_DOM_INVALID_CHARACTER_ERR;
  }

  return Base64UrlEncode(aBinaryData, aAsciiBase64String);
}

nsresult
nsContentUtils::Base64UrlDecode(const nsAString& aAsciiBase64String,
                     nsAString& aBinaryData)
{
  if (!Is8bit(aAsciiBase64String)) {
    aBinaryData.Truncate();
    return NS_ERROR_DOM_INVALID_CHARACTER_ERR;
  }

  nsresult rv = Base64UrlDecode(aAsciiBase64String, aBinaryData);
  if (NS_FAILED(rv) && rv == NS_ERROR_INVALID_ARG) {
    return NS_ERROR_DOM_INVALID_CHARACTER_ERR;
  }
  return rv;
}

It might make sense to prefix the function first like...
mozBase64UrlEncode

Comment 13

[Martin Thomson [:mt:]]

An encoder function was added in https://hg.mozilla.org/mozilla-central/rev/dda92c46bb23 so we are halfway there.

Comment 14

[Nathan Froyd [:froydnj]]

(In reply to Martin Thomson [:mt:] from comment #13)
> An encoder function was added in
> https://hg.mozilla.org/mozilla-central/rev/dda92c46bb23 so we are halfway
> there.

And then the decoder function was added in bug 1256488, so I think we are done here.