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

[5.x]: New headersFilter in app.web.php applying to Control Panel requests and not Site requests #15495

Closed
jamesmacwhite opened this issue Aug 7, 2024 · 6 comments · Fixed by #15499
Labels

Comments

@jamesmacwhite
Copy link
Contributor

jamesmacwhite commented Aug 7, 2024

What happened?

Description

The new feature in #15397 doesn't appear to be working correctly. I tried applying a as headersFilter in the app.web.php but this applied to Control Panel requests not site Requests.

Example config used in app.web.php:

// Attach the headers filter to the application
'as headersFilter' => [
    'class' => \craft\filters\Headers::class,
    'site' => 1,
    'headers' => [
        'X-Test-Header' => 'Test Value',
    ]
]

For a site request:

image

For a CP request:

image

Steps to reproduce

  1. Add as headersFilter example in Configurable CORS and site headers #15397
  2. Inspect response headers of a site request (value is not present)
  3. Inspect response of CP request e.g. /admin (value is present)

Expected behavior

Headers to be present on site requests only.

Actual behavior

Site requests do not have any headers added, but are appearing on Control Panel requests which is the reverse of intended functionality.

Craft CMS version

5.3.0.3

PHP version

8.3

Operating system and version

No response

Database type and version

MySQL

Image driver and version

No response

Installed plugins and versions

No response

@jamesmacwhite
Copy link
Contributor Author

@timkelty For reference regarding the new headersFilter in 5.3

@timkelty
Copy link
Contributor

timkelty commented Aug 7, 2024

Looking now, @jamesmacwhite

@timkelty
Copy link
Contributor

timkelty commented Aug 7, 2024

@jamesmacwhite Thanks for the report, should be fixed here: #15499

Amazing what difference a ! can make. We'll try and get a release out ASAP.

@jamesmacwhite
Copy link
Contributor Author

Happens to us all! Thanks for looking into it and proposing a quick patch. I'm currently use the request component to set headers on site requests only currently, but once a new release is made, I'll transition over.

@brandonkelly
Copy link
Member

Craft 4.11.1 and 5.3.1 are out with that fix. Thanks again for reporting!

@jamesmacwhite
Copy link
Contributor Author

No worries. Thanks for the quick fix @timkelty and release @brandonkelly.

It's great adding front end HTTP headers is consistent for Craft CMS apps now.

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

Successfully merging a pull request may close this issue.

3 participants