rainlab / pages-plugin

Adds static pages and menus
MIT License
121 stars 99 forks source link

Safe mode preventing static pages from saving #174

Closed hybridvision closed 5 years ago

hybridvision commented 8 years ago

After recently updating a site to the latest build (October Build 345 + Static Pages Plugin v1.2.6), some pages refuse to save, giving a safe mode error like this:

image

As I understand it, the safe mode feature was added to stop PHP being added to the code section of a CMS page. However, these are static pages with no PHP code section so it seems like a bug that it should trigger the exception.

So far I haven't been able to figure out any pattern to make it trigger but it won't allow some pages to save after editing while others are fine. When I set cms.enableSafeMode to false, all pages can save normally.

I don't have any more information to add at this point but hopefully it's enough to find the source of the problem...

fresswolf commented 8 years ago

I'm having this issue only in placeholder tabs. The main content tab isn't affected

henrikr commented 7 years ago

@hybridvision @fresswolf were you able to find a workaround or solution to this issue? Disabling safe mode in production doesn't seem like a great idea

One method was to replace placeholder with a variable but this limits you to only adding basic content and not snippets/components to sections in your layout.

jan-vince commented 7 years ago

Just a quick workaround - am using this to be able to edit PHP code when I am on the road:

In .htaccess I have:

<IfModule mod_setenvif.c>
    ## localhost
    SetEnvIf Remote_Addr "^127.0.0.1$" APP_ENV="dev". # DEV mode for localhost
    SetEnvIf Remote_Addr "^::1$" APP_ENV="dev"
    ## secure office IP
    SetEnvIf Remote_Addr "^111.222.333.444$" APP_ENV="dev". # connections from this IP will run web in DEV mode
</IfModule>

And I have /config/dev folder in OctoberCMS where I copied app.php file with 'debug' => true.

When I access my site from the IP, I can edit all data as I am in DEV mode. All other visitors see web in production mode.

I am using VPN to my home, where I have static IP.

hybridvision commented 7 years ago

@HenrikR - it has been more than a year since I worked on this project so I can't remember exactly but I don't recall finding a solution. In my case, it was ok to disable the safe mode so that's what I ended up doing because I couldn't spend any more time on it.

It would be nice if this bug was fixed but I think @jan-vince's solution is a good workaround until then...

henrikr commented 7 years ago

@jan-vince @hybridvision thanks for your responses! Your solution is assuming you need to enable it to simply manage content in the placeholder section yourself; whereas if you provide the static pages plugin to your client, they will not have the ability to change the content in those placeholders.

As a simple example, my client would like to update their homepage sidebar each week with a custom message, an image or place an ad snippet; unfortunately they will not be able to update the sidebar as Safe Mode is enabled on their site in production to avoid this.

In the case above, I as an admin would need to login and do so for them as I have it disabled — this kind of defeats the purpose of having predefined "placeholder" sections in a layout.

jan-vince commented 7 years ago

Sure - but maybe there is another way how to handle what you need.

I use custom page fields for almost every client's editable content - so maybe you can use them instead of placeholders.

vanmil commented 7 years ago

@jan-vince Using custom page fields works in many cases, but not in all. For us the main drawback of page fields in comparison to placeholders is that page fields don't accept snippets, where placeholder do accept them. Using Snippets makes the Static Pages plugin so much more useful for the editor user.

jan-vince commented 7 years ago

I know about it and I had a discussion about it earlier - but it won't be hard to allow snippets to work inside of custom fields. Just no one (me as well) had a time to add this feature yet...

LukeTowers commented 7 years ago

@jan-vince @vanmil If someone could dig deeper into the issue of why editing placeholder content is disabled when safe mode is enabled and provide a fix in a PR, that would be appreciated.

chocolata commented 6 years ago

Hi, please note that the same message pops up upon trying to edit the fields in the "meta" tab (meta title and meta description). I'm on version 1.2.18 of the Static Pages plugin, and build 434 of October CMS.

LukeTowers commented 6 years ago

@maartenmachiels could you dig deeper and see what is causing the issue?

chocolata commented 6 years ago

Hi Luke, dealing with a deadline now, but I'd be glad to start looking from the end of next week.

chocolata commented 6 years ago

Hi Luke, as promised, I've done some more investigation. Saving a default Static Page with only a content field works perfectly. Using variables like text, richeditor, repeater all work fine. As far as I can see, the message "Safe mode is currently enabled" is only triggeed when there's a placeholder present. So whenever we use a layout in which a placeholder exists, the error shows up:

{% placeholder sidebox title="Sidebar" %}

The message disappears when we disable safe mode or enable debugging mode.

I cannot seem to find any occurrence of enableSafeMode in the Static Pages plugin, it's a bit above my knowledge I'm afraid. At the moment we can just disable safe mode, but it would be nice if this could get fixed in future.

What do you think?

LukeTowers commented 6 years ago

This would be happening because of something within the CMS module. RainLab.Pages extends upon the CMS, so the enableSafeMode would be used in modules/cms. It's intended to prevent the modification of code sections for CMS pages if that helps you narrow it down.

bennothommo commented 5 years ago

This issue still persists as of October build 451.

bennothommo commented 5 years ago

@maartenmachiels I have taken a look into this further. The crux of the issue is that any content that is added to a placeholder is then saved into the "code" portion of the static page object by this method. When safe mode is enabled, no modifications can be made to the "code" portion of any CMS object, which is why the safe mode error appears.

@LukeTowers We could potentially disable safe mode checks for Static Page objects. Any thoughts on this? The other alternative is that we'd have to disable adding placeholder content when safe mode is enabled.

bennothommo commented 5 years ago

I have gone ahead and removed safe mode checks just for Static Page objects as of 6b6b06133aff3b70cc95f52d70eb259f14db8439.

The only way the "code" attribute of a Static Page is modified is through the use of placeholders, as far as I can tell. Placeholders are rendered as rich editors in the UI, making it less likely for dangerous code to be inserted - in addition, placeholders are rendered as Twig code blocks in the actual file content upon save, which is already sanitised for PHP code anyway, so there's no need for a safe mode check here.

chocolata commented 5 years ago

Hi @bennothommo thanks so much for your nice work. Great to see that October is being managed so well!