ActiveStorage::Current.host not set causing disk service to throw URI::InvalidURIError

[daqing]

Steps to reproduce

config/storage.yml:

test:
  service: Disk
  root: <%= Rails.root.join("tmp/storage") %>

local:
  service: Disk
  root: <%= Rails.root.join("storage") %>
  public: true

config/environments/development.rb:

  # Store uploaded files on the local file system (see config/storage.yml for options).
  config.active_storage.service = :local

app/models/user.rb:

class User < ApplicationRecord
  validates :username, presence: true

  has_one_attached :avatar

Expected behavior

In rails console:

irb> ActiveStorage::Current.host
=> nil

irb> User.find_by_username('david').avatar.url
=> [should generate permanent url]

Actual behavior

In rails console:

irb> ActiveStorage::Current.host
=> nil

irb> User.find_by_username('david').avatar.url
URI::InvalidURIError (bad URI(is not URI?): nil)

System configuration

Rails version:

Rails 6.1.0

Ruby version:

ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin19]

[daqing]

There's no documents about setting ActiveStorage::Current.host before generating public permanent url using the Disk service.

[timdorr]

Interestingly, Rails doesn't see this in their test suite because they set this value during setup:

rails/activestorage/test/test_helper.rb

Lines 52 to 54 in 9492339

	setup do
	ActiveStorage::Current.host = "https://example.com"
	end

For Rspec users, you can workaround this by setting it the same way in a spec helper:

RSpec.configure do |config|
  config.before(:each) do
    ActiveStorage::Current.host = "https://example.com"
  end
end

[hunterbmiller]

For those using ActionController to handle requests, there is a Concern available that will set ActiveStorage::Current.host to request.base_url as a before_action:

https://github.com/rails/rails/blob/master/activestorage/app/controllers/concerns/active_storage/set_current.rb

But, this is only a partial solution as it still wouldn't work outside of your controllers.

Since I'm using Grape for my request handling, my workaround right now is to just explicitly set ActiveStorage::Current.host before I use .url. Luckily, I only use the Disk service in testing and development.

It would be ideal if we could set this value in our config, similar to how we can set url options for ActionMailer:

config.action_mailer.default_url_options = { host: 'localhost:3000' }

Perhaps there is a better way?

[ryanseys]

Also seeing this issue. Is there any way to set this host globally e.g. via a configuration, as others have suggested? If that seems reasonable to the maintainers of Rails, I could also take a stab at implementing it myself.

[timdorr]

Presumably #39566 will resolve this by creating a system for URLs outside of the controller context. I'm not sure of a workaround for now, as we punted on our AS work once we ran into this issue.

[exocode]

Interestingly, Rails doesn't see this in their test suite because they set this value during setup:

RSpec.configure do |config|I dadasdf
  config.before(:each) do
    ActiveStorage::Current.host = "https://example.com"
  end
end

^^^ This approach won't work with my rSpec tests.

rails 6.1.3.1
Using rspec-support 3.10.2
Using rspec-core 3.10.1
Using rspec-expectations 3.10.1
Using rspec-mocks 3.10.2
Using rspec 3.10.0

I added your lines in my rails_helper.rb but it is still nil. I also placed ActiveStorage::Current.host = "http://example.com in the model itself, without success:
WON'T WORK

class User
  ActiveStorage::Current.host = "http://localhost:3000"
  def main_image_thumb_url
    main_image.variant(thumb_options).url
  end
end

ONLY when I place that right before calling the .url method, then my tests pass:

WORKS

class User
  def main_image_thumb_url
    ActiveStorage::Current.host = "http://localhost:3000"
    main_image.variant(thumb_options).url
  end
end

What I've done now is placing include ActiveStorage::SetCurrent in my ApplicationController. But I am not sure if this how it should be...

Normally I expect that this should be set magically in the background.

[gat-bryszard]

I've encountered similar problem, related to behavior of ActiveStorage::Current.host when you use Fibers (we do).

I'm thinking it might be worth a separate issue. I'm not sure though, so for now mentioning it here.

Basically the problem is this:

spec/spec_helper.rb

config.before do
  ActiveStorage::Current.host = "https://example.com"
end

And it works fine. Unless we use Fibers:

example_spec.rb

it "check ActiveStorage::Current host with Fibers" do
 fiber = Fiber.new do
   Fiber.yield "ActiveStorage::Current:host in Fiber - #{ActiveStorage::Current.host}"
 end

 puts fiber.resume
 puts "ActiveStorage::Current:host out of Fiber - #{ActiveStorage::Current.host}"
end

Result:

ActiveStorage::Current:host in Fiber - 
ActiveStorage::Current:host out of Fiber - https://example.com

So now my tests for cases when Fibers are used fail unless I set the ActiveStorage::Current.host = "https://example.com" if Rails.env.test? in the code, right before I'm calling the blob.url. Which is of course awful.

[intrip]

@gat-bryszard related to #40855 (comment)

That's because ActiveStorage::Current uses

rails/activesupport/lib/active_support/current_attributes.rb

Line 152 in 69dfa29

Thread.current[:current_attributes_instances] ||= {}

