Refactor of LibraryFileAlias.[secure_]url

Bug #76743 reported by Guilherme Salgado
4
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
Medium
Guilherme Salgado

Bug Description

Currently, we have LibraryFileAlias.url and LibraryFileAlias.secure_url, where the latter just replace 'http' with 'https' from the former's value and returns it. This is a problem because we don't want to have any https links on our development instance.

I discussed this with Steve and the plan is to define http_url (identical to the existing url) and https_url (identical to the current securel_url) properties and a getURL() method, where the latter will check the value of config.launchpad.virtual_hosts.user_https and return either the value of http_url or https_url.

Changed in launchpad:
assignee: nobody → salgado
importance: Undecided → Medium
status: Unconfirmed → Confirmed
Revision history for this message
Stuart Bishop (stub) wrote : Re: [Bug 76743] Refactor of LibraryFileAlias.[secure_]url

Guilherme Salgado wrote:

> Currently, we have LibraryFileAlias.url and LibraryFileAlias.secure_url,
> where the latter just replace 'http' with 'https' from the former's
> value and returns it. This is a problem because we don't want to have
> any https links on our development instance.

It is because while most stuff we want retrieved from the Librarian via
HTTP, images that get embedded in Launchpad pages such as emblems and
hackergotchis need to be retrieved via HTTPS to avoid warnings on some
browsers. So where this is the case, .secure_url is used to calculate the
URL rather than just .url

> I discussed this with Steve and the plan is to define http_url
> (identical to the existing url) and https_url (identical to the current
> securel_url) properties and a getURL() method, where the latter will
> check the value of config.launchpad.virtual_hosts.user_https and return
> either the value of http_url or https_url.

This bug report does not explain why any change is necessary - it just gives
a solution without explaining the problem.

--
Stuart Bishop <email address hidden> http://www.canonical.com/
Canonical Ltd. http://www.ubuntu.com/

Revision history for this message
Guilherme Salgado (salgado) wrote :

On Fri, Dec 22, 2006 at 12:53:45AM -0000, Stuart Bishop wrote:
[...]
> This bug report does not explain why any change is necessary - it just gives
> a solution without explaining the problem.
>

The problem is that we don't have the necessary apache magic (and I think we
don't want to) to be able to get files from the librarian via https. This
means that any page which includes an image will take forever to load on our
development boxes as it'll never succeed to download the image.

Revision history for this message
Stuart Bishop (stub) wrote :

Guilherme Salgado wrote:
> On Fri, Dec 22, 2006 at 12:53:45AM -0000, Stuart Bishop wrote:
> [...]
>> This bug report does not explain why any change is necessary - it just gives
>> a solution without explaining the problem.
>>
>
> The problem is that we don't have the necessary apache magic (and I think we
> don't want to) to be able to get files from the librarian via https. This
> means that any page which includes an image will take forever to load on our
> development boxes as it'll never succeed to download the image.

Hmm... I thought that secure_url was was supposed to return the HTTPS url if
the use_https config variable was set, and the HTTP url if not. I was mistaken.

It looks like the only uses of secure_url are places where we want this
behaviour, so I don't see the need for a new method - just change the
existing secure_url property?

--
Stuart Bishop <email address hidden> http://www.canonical.com/
Canonical Ltd. http://www.ubuntu.com/

Revision history for this message
Guilherme Salgado (salgado) wrote :

On Wed, Jan 03, 2007 at 03:27:28PM -0000, Stuart Bishop wrote:
[...]
>
> It looks like the only uses of secure_url are places where we want this
> behaviour, so I don't see the need for a new method - just change the
> existing secure_url property?
>

Yeah, this is what I initially suggested, but Steve said he was unhappy with
a secure_url property which may return an http or https URL, and I kind of
agree with him.

Changed in launchpad:
status: Confirmed → In Progress
Revision history for this message
Stuart Bishop (stub) wrote :

Guilherme Salgado wrote:
> On Wed, Jan 03, 2007 at 03:27:28PM -0000, Stuart Bishop wrote:
> [...]
>> It looks like the only uses of secure_url are places where we want this
>> behaviour, so I don't see the need for a new method - just change the
>> existing secure_url property?
>>
>
> Yeah, this is what I initially suggested, but Steve said he was unhappy with
> a secure_url property which may return an http or https URL, and I kind of
> agree with him.
>

You should remove secure_url at the same time then, or at least make it
private, as its current implementation is broken - we never want Launchpad
to produce https: urls in installations that do not support SSL.

--
Stuart Bishop <email address hidden> http://www.canonical.com/
Canonical Ltd. http://www.ubuntu.com/

Changed in launchpad:
status: In Progress → Fix Committed
Changed in launchpad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.