String shims are defined incorrectly



4 years ago
4 years ago


(Reporter: jon, Assigned: Fallen)





(2 attachments, 1 obsolete attachment)



4 years ago
User Agent: Mozilla/5.0 (X11; CrOS armv7l 6310.68.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.96 Safari/537.36

Steps to reproduce:

Installed Provider for Google Calendar and tried to run the code on this page as detailed below:

var str = "hello world";
var converter =

// we use UTF-8 here, you can choose other encodings.
converter.charset = "UTF-8";
// result is an out parameter,
// result.value will contain the array length
var result = {};
// data is an array of bytes
var data = converter.convertToByteArray(str, result);
var ch = Components.classes[";1"]
ch.update(data, data.length);
var hash = ch.finish(false);

// return the two-digit hexadecimal code for a byte
function toHexString(charCode)
  return ("0" + charCode.toString(16)).slice(-2);

// convert the binary hash data to a hex string.
var s = [toHexString(hash.charCodeAt(i)) for (i in hash)].join("");
// s now contains your hash in hex: should be
// 5eb63bbbe01eeed093cb22bb8f5acdc3

Actual results:

The hash variable returned from ch.finish had a length property of 16, but the lower code section containing for (i in hash) looped 17 times, meaning the eventual md5sum was 34 chars in length not 32

Expected results:

The loop should have iterated 16 times returning a 32 char md5sum. Uninstalling or downgrading the Provider for Google Calendar extension fixes the problem.

Comment 1

4 years ago
Does the provider for google calendar replace nsICryptoHash with a new implementation?

Have you debugged this at all to see why this is happening? Without debugging, it's not clear that this is a Mozilla bug instead of just a bug in the extension somehow.
Flags: needinfo?(jon)

Comment 2

4 years ago
Created attachment 8544191 [details]
Testing XPI for this bug

Comment 3

4 years ago
I've created a small xpi that just runs the test md5 code (attached to this bug). If you run it with the Provider installed, the md5sum generated is 34 chars long. Remove the Provider extension and the md5sum is the expected length.
Flags: needinfo?(jon)

Comment 4

4 years ago
Sorry forgot to answer your questions. The author of the Provider extension responded to a forum post here:!topic/provider-for-google-calendar/Rm2F0UtV3xU - it seems the extension does not make use of nsiCryptoHash.

Version 1.0.2 of the Provider extension does not provoke the bug. I ran a recursive diff on the versions but I've not come across anything that would obviously cause this behaviour.
(I'm the author by the way)

I do not make any changes to nsICryptoHash in the Provider extension, nor do I even use it. Between 1.0.2 and 1.0.3 I made a lot of changes to support older gecko versions, mostly shimming JS modules. I don't do any xpcom registrations though. If you want to take a brief look at the shims, they are all here:
The last property is 'includes'. Commenting out fixes the issue.

Run this with remote debugging to see it in the console:
for (var i in "abcdefghijklmnop") console.log("character: " + i)

Comment 7

4 years ago
Great thanks. This is a bug in the extension, which should be using .defineProperty to create a non-enumerable expando on String.
Last Resolved: 4 years ago
Resolution: --- → INVALID
Thanks for the analysis! Moving this bug back to calendar land to fix it.
Component: XPCOM → Provider: GData
Ever confirmed: true
Product: Core → Calendar
Resolution: INVALID → ---
Summary: Installation of Provider for Google Calendar extension v 1.0.3 breaks nsiCryptoHash md5 generation → String shims are defined incorrectly
Version: 31 Branch → unspecified
Assignee: nobody → philipp
Created attachment 8551744 [details] [diff] [review]
Fix - v1

This should take care.
Attachment #8551744 - Flags: review?(mohit.kanwal)
Created attachment 8552417 [details] [diff] [review]
Fix - v1
Attachment #8551744 - Attachment is obsolete: true
Attachment #8551744 - Flags: review?(mohit.kanwal)
Attachment #8552417 - Flags: review?(matthew.mecca)
Blocks: 1117541
Comment on attachment 8552417 [details] [diff] [review]
Fix - v1

Looks good, r=mmecca
Attachment #8552417 - Flags: review?(matthew.mecca) → review+
Keywords: checkin-needed
Target Milestone: --- → 4.0
Pushed to comm-central changeset c785d6f3f52a
Last Resolved: 4 years ago4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.