X-Frame-Options always set to deny

[louishuyng]

Before opening an issue please make sure you have consulted the Lookbook documentation (in particular the Troubleshooting section) and have checked the existing issues to see if this has already been reported.

Describe the bug

In the line where I debug, there is a condition headers["X-Frame-Options"] == "DENY". To change headers["X-Frame-Options"] back to "SAMEORIGIN"

The condition can not run because the value of headers["X-Frame-Options"] is in lowercase value "deny"

Source code: https://github.dev/ViewComponent/lookbook/blob/main/app/controllers/lookbook/previews_controller.rb

To Reproduce

Steps to reproduce the behavior:

I think we can add some changes like below to prevent this bug happening
From: headers["X-Frame-Options"] == "DENY"
To: headers["X-Frame-Options"].downcase == "deny"

 def permit_framing
    headers["X-Frame-Options"] = Lookbook.config.preview_embeds.policy if embedded?
    headers["X-Frame-Options"] = "SAMEORIGIN" if headers["X-Frame-Options"].downcase == "DENY"
  end

Expected behavior

It should set headers["X-Frame-Options"] to "SAMEORIGIN"

Screenshots

Version numbers

Please complete the following information:

• Lookbook: 2.1.1
• ViewComponent: 2.64.0
• Rails: 7.0.4
• Ruby: 3.2.1

Additional context

Add any other context about the problem here.

[allmarkedup]

Hey @louishuyng, many thanks for this. I think it definitely makes sense to normalize the strings to lower or uppercase before comparing them here.

Would you be happy to open a PR with your suggested change?

[louishuyng]

@allmarkedup here is the PR for that: #554. I guess it just small changes there is enough for fixing this issue

[allmarkedup]

@louishuyng merged now :) Many thanks for your time on this, much appreciated.

[adrienpoly]

@louishuyng I am getting an error now when I upgraded to 2.20

[louishuyng]

@adrienpoly could you help to contribute for that. I believe we just simply check nil for that before calling upcase method

headers["X-Frame-Options"]&.upcase == "DENY"

[adrienpoly]

I will test it, I am pretty sure your proposal should fix it

[adrienpoly]

@louishuyng yes it does fix the issue

[adrienpoly]

I opened #561 to fix that