Which stores the settings on a Fiber local variable hence when you create a new Fiber you loose this value (Same applies for a new Thread)

If helps you can workaround this by storing Thread.current[:current_attributes_instances] in a local variable and then restoring it manually:

it "check ActiveStorage::Current host with Fibers" do
 local_current_attributes = Thread.current[:current_attributes_instances]
 fiber = Fiber.new do
   Thread.current[:current_attributes_instances] = local_current_attributes
   Fiber.yield "ActiveStorage::Current:host in Fiber - #{ActiveStorage::Current.host}"
 end

 puts fiber.resume
 puts "ActiveStorage::Current:host out of Fiber - #{ActiveStorage::Current.host}"
end

[intrip]

#39566 would definitely help, until we'll have that feature I was wondering if showing a more meaningful error when ActiveStorage::Current.host is nil would help, @zzak what's your take on this?

[jasondoc3]

For those looking for a quick fix, I've been able to workaround this by monkey patching ActiveStorage::Blob#service_url

# config/storage.yml
local:
  service: Disk
  root: <%= Rails.root.join("storage") %>

module CoreExtensions
  module ActiveStorage
    module Blob
      module ServiceUrl
        def service_url(*)
          if Rails.configuration.active_storage.service == :local
            h = "http://#{Rails.application.routes.default_url_options[:host]}"
            ::ActiveStorage::Current.set(host: h) { super }
          else
            super
          end
        end
      end
    end
  end
end

# The service_url method does appear to work with the active storage "disk" service
# This monkey patch is applied in development and test only
if %w[development test].include?(Rails.env)
  ActiveSupport.on_load(:active_storage_blob) do
    prepend(CoreExtensions::ActiveStorage::Blob::ServiceUrl)
  end
end

[zzak]

@intrip I think just inheriting the application url would help yes. Alternatively attachment.url should not raise an exception if ActiveStorage::Current.host is nil but a warning instead? 🤷

[intrip]

@intrip I think just inheriting the application url would help yes. Alternatively attachment.url should not raise an exception if ActiveStorage::Current.host is nil but a warning instead? 🤷

@zzak good point, I'd start with a PR which logs a warning and returns nil when .url is not set, ok for you?

[zzak]

@intrip One more question before you start any work, would returning nil have other consequences like on peoples test suites because of the behavior change? 🤔

[intrip]

@zzak I've just found #41996, shouldn't it be expressive enough for now?

[zzak]

@intrip Ah yeah, I totally missed that. Good find!

[ryankopf]

For those searching for this error from Google, note the proper usage in views is to use "url_for" such as:

url_for(profile.image)

and not

profile.image.url

[jepzen]

I fixed my rspec by adding

include ActiveStorage::SetCurrent

to my controller. Non of the other suggesions did anything

[manhcuongdtbk]

Here's the handy link to the api doc in case anybody wants further research: https://api.rubyonrails.org/v7.0.2.2/classes/ActiveStorage/SetCurrent.html

Sets the ActiveStorage::Current.url_options attribute, which the disk service uses to generate URLs. Include this concern in custom controllers that call ActiveStorage::Blob#url, ActiveStorage::Variant#url, or ActiveStorage::Preview#url so the disk service can generate URLs using the same host, protocol, and port as the current request.

[skalee]

It would be ideal if we could set this value in our config, similar to how we can set url options for ActionMailer:

config.action_mailer.default_url_options = { host: 'localhost:3000' }

I believe this is a great suggestion. Especially that config.action_mailer.default_url_options stays despite of #39566 (adding Rails.application.url) as far as I understand?

There's even a PR #42847 which adds exactly that, sadly never merged in (I can rebase and double-check it if this is an issue).

[skalee]

Another clean solution is to implement a custom Active Storage service, extending ActiveStorage::Service::DiskService, and overriding #url_options. For instance:

# lib/active_storage/service/disk_with_host_service.rb

require "active_storage/service/disk_service"

class ActiveStorage::Service::DiskWithHostService < ActiveStorage::Service::DiskService
  def url_options
    Rails.application.default_url_options # or something else for greater flexibility
  end
end

# config/storage.yml

local:
  service: DiskWithHost
  root: <%= Rails.root.join("storage") %>

I believe it is usually okay to do this in test or development environments.

[lapser]

Another possible solution at https://stackoverflow.com/a/76079790/

[bill-transue]

2023 and this is still a problem. 😡

[rafaelfranca]

The issue is closed because this isn't a problem. If you are seeing this error you are probably missing this https://api.rubyonrails.org/v7.0.2.2/classes/ActiveStorage/SetCurrent.html

[hunterbmiller]

The issue is closed because this isn't a problem. If you are seeing this error you are probably missing this https://api.rubyonrails.org/v7.0.2.2/classes/ActiveStorage/SetCurrent.html

Unfortunately this solution only works when generating urls inside of your controllers. I've just gotten accustomed to setting ActiveStorage::Current.url_options manually in those situations.

I still feel like this is something we should be able to set inside of our configs as mentioned above, but I understand its a bit of a niche situation.

[dotgrid]

For those searching for this error from Google, note the proper usage in views is to use "url_for" such as:

