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

[4.x]: UrlHelper::actionUrl returning wrong host #14440

Closed
soerenmeier opened this issue Feb 21, 2024 · 10 comments
Closed

[4.x]: UrlHelper::actionUrl returning wrong host #14440

soerenmeier opened this issue Feb 21, 2024 · 10 comments
Assignees

Comments

@soerenmeier
Copy link

What happened?

Description

I'm working on a headless commerce shop and using a custom payment gateway. In that gateway i use the function UrlHelper::actionUrl for the return url.

Steps to reproduce

  1. configure craft to omitScriptNameInUrls but don't change the pathParam.
  2. set headlessMode=true
  3. Call UrlHelper::actionUrl from a non cp url in my case /actions/commerce/payments/pay

Expected behavior

Returns a url with the host pointing to `@web´.

Actual behavior

Returns an url with the host being the site.

I think the issue lies here:

} else {
$baseUrl = static::host() . $request->getScriptUrl();
}

cpuUrl gets set by actionUrl to true because headlessMode is active but then get's ignored in the call to static::host.

Temporary fix

set pathParam to null

Craft CMS version

Craft Pro 4.7.3

PHP version

8.2

Operating system and version

Darwin 23.2.0

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

No response

@brandonkelly
Copy link
Member

  • What is @web set to?
  • What is the full URL that you are requesting, including the hostname?

actionUrl() is expected to return a URL based on the current request’s host name.

@soerenmeier
Copy link
Author

@web is set to: http://my.domain/
site is set to : http://localhost:8080/

The request is made to:
POST http://my.domain/actions/commerce/payments/pay
with Accept: 'application/json'

The generated url is: http://localhost:8080/index.php?p=admin/actions/commerce/payments/complete-payment&commerceTransactionHash=hash

@brandonkelly
Copy link
Member

Go to http://my.domain/admin/utilities/php-info and search for $_SERVER['SERVER_NAME']. What value is displayed?

@soerenmeier
Copy link
Author

SERVER_NAME is set to my.domain.

@brandonkelly brandonkelly self-assigned this Feb 28, 2024
@brandonkelly
Copy link
Member

Thanks! Was able to reproduce, and agree it’s not expected behavior. Fixed for the next release.

@brandonkelly
Copy link
Member

4.8.1 is out now with that fix.

@mihob
Copy link

mihob commented Mar 7, 2024

@brandonkelly Hey, on my craft site frontend and control panel are running under different domains. www.example.com and cms.example.com

Your change now leads to various URL's pointing to the CP and no longer to the frontend which leads to many problems. (sprig tries to load its components via the CP, commerce payment redirects suddenly run via the control panel etc.)

I have set @web to the CP domain. For example, if I change @web to the frontend domain, the control panel no longer works.

What exactly can I do here? Is this a configuration problem?

@adrum
Copy link

adrum commented Mar 8, 2024

We also have baseCpUrl set to admin.site.test with 'cpTrigger' => null and this change causes all actionUrls on the CP to be pointed at @web instead, and is causing CORS issues from the CP.

It seems like maybe adding bool $useRequestHostInfo = false, to the actionUrl function gives OP the flexibility to decide for themself how it gets utilized. If that is set to default to true, then it seems like src/web/assets/cp/CpAsset.php would need the following in _craftData:

'actionUrl' => UrlHelper::actionUrl(
  useRequestHostInfo: false,
),

@brandonkelly
Copy link
Member

@web should be set to the current base URL. If your site and CP have different base URLs, you should set it dynamically based on the requested host name.

(Or just don't set it at all, and let Craft set it automatically. That’s only a security concern if you are setting a site URL based on it, which you should definitely not be doing if your CP lives on a different hostname.)

@mihob
Copy link

mihob commented Mar 12, 2024

@brandonkelly Thx for your support! That fixed my issues

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

No branches or pull requests

4 participants