Closed
Bug 1116227
Opened 10 years ago
Closed 10 years ago
String shims are defined incorrectly
Categories
(Calendar :: Provider: GData, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
4.0.0.1
People
(Reporter: jon, Assigned: Fallen)
References
Details
Attachments
(2 files, 1 obsolete file)
3.67 KB,
application/zip
|
Details | |
2.71 KB,
patch
|
mmecca
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•10 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)
Reporter | ||
Comment 2•10 years ago
|
||
Reporter | ||
Comment 3•10 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•10 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•10 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/
![]() |
||
Comment 6•10 years ago
|
||
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)
Comment 7•10 years ago
|
||
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
Closed: 10 years ago
Resolution: --- → INVALID
Assignee | ||
Comment 8•10 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•10 years ago
|
Assignee: nobody → philipp
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 9•10 years ago
|
||
This should take care.
Attachment #8551744 -
Flags: review?(mohit.kanwal)
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8551744 -
Attachment is obsolete: true
Attachment #8551744 -
Flags: review?(mohit.kanwal)
Attachment #8552417 -
Flags: review?(matthew.mecca)
Comment 11•10 years ago
|
||
Comment on attachment 8552417 [details] [diff] [review]
Fix - v1
Looks good, r=mmecca
Attachment #8552417 -
Flags: review?(matthew.mecca) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → 4.0
Assignee | ||
Comment 12•10 years ago
|
||
Pushed to comm-central changeset c785d6f3f52a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•