url_for(profile.image)

and not

profile.image.url

This fixed it for me! Thank you so much!

[prototype2010]

For those searching for this error from Google, note the proper usage in views is to use "url_for" such as:

url_for(profile.image)

and not

profile.image.url

man you have just saved my evening !

[danielricecodes]

# lib/active_storage/service/disk_with_host_service.rb

require "active_storage/service/disk_service"

class ActiveStorage::Service::DiskWithHostService < ActiveStorage::Service::DiskService
  def url_options
    Rails.application.default_url_options # or something else for greater flexibility
  end
end

This actually works and lets my RSpec test suite pass.

@rafaelfranca - this really should not be necessary. There is absolutely no documentation on this hidden "feature" in ActiveStorage. Could you please prescribe how one is supposed to set ActiveStorage::Current.url_options without resorting to the above? I'm happy to even update the Rails Guide for Active Storage or issue a Pull Request for the default Disk Service file that incorporates the above.

Note: none of the solutions below worked in my Rails 7.0.5 application. In my opinion, these solutions are better and more intuitive than creating a custom Disk Service so why wouldn't they work in the test environment?

In test.rb environment file

  # Store uploaded files on the local file system in a temporary directory.
  config.active_storage.service = :test
  config.active_storage.url_options = { host: 'www.example.com' }

In rails_helper.rb - RSpec configuration file:

RSpec.configure do |config|
  config.before(:each) do
    ActiveStorage::Current.url_options = {host: 'https://example.com'}
  end
end

[santib]

Hey all, I think I now understand why setting ActiveStorage::Current in the tests doesn't always work and it's because Rails is clearing ActiveStorage::Current. In my case it only happens with RSpec request specs when after performing the HTTP request I have an assertion like expect(something.file.url).to eq(something_else)

Not sure if this happens just in RSpec or in Minitest as well.

I'm wondering why we are calling reset_all just like that instead of:

1. in to_run: storing in memory the attributes, and then reset_all
2. in to_complete: reset_all, and loading the previously stored attributes.

Does this make sense or am I missing something?

Anyways I got it fixed by doing this:

# spec/rails_helper.rb

Rails.application.executor.to_complete do
  ActiveStorage::Current.url_options = { host: ENV.fetch('SERVER_HOST', nil),
                                         port: ENV.fetch('PORT', 3000) }
end

[santib]

I'll leave here a reproduction script. @rafaelfranca If you agree this is something we should fix, I'm willing to help.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "rails", "~> 7.0.0"
  gem "sqlite3"
end

require "active_record/railtie"
require "active_storage/engine"
require "tmpdir"

class TestApp < Rails::Application
  config.root = __dir__
  config.hosts << "example.org"
  config.eager_load = false
  config.session_store :cookie_store, key: "cookie_store_key"
  config.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  config.active_storage.service = :local
  config.active_storage.service_configurations = {
    local: {
      root: Dir.tmpdir,
      service: "Disk"
    }
  }

  routes.draw do
    get "/" => "test#index"
  end
end

class TestController < ActionController::Base
  include Rails.application.routes.url_helpers

  def index
    render plain: "Home"
  end
end

ENV["DATABASE_URL"] = "sqlite3::memory:"

Rails.application.initialize!

require "minitest/autorun"

class BugTest < Minitest::Test
  include Rack::Test::Methods

  def test_doesnt_clear_active_storage_current
    ActiveStorage::Current.url_options = { host: 'localhost', port: '3000' }
    get "/" 
    assert_equal({ host: 'localhost', port: '3000' }, ActiveStorage::Current.url_options)
  end

  private

  def app
    Rails.application
  end
end

[santib]

After giving it some thought I think the correct solution would be just replacing ActiveStorage::Current which is a per-thread config, for a global config in the DiskService definition (it only applies here). Something like:

# config/storage.yml

local:
  service: Disk
  root: <%= Rails.root.join('storage') %>
  url_options:
    protocol: 'http'
    host: 'localhost'
    port: '3000'
# ...

thoughts?

[JDrizzy]

I encountered this issue when using local disk for tests only. Development and production both use S3.

To resolve it, instead of including module ActiveStorage::SetCurrent in ApplicationController - which wasn't needed for dev and prod. I've used the lazy load hook on_load in file test_helper.rb, including the module in action_controller e.g.

# Sets ActiveStorage::Current.host for tests using local disk
ActiveSupport.on_load :action_controller do
  include ActiveStorage::SetCurrent
end

Please let me know if this is not advised. Thanks

[mikehowson]

Another clean solution is to implement a custom Active Storage service, extending ActiveStorage::Service::DiskService, and overriding #url_options. For instance:

# lib/active_storage/service/disk_with_host_service.rb

require "active_storage/service/disk_service"

class ActiveStorage::Service::DiskWithHostService < ActiveStorage::Service::DiskService
  def url_options
    Rails.application.default_url_options # or something else for greater flexibility
  end
end

# config/storage.yml

local:
  service: DiskWithHost
  root: <%= Rails.root.join("storage") %>

I believe it is usually okay to do this in test or development environments.

This fixed it from within rspec for me