The default bug view has changed. See this FAQ.

String shims are defined incorrectly

RESOLVED FIXED in 4.0.0.1

Status

Calendar
Provider: GData
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Jonathan Cutting, Assigned: Fallen)

Tracking

unspecified
4.0.0.1
Other
Other

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 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 https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsICryptoHash as detailed below:

var str = "hello world";
var converter =
  Components.classes["@mozilla.org/intl/scriptableunicodeconverter"].
    createInstance(Components.interfaces.nsIScriptableUnicodeConverter);

// 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["@mozilla.org/security/hash;1"]
                   .createInstance(Components.interfaces.nsICryptoHash);
ch.init(ch.MD5);
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.
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)
(Reporter)

Comment 2

2 years ago
Created attachment 8544191 [details]
Testing XPI for this bug
(Reporter)

Comment 3

2 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)
(Reporter)

Comment 4

2 years ago
Sorry forgot to answer your questions. The author of the Provider extension responded to a forum post here: https://groups.google.com/forum/#!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.
(Assignee)

Comment 5

2 years ago
(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: http://mxr.mozilla.org/comm-central/source/calendar/providers/gdata/modules/shim/
The last property is 'includes'. Commenting http://mxr.mozilla.org/comm-central/source/calendar/providers/gdata/modules/shim/Loader.jsm#70 out fixes the issue.

Run this with remote debugging to see it in the console:
for (var i in "abcdefghijklmnop") console.log("character: " + i)
Great thanks. This is a bug in the extension, which should be using .defineProperty to create a non-enumerable expando on String.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → INVALID
(Assignee)

Comment 8

2 years ago
Thanks for the analysis! Moving this bug back to calendar land to fix it.
Status: RESOLVED → REOPENED
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)

Updated

2 years ago
Assignee: nobody → philipp
Status: REOPENED → ASSIGNED
(Assignee)

Comment 9

2 years ago
Created attachment 8551744 [details] [diff] [review]
Fix - v1

This should take care.
Attachment #8551744 - Flags: review?(mohit.kanwal)
(Assignee)

Comment 10

2 years ago
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)
(Assignee)

Updated

2 years ago
Blocks: 1117541
Comment on attachment 8552417 [details] [diff] [review]
Fix - v1

Looks good, r=mmecca
Attachment #8552417 - Flags: review?(matthew.mecca) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(Assignee)

Updated

2 years ago
Target Milestone: --- → 4.0
(Assignee)

Comment 12

2 years ago
Pushed to comm-central changeset c785d6f3f52a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.