1634052 - Folder lookup service doesn't honour all possible valid URI schemes

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[[e-mail]]

Bug 1527764 introduced the following code:

let scheme = uri.match(/\w*/)[0];

However, the +, - and . characters are also valid in schemes. This could cause a compatibility issue if a third-party account had been using one of those characters in its schemes.

Comment 1

[[e-mail]]

Attachment: Proposed patch

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 9144446 [details] [diff] [review]
Proposed patch

Aceman hasn't been seen for a while.

Comment 3

[Ben Campbell]

Comment on attachment 9144446 [details] [diff] [review]
Proposed patch

Review of attachment 9144446 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, and the new regex passes the manually-try-out-assorted-odd-URIs-in-the-javascript-console test :-)
Thanks Neil!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9129a21f6fac
Make FOlderLookupService match the same schemes that the RDF service supported. r=benc

Comment 5

[Magnus Melin [:mkmelin]]

This was not quite right. I notice running comm/mail/test/browser/account/browser_values.js

0:12.57 GECKO(8341) JavaScript error: resource:///modules/FolderLookupService.jsm, line 69: TypeError: can't access property 1, uri.match(...) is null
0:12.63 INFO Console message: [JavaScript Error: "TypeError: can't access property 1, uri.match(...) is null" {file: "resource:///modules/FolderLookupService.jsm" line: 69}]
getOrCreateFolderForURL@resource:///modules/FolderLookupService.jsm:69:22
getOrCreateFolder@resource:///modules/MailUtils.jsm:92:16
checkJunkTargetFolder@chrome://messenger/content/amUtils.js:91:32
sanitizeJunkTargets@chrome://messenger/content/amUtils.js:163:30
onInit@chrome://messenger/content/am-junk.js:49:26
restorePage@chrome://messenger/content/AccountManager.js:1536:29
onPanelLoaded@chrome://messenger/content/AccountManager.js:1305:16
onload@chrome://messenger/content/am-junk.xhtml:1:8
waitFor@resource://testing-common/mozmill/utils.jsm:425:12
click_account_tree_row@resource://testing-common/mozmill/AccountManagerHelpers.jsm:88:9
subtest_check_invalid_junk_target@chrome://mochitests/content/browser/comm/mail/test/browser/account/browser_values.js:289:25
test_invalid_junk_target/<@chrome://mochitests/content/browser/comm/mail/test/browser/account/browser_values.js:269:38
open_advanced_settings@resource://testing-common/mozmill/AccountManagerHelpers.jsm:55:11
test_invalid_junk_target@chrome://mochitests/content/browser/comm/mail/test/browser/account/browser_values.js:268:25
Tester_execTest/<@chrome://mochikit/content/browser-test.js:1064:34
async*Tester_execTest@chrome://mochikit/content/browser-test.js:1104:11
nextTest/<@chrome://mochikit/content/browser-test.js:927:14
SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:918:23

uri=some random non-existent URI/Junk

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: bug1634052_folder_scheme.patch

Comment 7

[Ben Campbell]

Comment on attachment 9149370 [details] [diff] [review]
bug1634052_folder_scheme.patch

Review of attachment 9149370 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like a good improvement to me.

Comment 8

[Ben Campbell]

Comment on attachment 9149370 [details] [diff] [review]
bug1634052_folder_scheme.patch

(hmm - looks like I clobbered the other r?...)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6d85fa4f17a2
getOrCreateFolderForURL should handle bad uris. r=benc DONTBUILD