#1394 compute_asset_host bombs when asset_host = Proc and called from a Mailer - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#1394 ✓committed

compute_asset_host bombs when asset_host = Proc and called from a Mailer

Reported by Tekin | November 17th, 2008 @ 09:59 PM | in 2.x

compute_asset_host is falling over when used in Mailer views under certain conditions.

If you set asset_host to a 2 arguement Proc, compute_asset_host calls the Proc, passing in the source and the request object. Problem is, Mailer objects will not have a request object so compute_asset_host falls over with an undefined method error.

This simple patch updates compute_asset_host to pass nil in place of the request if no request is present. This way, you can write your Proc to cope with a nil request if necessary.

stop_compute_asset_host_bombing_when_no_request_object.diff 2.1 KB

Comments and changes to this ticket

Michael Koziarski November 18th, 2008 @ 06:28 PM
Can you also provide a test for actionmailer which shows the problem?

Would help avoid regressions and make the changeset a little more obvious.
You flagged this item as spam.
Tekin November 18th, 2008 @ 06:35 PM
Yeah, sorry, had been a long day. Will get a proper patch in asap.
You flagged this item as spam.
Tekin November 20th, 2008 @ 11:27 PM
Just looking into this a bit more, although it's still not right that compute_asset_host falls over in mailer templates when asset_host is set to a 2 arguement Proc, it seems the primary reason you'd use a Proc this way would be to provide an alternative host for SSL requests.

I'm just wondering, would it make more sense to be able to explicitly specify an optional SSL specific host?

config.action_controller.ssl_asset_host = "https://www.example.com"

That would certainly scratch my itch. What do you think?
You flagged this item as spam.
Tekin November 25th, 2008 @ 11:38 AM
Here are some tests, including one that fails.

Although my patch fixes the problem, long term, I'm not convinced it's the right solution. The second argument was only really introduced to help deal with SSL:

http://dev.rubyonrails.org/ticke...

Long term, I think it make sense to depreciate this second parameter and add ssl_asset_host as an option to ActionController::Base. Unless there is a use case for the request arguement that I am missing?
- failing_tests_for_action_mailer_with_asset_host.diff 2.9 KB
Repository December 1st, 2008 @ 06:39 PM
- State changed from “new” to “committed”
(from [dab78e55cfcc111b898a1c2475c0c5c327db30f9]) Ensure ActionMailer doesn't blow up when a two argument proc is set for the asset host

Signed-off-by: Michael Koziarski michael@koziarski.com [#1394 state:committed] http://github.com/rails/rails/co...
You flagged this item as spam.
Tekin December 1st, 2008 @ 08:04 PM
Cheers. Much more concise!
Jeff Kreeftmeijer November 8th, 2010 @ 08:21 AM
- Tag cleared.
Automatic cleanup of spam.