383991 - Full SSL support for Calendars

Status: RESOLVED FIXED

(Calendar :: Provider: GData, enhancement)

Description

[Sebastian Nohn]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4
Build Identifier: 

Google Calendar Provider should fully support SSL. Only some .js-Files in the xpi need to be adjusted for that.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Sebastian Nohn]

Attachment: Version 0.2.1 with SSL support => 0.2.2

Comment 2

[Philipp Kewisch [:Fallen][☁️📆]]

Sebastian, please create a diff for the specific files you changed, instead of a new xpi. I'd like to use a preference for this, something like calendar.google.ssl which defaults to true. Please make sure this is included int the patch. Also, you may request review from my email ([e-mail]) via the review fields when you have a ready patch.

I was not aware that google supported ssl for more than authentication. I just read a newsgroup article where it was required for certain types of google apps for your domain accounts [1]. It mentions that requests will be redirected, which the provider currently handles. I'd love to have a test account though, or at least someone who has a test account (do you?).

[1] http://groups.google.com/group/google-calendar-help-dataapi/browse_thread/thread/5cd402af96150b48/

Comment 3

[Sebastian Nohn]

Hi Philipp, this is the diff *without* the requests preference setting, I first need to understand how properties are handled:

----- cut here ----

./js/calGoogleCalendar.js
235c235
<         this.mFullUri = ioService.newURI("http://www.google.com" +
---
>         this.mFullUri = ioService.newURI("https://www.google.com" +
./js/calGoogleRequest.js
100c100
<                 this.uri = "http://www.google.com/calendar/feeds/" +
---
>                 this.uri = "https://www.google.com/calendar/feeds/" +
232c232
<                     this.mUriString = "http://www.google.com/calendar/feeds/"
---
>                     this.mUriString = "https://www.google.com/calendar/feeds/"

----- cut here ----

Comment 4

[Philipp Kewisch [:Fallen][☁️📆]]

Please read http://developer.mozilla.org/en/docs/Creating_a_patch and attach your patches as an attachment, or see other bugs how patches are generally handled. For some hints on prefs, see calUtils.js. It has a getPrefSafe() function.

Comment 5

[Philipp Kewisch [:Fallen][☁️📆]]

Actually, I think it might be better to just detect the scheme from the entered URL and use that. I.e if the user enters https://www.google.com/feeds/calendar/... then he will have full ssl support, otherwise not.

My initial though would be that we want to encourage users to use SSL, but on the other hand, bug 378873 could have a checkbox that allows the user to select SSL for their calendar.

Sebastian, are you still interested in implementing this? Otherwise I will upload a patch next week.

Comment 6

[Sebastian Nohn]

Go ahead. I'm busy at the moment.

Comment 7

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: SSL Support - v1

This should do it. To use SSL all that is needed is to make the URI have https instead of http. Also patches the edit uri, since that is not https even if a https request is made.

Comment 8

[cmtalbert]

Comment on attachment 270552 [details] [diff] [review]
SSL Support - v1

The patch itself looks OK.  I tested this by downloading the currently available version of gdata from AMO and replacing these files in it.  Then I changed my XML subscription URL from http to https.  I can read and write entire events ok, but I cannot update existing events.  I am going to withhold the review until we figure out if (a) Google isn't handling update on https URLs properly or (b) I did something wrong in setting up my test.

Comment 9

[cmtalbert]

Comment on attachment 270552 [details] [diff] [review]
SSL Support - v1

ok, problem resolved, I did something wrong. :-)
r=ctalbert

Comment 10

[Philipp Kewisch [:Fallen][☁️📆]]

Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED