impresscms icon indicating copy to clipboard operation
impresscms copied to clipboard

Add default escape characters to icms_core_DataFilter::addSlashes for backwards compatibility

Open Copilot opened this issue 2 months ago • 5 comments

User description

icms_core_DataFilter::addSlashes() was calling addcslashes() without default characters, causing it to escape nothing when invoked without the second parameter. This broke SQL queries with single quotes, triggered false Protector SQL injection warnings, and caused unexpected character conversions during save operations.

Changes

  • Added DEFAULT_ESCAPE_CHARS constant containing ', ", \, and \0 (NUL byte) to match PHP's addslashes() behavior
  • Modified addSlashes() to use the constant when $param is null, providing backwards compatibility while preserving the ability to specify custom character lists
  • Updated documentation to clarify default behavior and remove obsolete magic_quotes_gpc references

Example

// Before: No escaping occurred, causing SQL syntax errors
icms_core_DataFilter::addSlashes("O'Brien's content");
// => "O'Brien's content" (unchanged - breaks SQL)

// After: Properly escapes like addslashes()
icms_core_DataFilter::addSlashes("O'Brien's content");
// => "O\'Brien\'s content" (SQL-safe)

// Custom character lists still work
icms_core_DataFilter::addSlashes("test_value", "_");
// => "test\_value"
Original prompt

This section details on the original issue you should resolve

<issue_title>Add defaults to icms_core_Datafilter::addSlashes for backwards compatibility</issue_title> <issue_description>After troubleshooting an upgrade, I discovered the difficulty was caused by icms_core_DataFilter::addSlashes not having any default characters for addcslashes() to produce the same output as addslashes() would - it escapes single quote, double quote, backslash, and NUL.

What happened was Protector would stop the save and say it detected an SQL injection. With Protector disabled, I was able to save, but several records weren't updated, because SQL complained about the syntax. The content being saved had a single quote in it. I also encountered some characters being converted. </issue_description>

Comments on the Issue (you are @copilot in this section)

  • Fixes ImpressCMS/impresscms#1674

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.


PR Type

Bug fix, Enhancement


Description

  • Add DEFAULT_ESCAPE_CHARS constant matching PHP's addslashes() behavior

  • Fix addSlashes() to escape single quotes, double quotes, backslashes, NUL bytes by default

  • Improve method documentation and remove obsolete magic_quotes_gpc references

  • Maintain backwards compatibility while allowing custom character escaping


Diagram Walkthrough

flowchart LR
  A["addSlashes called<br/>without param"] -->|"param === null"| B["Use DEFAULT_ESCAPE_CHARS<br/>constant"]
  B -->|"Contains: ', \", \\, \\0"| C["addcslashes escapes<br/>properly"]
  C -->|"Returns"| D["SQL-safe escaped text"]
  E["addSlashes called<br/>with custom param"] -->|"param provided"| C

File Walkthrough

Relevant files
Bug fix
DataFilter.php
Add default escape characters constant and fix addSlashes method

htdocs/libraries/icms/core/DataFilter.php

  • Added DEFAULT_ESCAPE_CHARS class constant containing single quote,
    double quote, backslash, and NUL byte characters
  • Modified addSlashes() method to use DEFAULT_ESCAPE_CHARS when $param
    is null
  • Updated method documentation to clarify default behavior and remove
    obsolete magic_quotes_gpc references
  • Improved code comments explaining backwards compatibility with PHP's
    addslashes()
+20/-4   

Copilot avatar Dec 02 '25 08:12 Copilot