Closed Bug 1252831 Opened 9 years ago Closed 6 years ago

https://30boxes.com serves an expired certificate

Categories

(Firefox :: File Handling, defect)

Product:
Firefox
Component:
File Handling
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 67
Tracking Flags:
Tracking Status
relnote-firefox --- 67+
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed

People

(Reporter: florian, Assigned: Gijs)

References

Details

Attachments

(2 files)

1252831.patch
(deleted), patch
Dolske
: review+
Details | Diff | Splinter Review
Bug 1252831 - remove 30boxes webcal handler as both possible and default handler, r?paolo!,dolske!
(deleted), text/x-phabricator-request
Details
Bug 840699 changed the URL of the 30boxes.com integration to use https URLs, but the comments in that bug seem to indicate we made this change without contacting anybody at 30boxes. When testing this feature (web handler for webcal: ) today, I got an error page: 30boxes.com:443 uses an invalid security certificate. The certificate expired on 1 novembre 2014 19:07. The current time is 2 mars 2016 12:48. Error code: <a id="errorCode" title="SEC_ERROR_EXPIRED_CERTIFICATE">SEC_ERROR_EXPIRED_CERTIFICATE</a> I don't know more about this. Is this a temporary issue or something that has lasted for a while already? If we no longer have a relationship with 30boxes, should remove the integration? I'm cc'ing a few people in the hope someone will know what to do with this.
Sadly I have no idea when 30boxes was added, and the addition predates Mercurial.
(In reply to Francesco Lodolo [:flod] from comment #1) > Sadly I have no idea when 30boxes was added, and the addition predates > Mercurial. It was in 2008 in bug 395277 / bug 413630.
We should remove 30 boxes, not just because the https doesn't work, but because their login doesn't even use https: http://30boxes.com/login I don't think we should be pushing this service.
Attached patch 1252831.patch (deleted) — — Splinter Review
Attachment #8841728 - Flags: review?(dolske)
Comment on attachment 8841728 [details] [diff] [review] 1252831.patch Review of attachment 8841728 [details] [diff] [review]: ----------------------------------------------------------------- r+, but I think you also need increment the defaultHandlersVersion in region.properties. Please do that and verify that this actually results in webcal handler for an existing profile being purged. It doesn't look like we've ever removed a service before (just added and modified URLs), so let's make sure this actually works as one would hope. FTR, there's also some references to "welcal.0" here, but they're fine to keep. http://searchfox.org/mozilla-central/rev/9c1c7106eef137e3413fd867fc1ddfb1d3f6728c/browser/app/profile/firefox.js#733-741 Also, this only removed 30boxes.com from en-US builds. We'll also need to remove it from the various locales that reference it... https://dxr.mozilla.org/l10n-central/search?q=30box&redirect=false shows 187 references. Note that 2 locales have an additional handler, so their webcal.1 entries will need to change to webcal.0 https://dxr.mozilla.org/l10n-central/search?q=webcal.1&redirect=true I think these can be handled in a separate bug or as a quick followup here.
Attachment #8841728 - Flags: review?(dolske) → review+
Axel, can you help with the L10N changes? (Assuming Brian doesn't want to volunteer to do that too.)
Flags: needinfo?(l10n)
Two other notes: * I checked with mconnor as to if we care about this change from a partner perspective. It's fine to remove. * It occurs to me that the other handler for the ro/uk locales is Google Calendar, so in theory we could just change things from 30boxes to GCal. But let's not get caught up in that here. I can't find any bugs on file for making that work (although a Google search suggests some people do so manually). So given that there's been little interest in change this over the last decade, not worth the effort to think about it.
Assignee: nobody → geeknik
I'll fling the l10n parts over to flod, he's been taking care of those bits for a while now.
Flags: needinfo?(l10n) → needinfo?(francesco.lodolo)
(In reply to Justin Dolske [:Dolske] from comment #5) > I think these can be handled in a separate bug or as a quick followup here. The "quick followup" will require landing in 90x2 repositories, i.e. it won't be quick (I wish this was centralized like searchplugins). I will file a bug and take care of it at the beginning of the next Aurora cycle, but I would really like someone to confirm if we actually need to bump the defaultHandlersVersion before working on a script.
Flags: needinfo?(francesco.lodolo)
Depends on: 1343454
> * It occurs to me that the other handler for the ro/uk locales is Google > Calendar, so in theory we could just change things from 30boxes to GCal. Nit: for uk is yandex, not Google. If we plan to replace 30boxes with Google, it would be really helpful to know before starting to touch all l10n repositories, in order to do everything in one pass.
(In reply to Francesco Lodolo [:flod] from comment #9) > I will file a bug and take care of it at the beginning of the next Aurora > cycle, but I would really like someone to confirm if we actually need to > bump the defaultHandlersVersion before working on a script. From a quick test hacking browser/omni.ja and replacing 30 Boxes with Google Calendar (haven't tried if the link actually works): * Without touching defaultHandlersVersion Google Calendar doesn't show up. * Incrementing defaultHandlersVersion makes Google Calendar appear, but also 30 Boxes remains there, it's not removed.
(In reply to Justin Dolske [:Dolske] from comment #5) > r+, but I think you also need increment the defaultHandlersVersion in > region.properties. Please do that and verify that this actually results in > webcal handler for an existing profile being purged. We don't remove existing handlers when we import new defaults, so there's also no point in updating the defaultHandlersVersion. https://dxr.mozilla.org/mozilla-central/rev/e1135c6fdc9bcd80d38f7285b269e030716dcb72/uriloader/exthandler/nsHandlerService.js#242 If you want, you can remove the old entry from the list of possibleApplicationHandlers but: - You should only do it if it's not also the preferredApplicationHandler - You need to know the previous URL or the description for the specific locale This seems more suited for a migration step in nsBrowserGlue.js. If you fancy a rewrite of handler import, it's probably better to move away from the weird nsIPrefLocalizedString system anyways... The fun thing is that the URI is the only key, so bug 840699 actually resulted in a second item being added to possibleApplicationHandlers, so we have potentially two entries for that service to remove in old profiles. https://dxr.mozilla.org/mozilla-central/rev/e1135c6fdc9bcd80d38f7285b269e030716dcb72/uriloader/exthandler/nsWebHandlerApp.js#56 You can also argue that since we left things behind once, we can do that twice, and ignore the issue of removing old possible handlers.
(In reply to :Paolo Amadini from comment #12) > We don't remove existing handlers when we import new defaults, so there's > also no point in updating the defaultHandlersVersion. > [...] > The fun thing is that the URI is the only key, so bug 840699 actually > resulted in a second item being added to possibleApplicationHandlers, so we > have potentially two entries for that service to remove in old profiles. Ugh. It feels like this is a mess we should really clean up, and not just leave our existing users with these settings. The duplication of http/https entries was unfortunate, but the current state of things seems more serious. Nuking these entries (as in the current patch) and adding a migration step to explicitly clear 30boxes would work for me. I agree trying to fix up the current pref-based scheme would be gross, so let's not try to fix the general problem with changes/deletions. Brian, are you game to take that on?
(In reply to Francesco Lodolo [:flod] from comment #10) > If we plan to replace 30boxes with Google, it would be really helpful to > know before starting to touch all l10n repositories, in order to do > everything in one pass. I will posit that we are not going to replace it or otherwise add Google. (I'm not opposed to doing so if it somehow makes things simpler, but it seems like it would just be a complication that adds no value.) In fact, I would go so far as to say that if the 2 locales with Google cause significant hassle in removing 30boxes, we could just remove GCal from those two locales too. (Thus making this "remove all default webcal support from everywhere"). Doesn't sound like a problem so far, though.
(In reply to Justin Dolske [:Dolske] from comment #14) > In fact, I would go so far as to say that if the 2 locales with Google cause "Google or Yandex" :)
Yup, I'm game. I should be able to knock this out next week unless something comes up.
NI just to make sure this bug is not forgotten (I need to understand the details of the migration and how they affect l10n builds):
Flags: needinfo?(geeknik)
Sorry, something did come up last week. About the `migration step` being mentioned in multiple places, I don't know how/where to touch nsBrowserGlue.js for this... (In reply to Justin Dolske [:Dolske] from comment #13) > Nuking these entries (as in the current patch) and adding a migration step > to explicitly clear 30boxes would work for me. I agree trying to fix up the > current pref-based scheme would be gross, so let's not try to fix the > general problem with changes/deletions. (In reply to :Paolo Amadini from comment #12) > This seems more suited for a migration step in nsBrowserGlue.js. If you > fancy a rewrite of handler import, it's probably better to move away from > the weird nsIPrefLocalizedString system anyways...
See the bottom of _migrateUI: http://searchfox.org/mozilla-central/rev/a5c2b278897272497e14a8481513fee34bbc7e2c/browser/components/nsBrowserGlue.js#1677 Basically, increment UI_VERSION, and add a version check to clear 30boxes from the default handlers.
So something like this? if (currentUIVersion < 44) { // Remove 30Boxes #1252831 try { if (Services.prefs.getCharPref("gecko.handlerService.schemes.webcal.0.name") = 30 Boxes) { Services.prefs.clearUserPref("gecko.handlerService.schemes.webcal.0.name"); Services.prefs.clearUserPref("gecko.handlerService.schemes.webcal.0.uriTemplate"); } catch (ex) {} }
Flags: needinfo?(geeknik)
No, you have to call nsIExternalProtocolHandlerService.getProtocolHandlerInfo and then call nsIHandlerService.store only if you actually had to remove items from the possibleApplicationHandlers. You can look at the code referenced in comment 12 for something comparable.
As Alphan noted in bug 1287660 comment 75, we've added some testing around the import, so this patch may have to modify the new tests as well.
Unassigning: I don't have the cycles available for this.
Assignee: geeknik → nobody
I'm going to call this fixed, since we removed 30boxes entirely.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
(In reply to Mike Connor [:mconnor] from comment #24) > I'm going to call this fixed, since we removed 30boxes entirely. We really didn't… This patch never landed, pending the migration questions https://hg.mozilla.org/mozilla-central/file/tip/browser/locales/en-US/chrome/browser-region/region.properties#l25
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: nobody → gijskruitbosch+bugs
Status: REOPENED → ASSIGNED
Attachment #9040090 - Attachment description: Bug 1252831 - remove 30boxes webcal handler, r?paolo!,dolske! → Bug 1252831 - remove 30boxes webcal handler as both possible and default handler, r?paolo!,dolske!
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b3bcf26d4dad remove 30boxes webcal handler as both possible and default handler, r=paolo,Dolske

Release Note Request (optional, but appreciated)
[Why is this notable]: Removes a long-standing webcal protocol handler
[Affects Firefox for Android]: yes
[Suggested wording]: Firefox no longer supports handling webcal: links with 30boxes.com
[Links (documentation, blog post, etc)]: n/a

Joni, can you give the good people at SUMO a heads-up that this is incoming, in case any confused users end up on SUMO forums? We don't believe this is in serious amounts of use so don't expect much trouble, but just in case.

For reference, the best workaround would be 30boxes using the registerProtocolHandler API to allow users to add support back if they want, though there were some issues with the URL that was in use in Firefox that might mean that they'd want to update the handling URL. I can't find a good contact point for 30boxes on their website, but if people still use it hopefully they have contacts themselves...

relnote-firefox: --- → ?
Flags: needinfo?(jsavage)

Backed out changeset b3bcf26d4dad (Bug 1252831) for xpcshell failures in uriloader/exthandler/tests/unit/test_handlerService.js CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=227271736

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=227271736&repo=autoland&lineNumber=2062

[task 2019-02-08T20:44:54.710Z] 20:44:54 WARNING - TEST-UNEXPECTED-FAIL | uriloader/exthandler/tests/unit/test_handlerService.js | xpcshell return code: 0
[task 2019-02-08T20:44:54.710Z] 20:44:54 INFO - TEST-INFO took 596ms
[task 2019-02-08T20:44:54.710Z] 20:44:54 INFO - >>>>>>>
[task 2019-02-08T20:44:54.710Z] 20:44:54 INFO - PID 12661 | [12661, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2532
[task 2019-02-08T20:44:54.710Z] 20:44:54 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2019-02-08T20:44:54.714Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 98] true == true
[task 2019-02-08T20:44:54.716Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 100] "nonexistent/type" == "nonexistent/type"
[task 2019-02-08T20:44:54.717Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 103] "nonexistent/type" == "nonexistent/type"
[task 2019-02-08T20:44:54.718Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 106] 0 == 0
[task 2019-02-08T20:44:54.722Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 107] null == null
[task 2019-02-08T20:44:54.724Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 108] 0 == 0
[task 2019-02-08T20:44:54.725Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 109] true == true
[task 2019-02-08T20:44:54.731Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 113] "" == ""
[task 2019-02-08T20:44:54.734Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 114] false == false
[task 2019-02-08T20:44:54.735Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 115] "" == ""
[task 2019-02-08T20:44:54.737Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 137] 1 == 1
[task 2019-02-08T20:44:54.739Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 138] true == true
[task 2019-02-08T20:44:54.741Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 145] 0 == 0
[task 2019-02-08T20:44:54.743Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 146] true == true
[task 2019-02-08T20:44:54.744Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 155] 0 == 0
[task 2019-02-08T20:44:54.746Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 156] true == true
[task 2019-02-08T20:44:54.748Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 162] 2 == 2
[task 2019-02-08T20:44:54.751Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 170] true == true
[task 2019-02-08T20:44:54.753Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 176] 2 == 2
[task 2019-02-08T20:44:54.754Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 185] true == true
[task 2019-02-08T20:44:54.756Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 200] 2 == 2
[task 2019-02-08T20:44:54.758Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 201] true == true
[task 2019-02-08T20:44:54.760Z] 20:44:54 INFO - PID 12661 | [12661, Main Thread] WARNING: 'mIndex >= Count()', file /builds/worker/workspace/build/src/xpcom/ds/nsStringEnumerator.cpp, line 208
[task 2019-02-08T20:44:54.761Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 221] 2 == 2
[task 2019-02-08T20:44:54.763Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 223] [object XPCWrappedNative_NoHelper] != null
[task 2019-02-08T20:44:54.765Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 225] "object" == "object"
[task 2019-02-08T20:44:54.767Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 226] "Local Handler" == "Local Handler"
[task 2019-02-08T20:44:54.769Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 227] true == true
[task 2019-02-08T20:44:54.771Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 229] "/tmp/xpc-profile-k4BS8n" == "/tmp/xpc-profile-k4BS8n"
[task 2019-02-08T20:44:54.772Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 231] true == true
[task 2019-02-08T20:44:54.774Z] 20:44:54 INFO - PID 12661 | [12661, Main Thread] WARNING: 'mIndex >= Count()', file /builds/worker/workspace/build/src/xpcom/ds/nsStringEnumerator.cpp, line 208
[task 2019-02-08T20:44:54.776Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 244] 0 != -1
[task 2019-02-08T20:44:54.778Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 244] 0 != -1
[task 2019-02-08T20:44:54.780Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 244] 3 != -1
[task 2019-02-08T20:44:54.781Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 244] 1 != -1
[task 2019-02-08T20:44:54.783Z] 20:44:54 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 244] 1 != -1
[task 2019-02-08T20:44:54.785Z] 20:44:54 WARNING - TEST-UNEXPECTED-FAIL | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 247] 1 == 0
[task 2019-02-08T20:44:54.787Z] 20:44:54 INFO - /builds/worker/workspace/build/tests/xpcshell/tests/uriloader/exthandler/tests/unit/test_handlerService.js:run_test:247
[task 2019-02-08T20:44:54.789Z] 20:44:54 INFO - /builds/worker/workspace/build/tests/xpcshell/head.js:_execute_test:521
[task 2019-02-08T20:44:54.790Z] 20:44:54 INFO - -e:null:1
[task 2019-02-08T20:44:54.792Z] 20:44:54 INFO - exiting test
[task 2019-02-08T20:44:54.794Z] 20:44:54 INFO - PID 12661 | [12661, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, kKnownEsrVersion) failed with result 0x80004002: file /builds/worker/workspace/build/src/toolkit/components/resistfingerprinting/nsRFPService.cpp, line 662
[task 2019-02-08T20:44:54.796Z] 20:44:54 INFO - PID 12661 | [12661, Main Thread] WARNING: OOPDeinit() without successful OOPInit(): file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 3125
[task 2019-02-08T20:44:54.798Z] 20:44:54 INFO - PID 12661 | nsStringStats

Flags: needinfo?(gijskruitbosch+bugs)
Backout by shindli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3038495b95fc Backed out changeset b3bcf26d4dad for xpcshell failures in uriloader/exthandler/tests/unit/test_handlerService.js CLOSED TREE

Thanks for the heads up! I've shared this and the workaround with our SUMO folks. If we see a lot of users come in, we'll report back.

Flags: needinfo?(jsavage)
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0325530dd930 remove 30boxes webcal handler as both possible and default handler, r=paolo,Dolske
Flags: needinfo?(gijskruitbosch+bugs)

https://hg.mozilla.org/mozilla-central/rev/0325530dd930

Status: ASSIGNED → RESOLVED
Closed: 7 years ago → 6 years ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

(In reply to :Gijs (he/him) from comment #28)

Release Note Request (optional, but appreciated)
[Why is this notable]: Removes a long-standing webcal protocol handler
[Affects Firefox for Android]: yes
[Suggested wording]: Firefox no longer supports handling webcal: links with 30boxes.com

Added to Desktop 67 nightly notes

(In reply to Mike Kaply [:mkaply] from comment #3)

We should remove 30 boxes, not just because the https doesn't work, but
because their login doesn't even use https:

http://30boxes.com/login

I don't think we should be pushing this service.

The certificate works since 2017 and the login is at https://www.30boxes.com/login

Is there any other reason for this removal?
And is there any data about its usage?

(In reply to Thomas Bertels from comment #35)

Is there any other reason for this removal?

Yes. If you have contacts at 30boxes, please tell them to contact me privately.

The very general story is that there's no evidence the site is being maintained and this has been causing problems (like the cert issue that occurred a few years ago and led to the filing of this bug).

And is there any data about its usage?

Not to my knowledge - I don't believe we have usage data for any of the protocol handlers we ship by default.

However, considering it was broken for a significant amount of time without anyone noticing or caring, it can't be very high...

Note that users who want to continue to use 30boxes are free to do so, and 30boxes can easily provide the opportunity for its users to make their site the default webcal: handler using the registerProtocolHandler API available to any webpage, which is what other websites do (e.g. irccloud for irc: links).

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: