FailureApps script_name: nil breaks route generation within mounted engines

[jvdp]

Since upgrading to Devise 3.5.2, the Devise::FailureApp from within a Rails engine no longer generates routes with the path it is mounted under.

The breaking change was made in aa675f7, which always passes a script_name (possibly nil) to the url generator. This is problematic because the mount point path is only computed when script_name is not passed to options: https://github.com/rails/rails/blob/v4.2.3/actionpack/lib/action_dispatch/routing/mapper.rb#L631

I'm not sure how to fix this (this is all crazy complicated code), but perhaps there's a better fix for #3643 than forcing path_name to be passed to url_for.

[josevalim]

Which Rails version are you using?

[jvdp]

Rails 4.2.3.

[josevalim]

@jvdp so apparently if we don't set it, there is a infinite loop. If we set it, we break engines. :(

[jvdp]

Yep, it sucks. Do you think this should be fixed in Rails instead?

[mmitchellg5]

We're running into this as well, with Rails 4.2.4

[variousred]

FWIW as a test I deleted this whole bit:

      # Rails 4.2 goes into an infinite loop if opts[:script_name] is unset
      if (Rails::VERSION::MAJOR >= 4) && (Rails::VERSION::MINOR >= 2)
        opts[:script_name] = (config.relative_url_root if config.respond_to?(:relative_url_root))
      else
        if config.respond_to?(:relative_url_root) && config.relative_url_root.present?
          opts[:script_name] = config.relative_url_root
        end
      end

and I did NOT get an infinite loop, everything worked.

[mmitchellg5]

@jvdp @josevalim

[jvdp]

@mmitchellg5 hey if that fixes it, great, but doesn't that break a configuration option? If you can make a PR / fork I'll try it on my app.

For completeness I think #3738 is a duplicate of this (and #3739 a proposed fix, but by adding instead of removing code.)

[variousred]

@jvdp sure does, but you can just delete the conditional since apparently rails 4.2.4 does NOT go into an infinite loop without the setting.

[stanhu]

I think #3738 may be similar but not exactly the same. I just tried disabling the setting of script_name, and it didn't solve my use case. Warden passes the entire fullpath, leaving it to the callback function to sort things out.

[kont-noor]

@variousred Also no infinite loop issue in Rails 4.2.3
UPD: also nor in any of the versions from 4.2.0 to 4.2.4

Can anything else be the reason of the bug? (Probably some another modules)

[cipater]

@josevalim I've submitted PR #3807 that attempts to fix both #3643 and bug it caused above.

Despite suggestions to the contrary, there is definitely a infinite loop in cases where mounted engines change the SCRIPT_NAME env variable to be something other than intended path for a given scope. The '/admin/sidekiq' example in #3643 is a perfect example. I don't know enough about the internals, but it would seem something in 4.2 changed to take the SCRIPT_NAME env variable into account in the absence of a :script_name option being passed in.

[josevalim]

Awesome, thanks ofr the PR!

[lucasmazza]

Merged #4081 with a fix for the redirection issue - closing this for now.