Closed Bug 1116227 Opened 10 years ago Closed 10 years ago

String shims are defined incorrectly

Categories

(Calendar :: Provider: GData, defect)

Other
Other
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
4.0.0.1

People

(Reporter: jon, Assigned: Fallen)

References

Details

Attachments

(2 files, 1 obsolete file)

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)
Attached file Testing XPI for this bug β€”
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)
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.
(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
Closed: 10 years ago
Resolution: --- → INVALID
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: nobody → philipp
Status: REOPENED → ASSIGNED
Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
This should take care.
Attachment #8551744 - Flags: review?(mohit.kanwal)
Attached patch Fix - v1 β€” β€” Splinter Review
Attachment #8551744 - Attachment is obsolete: true
Attachment #8551744 - Flags: review?(mohit.kanwal)
Attachment #8552417 - Flags: review?(matthew.mecca)
Comment on attachment 8552417 [details] [diff] [review] Fix - v1 Looks good, r=mmecca
Attachment #8552417 - Flags: review?(matthew.mecca) → review+
Target Milestone: --- → 4.0
Status: ASSIGNED → RESOLVED
Closed: 10 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: