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

Fix swoole host only configurable via --host parameter + incorrect default port #762

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

xorinzor
Copy link
Contributor

@xorinzor xorinzor commented Oct 15, 2023

Description:

When attempting to configure Swoole to bind on a different host using anything except --host in the commandline, it wouldn't work and always stick to 127.0.0.1.

Additionally, bin/createSwooleServer.php contains a default port value of 8080 despite the command stating the default is 8000.

The issue

  • When you configure the config value octane.host, the $serverState['host'] will stay at 127.0.0.1, and only $serverState['octaneConfig']['host'] changes to the configured value.

The cause

  • src/Commands/StartCommand.php is using a default value of 127.0.0.1 for --host when this argument is not provided in the command. But because src/Commands/Concerns/InteractsWithServers.php's getHost looks at this option first, it will always use the value 127.0.0.1 when you want to configure it using any method except the command line.

The fix

  • In src/Commands/StartCommand.php & src/Commands/StartSwooleCommand.php I removed the default value of 127.0.0.1 for --host as that's already handled by src/Commands/Concerns/InteractsWithServers.php in the getHost() method.

    • the same getHost method is also used with Roadrunner, so I did the same modification for src/Commands/StartRoadRunnerCommand.php.
  • The current default port value is provided via the InteractsWithServers.php Concern using the correct value of 8000 to createSwooleServer. Therefor the modification of 8080 to 8000 in createSwooleServer should have no effect on existing deployments (ie: there should be no way for the createSwooleServer port variable to ever be empty and having to resort to the default).

@taylorotwell taylorotwell merged commit 6dbb1d8 into laravel:2.x Oct 18, 2023
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

Successfully merging this pull request may close these issues.

2 participants