If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

package directories with uppercase names cause the ID mechanism in CFX to fail

RESOLVED FIXED

Status

Add-on SDK
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: dbuc, Assigned: KWierso)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
Here is the output of the failure:

---

raise ValueError('invalid resource hostname: %s' % name)
ValueError: invalid resource hostname: jid0-bl4lyaykaevzios5naqsnyabajc-MailPing-lib

---

Even if we insist that lowercase names be used, we should still output a more descriptive error to the developer in the case they do this by mistake :)
Duplicate of this bug: 560442
Assignee: myk → nobody
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
Version: Trunk → unspecified
(Assignee)

Comment 3

7 years ago
Created attachment 518306 [details] [diff] [review]
warn user better

I have no clue what I'm doing in Python, but this looks like it might work for giving the user a more specific error. 

Haven't really tested it, as it's 2:30 in the morning and I'm due for a nap.
Attachment #518306 - Flags: feedback?(dietrich)
(Assignee)

Comment 4

7 years ago
Alternatively, would it work if line 69 in the original file was changed from this:
if not RESOURCE_HOSTNAME_RE.match(name):

to this:
if not RESOURCE_HOSTNAME_RE.match(name.lower()):

If it's just that RESOURCE_HOSTNAME_RE is all in lowercase so that it doesn't match the case of name in python (but everything else would still work), this would get around a case-mismatch throwing that error at all. I think.
Comment on attachment 518306 [details] [diff] [review]
warn user better

canceling review. per your subsequent comment, can you see if that fixes it? if it appears more complex than that, we can take this instead.
Attachment #518306 - Flags: feedback?(dietrich)
(Assignee)

Comment 6

7 years ago
Seems that comment 4's idea might work. (Or at least, with that change, I'm not seeing any errors thrown after creating a package using cfx init inside of a folder containing called "UPPERCASE", then using cfx run to test it.)
Don't have time to do any more thorough testing than that, though.
(Assignee)

Comment 7

7 years ago
Created attachment 519413 [details] [diff] [review]
warning that actually works

On a second look, it seems cfx run was using a 3.6 binary, so nothing was happening at all when it loaded Firefox, thus no errors. Oops.

Passing it a 4.0 binary, I get the following exception thrown when I try comment 4's change and a mixed case name:
[Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIIOService.newChannelFromURI]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: resource://jid0-hyn5caoq3naioibtljcs1kee6jy-api-utils-lib/securable-module.js :: resolveModule :: line 585"  data: no] (undefined:585)
FAIL



And trying to add some logging around line 585 in securable-module to figure out what it's doing makes cfx run throw the following exception:
[Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm -> file:///tmp/tmpP4doNR.mozrunner/extensions/xulapp@toolness.com/bootstrap.js -> file:///tmp/tmpP4doNR.mozrunner/extensions/xulapp@toolness.com/components/harness.js :: buildLoader :: line 180"  data: no] (undefined:180)
FAIL



And trying out the patch in comment 3, I get the following:
Traceback (most recent call last):
  File "/home/kwierso/Desktop/addon-sdk-1.0b3/bin/cfx", line 29, in <module>
    cuddlefish.run()
  File "/home/kwierso/Desktop/addon-sdk-1.0b3/python-lib/cuddlefish/__init__.py", line 598, in run
    include_dep_tests=options.dep_tests
  File "/home/kwierso/Desktop/addon-sdk-1.0b3/python-lib/cuddlefish/packaging.py", line 219, in generate_build_for_target
    validate_resource_hostname(prefix)
  File "/home/kwierso/Desktop/addon-sdk-1.0b3/python-lib/cuddlefish/packaging.py", line 68, in validate_resource_hostname
    if not name.match(name.lower()):
AttributeError: 'unicode' object has no attribute 'match'





Comment 4's idea clearly would take more than a simple .lower() to work, so here's a new version of comment 3's patch that actually works as intended.
Attachment #518306 - Attachment is obsolete: true
Attachment #519413 - Flags: review?(dietrich)
Comment on attachment 519413 [details] [diff] [review]
warning that actually works

r=me, thanks!
Attachment #519413 - Flags: review?(dietrich) → review+
Keywords: checkin-needed
https://github.com/mozilla/addon-sdk/commit/497828c513eb422e92c7d58e84148c556cd82cde
Assignee: nobody → kwierso
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Duplicate of this bug: 616844
You need to log in before you can comment on or make changes to this bug.