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

wp server correct WP_HOME value should be retrieved from $_SERVER HTTP_HOST, not SERVER_NAME #101

Open
lkraav opened this issue Sep 13, 2019 · 8 comments
Labels

Comments

@lkraav
Copy link
Contributor

lkraav commented Sep 13, 2019

Describe the bug

If we run our web server on an alternate port, like 8080, let's say using wp server, then $_SERVER SERVER_NAME will not provide the correct WP_HOME calculation value, but $_SERVER HTTP_HOST does.

print_r( $_SERVER );

Array
(
    ...
    [SERVER_NAME] => localhost
    [SERVER_PORT] => 8080
    [REQUEST_URI] => /
    [HTTP_HOST] => localhost:8080
    [SERVER_ADDR] => 127.0.0.1
    [WP_ENV] => development
    ...
)

Alternatively, $_SERVER SERVER_PORT value maybe also has to be used in WP_HOME auto-calculation.

To Reproduce

  1. Run webserver on a non-standard port, like 8080
  2. Use get_theme_file_uri() to calculate an enqueue asset URL
  3. Watch 404 errors for enqueued assets

Expected behavior
Port value recognized.

@gmazzap
Copy link
Member

gmazzap commented Sep 13, 2019

Even this is true in some cases, the value of $_SERVER widely change depending on the web-server, proxy, etc. So no auto-calculation is really trustable.

Moreover, $_SERVER values that start with HTTP_ can be overridden via headers, meaning that a client can override those (and that could have security implications).

This is why it is usually preferred to avoid them, and rely on values like SERVER_NAME that depends on server configuration.

This is also why is highly suggested to manually set WP_HOME: auto-calculation is not really trustable here.

That said, looking at SERVER_PORT is something we could do, because it is little effort and can be useful.

@lkraav
Copy link
Contributor Author

lkraav commented Sep 14, 2019

I agree, patching SERVER_PORT is probably the way to go here.

@lkraav
Copy link
Contributor Author

lkraav commented Sep 16, 2019

This issue stems a bit from #100

We could configure our local staging port values dynamically via something like wp server --port 8081, without needing to mod the committed .env template. It's nearly the only thing we need to mod.

@gmazzap
Copy link
Member

gmazzap commented Nov 25, 2021

Please check if 4c76d9d solved it. Thanks.

@lkraav
Copy link
Contributor Author

lkraav commented Feb 4, 2023

Please check if 4c76d9d solved it. Thanks.

I tested with latest dev build today, and something is still going wrong. Without explicitly setting WP_HOME, a ton of errors are output:

Notice: Undefined index: host in wpstarter.git/vendor/wp-cli/server-command/router.php on line 77

I'm serving with wp server --docroot=public.

@gmazzap
Copy link
Member

gmazzap commented Feb 4, 2023

@lkraav, I think you have to pass the URL to WP CL with the --url parameter.

WP Starter is designed to guess the URL in a web server environment.

When you're on CLI, $_SERVER only contains env variables, nothing that can help WP Starter determine the URL correctly.

The best thing you can do is tell WP CLI which URL you want to "emulate".

gmazzap added a commit that referenced this issue Feb 27, 2023
@gmazzap
Copy link
Member

gmazzap commented Feb 27, 2023

@lkraav with 1cb6f60 wp server seems to work for me.

Can you confirm?

@gmazzap gmazzap added the v3 Version 3 label Feb 27, 2023
@gmazzap
Copy link
Member

gmazzap commented Nov 7, 2023

Can I close this?

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

No branches or pull requests

2 participants