Closed
Bug 334423
Opened 18 years ago
Closed 17 years ago
calDAV provider leaks auth info into X-MOZ-LOCATIONURI
Categories
(Calendar :: Provider: CalDAV, defect)
Calendar
Provider: CalDAV
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: browning, Unassigned)
Details
Attachments
(2 files, 1 obsolete file)
908 bytes,
patch
|
dmosedale
:
first-review+
|
Details | Diff | Splinter Review |
1.32 KB,
patch
|
mattwillis
:
first-review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060313 Fedora/1.5.0.1-9 Firefox/1.5.0.1 pango-text Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060417 Mozilla Sunbird/0.3a1+ When a calDAV calendar is defined in the form http(s)://user:pass@host.... calendar inserts the URI, including the auth info, into an X-MOZ-LOCATIONURI property in events created in the calendar. Reproducible: Always Steps to Reproduce: 1. Create CalDAV calendar using a spec with embedded auth info 2. Create an event in that calendar 3. Examine X-MOZ-LOCATIONURI property of event Actual Results: X-MOZ-LOCATIONURI shows username, password to the calendar Expected Results: Calendar should not expose user authentication info
Reporter | ||
Comment 1•18 years ago
|
||
patch replaces X-MOZ-LOCATIONURI with X-MOZ-LOCATIONPATH. X-MOZ-LOCATIONPATH should be the portion of the item URI not already specified by the calendar .spec. This patch just uses item.id + ".ics" for new items but is marginally smarter in reportInternal.
Attachment #218761 -
Flags: first-review?(dmose)
Comment 2•18 years ago
|
||
Comment on attachment 218761 [details] [diff] [review] change X-MOZ-LOCATIONURI to X-MOZ-LOCATIONPATH Looks good; thanks for reporting this and posting a patch so quickly! I've got a couple of really minor nits, which I'll fix before checking in: >Index: calendar/providers/caldav/calDavCalendar.js >=================================================================== >RCS file: /cvsroot/mozilla/calendar/providers/caldav/calDavCalendar.js,v >retrieving revision 1.55 >diff -p -u -r1.55 calDavCalendar.js >--- calendar/providers/caldav/calDavCalendar.js 15 Apr 2006 13:31:18 -0000 1.55 >+++ calendar/providers/caldav/calDavCalendar.js 18 Apr 2006 02:28:12 -0000 >@@ -265,7 +265,7 @@ calDavCalendar.prototype = { > > aItem.calendar = this; > aItem.generation = 1; >- aItem.setProperty("X-MOZ-LOCATIONURI", itemUri.spec); >+ aItem.setProperty("X-MOZ-LOCATIONPATH", aItem.id + ".ics"); Since |aItem.id + ".ics"| is already done earlier in the function, I'll hoist that definition out to a single "const" statement. >@@ -557,8 +557,9 @@ calDavCalendar.prototype = { > item.calendar = thisCalendar; > > // save the location name in case we need to modify >- item.setProperty("X-MOZ-LOCATIONURI", aResource.spec); >- debug("X-MOZ-LOCATIONURI = " + aResource.spec + "\n"); >+ var localPath = aResource.path.substr(thisCalendar.mCalendarUri.path.length); I think it makes a bit more sense to call this variable "locationPath", since it's not really "local" per se.
Attachment #218761 -
Flags: first-review?(dmose) → first-review+
Updated•18 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
Comment 3•18 years ago
|
||
Landed; thanks for the patch!
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 4•18 years ago
|
||
reopening because previous patch breaks on oracle calDAV server, which urlencodes the path in question
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 5•18 years ago
|
||
this patch (against current calendar) sets LOCATIONPATH correctly on both current version of cosmo and the oracle calDAV server.
Attachment #218761 -
Attachment is obsolete: true
Reporter | ||
Updated•18 years ago
|
Attachment #219698 -
Flags: first-review?(dmose)
Comment 6•18 years ago
|
||
Comment on attachment 219698 [details] [diff] [review] urldecode path Thanks for the patch; r=dmose. I've landed the patch, but I'm leaving the bug open, as I think this needs one more tweak: we have no guarantee that the user created this calendar with an unescaped URL. So I think we need to create a guaranteed-unescaped version in the setter that we keep around and then use to do this comparison.
Attachment #219698 -
Flags: first-review?(dmose) → first-review+
Updated•18 years ago
|
Attachment #219698 -
Attachment is patch: true
Attachment #219698 -
Attachment mime type: text/x-patch → text/plain
Reporter | ||
Comment 7•17 years ago
|
||
Attachment #248801 -
Flags: second-review?(dmose)
Comment 8•17 years ago
|
||
Comment on attachment 248801 [details] [diff] [review] urldecoded mLocationPath This does not need a 2nd review anymore, since the former module owner (dmose) already gave his review.
Attachment #248801 -
Flags: second-review?(dmose)
Comment 9•17 years ago
|
||
Comment on attachment 248801 [details] [diff] [review] urldecoded mLocationPath Actually, this is a separate patch that _does_ need review. r=lilmatt with the }, line indented properly
Attachment #248801 -
Flags: first-review+
Comment 10•17 years ago
|
||
Patch (with spacing fixed) checked in on MOZILLA_1_8_BRANCH and trunk. -> FIXED
Status: REOPENED → RESOLVED
Closed: 18 years ago → 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•