Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

url helper not picking the base root from the config setting #17385

Closed
amosmos opened this issue Jan 18, 2017 · 8 comments
Closed

url helper not picking the base root from the config setting #17385

amosmos opened this issue Jan 18, 2017 · 8 comments

Comments

@amosmos
Copy link

amosmos commented Jan 18, 2017

  • Laravel Version: 5.3.22
  • PHP Version: 7.0
  • Database Driver & Version: N/A

Description:

When using the url helper, the root is calculated by symfony and not picked up according to the APP_URL env variable.

I found out about it when I was trying to use the url helper in an email notification which was sent based on an eloquent event listener (placed in a service provider) while running the app on homestead. The helper didn't pick up the 8000 port. After checking it I noticed the helper is using the request root function which is calculating the root based on a prepareBaseUrl symfony function (look here: https://github.com/laravel/framework/blob/5.3/src/Illuminate/Http/Request.php#L87, while the getBaseUrl is calling prepareBaseUrl), and that function somehow miss the port number in this specific call (from a service provider). So even when I manually set the 8000 port on the APP_URL env variable, nothing changed.

Note - when the email notification was called regularly from a controller, the port was received correctly. It only happened to me when the notification was called from a service provider.
Also, I believe on production there shouldn't be a problem as you don't need a special port like when using homestead.

Steps To Reproduce:

  1. Create a laravel app locally on homestead.
  2. Create a mail notification and place a url() helper call in it to some link or asset.
  3. Create an eloquent event listener in a service provider (for example appServiceProvider (for example a listen to a model creation event) which then sends the email notification.
  4. Create that model to invoke the listener and the notification sending.
  5. Look at the returned URL created with the url helper and see there is no 8000 port.
@themsaid
Copy link
Member

The base URL is auto detected, it's not meant to be picked from the config file.

I'm using valet now but I don't think I ever had any problem with ports when I was on Homestead, it used to pick it up with no problem, it's also strange that it works from within a controller but not a listener.

@GrahamCampbell
Copy link
Member

I guess this is a queued listener. Fix will be to make it not queued, and only the queue sending part, after the content has been computed. If you're rendering a view to do that, you'll need to upgrade to L5.4, because it renders the view before queuing, unlike 5.3, (I think, you'll need to check this in the laravel source code).

@amosmos
Copy link
Author

amosmos commented Jan 19, 2017

@GrahamCampbell nope, not queued. So it's a different bug. Try to reproduce and you'll see it too.

@themsaid
Copy link
Member

@amosmos any progress on this?

@amosmos
Copy link
Author

amosmos commented Jan 30, 2017

No, nothing really changed... The route() function is based on symphony's prepareBaseUrl which just misses the port when the call is from a service provider. You can just reproduce it and see it too... I don't know how to fix this...

@mtx-z
Copy link

mtx-z commented Mar 20, 2017

Can you try what i posted #14139 ?

@aik099
Copy link
Contributor

aik099 commented Aug 18, 2017

@amosmos , alternatively you can replace \Illuminate\Foundation\Bootstrap\SetRequestForConsole class and change bootstrap method to be as in #14139 (comment) comment of mine.

@ronnyandre
Copy link

Anyone managed to fix this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants