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

fix: esc() for 'raw' context #8633

Merged
merged 1 commit into from
Mar 22, 2024
Merged

fix: esc() for 'raw' context #8633

merged 1 commit into from
Mar 22, 2024

Conversation

Cleric-K
Copy link
Contributor

@Cleric-K Cleric-K commented Mar 18, 2024

Description
Fixes #8624

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis changed the title esc() for 'raw' context (Fixes #8624) fix: esc() for 'raw' context Mar 18, 2024
@kenjis
Copy link
Member

kenjis commented Mar 18, 2024

@Cleric-K
Copy link
Contributor Author

Please sign all commits. See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/signing.md

Sorry, I saw the requirement after PRing. It should be OK now.

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 18, 2024
system/Common.php Outdated Show resolved Hide resolved
Copy link
Collaborator

@ddevsr ddevsr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add tests for data array? recursive and non-recursive

@ddevsr ddevsr added the tests needed Pull requests that need tests label Mar 19, 2024
@kenjis
Copy link
Member

kenjis commented Mar 19, 2024

The following php-cs-fixer error is not related to this PR.

Files that were not fixed due to errors reported during fixing:
   1) /home/runner/work/CodeIgniter4/CodeIgniter4/app/Config/Logger.php

https://github.com/codeigniter4/CodeIgniter4/actions/runs/8327332402/job/22819194579?pr=8633

I sent a bug report: PHP-CS-Fixer/PHP-CS-Fixer#7896

@Cleric-K
Copy link
Contributor Author

Can you add tests for data array? recursive and non-recursive

Can you point me to an existing test for a function that I can use as an example? I'm not familiar with the CI project structure.

@kenjis
Copy link
Member

kenjis commented Mar 19, 2024

See

public function testEscapeWithDifferentEncodings(): void
{
$this->assertSame('&lt;x', esc('<x', 'html', 'utf-8'));
$this->assertSame('&lt;x', esc('<x', 'html', 'iso-8859-1'));
$this->assertSame('&lt;x', esc('<x', 'html', 'windows-1251'));
}
public function testEscapeBadContext(): void
{
$this->expectException('InvalidArgumentException');
esc(['width' => '800', 'height' => '600'], 'bogus');
}
public function testEscapeBadContextZero(): void
{
$this->expectException('InvalidArgumentException');
esc('<script>', '0');
}

@Cleric-K
Copy link
Contributor Author

I added two tests. I don't have much experience with unit tests so please tell me if it seems right.

@kenjis kenjis removed the tests needed Pull requests that need tests label Mar 20, 2024
@kenjis
Copy link
Member

kenjis commented Mar 20, 2024

@Cleric-K Please fix the coding style.
composer cs-fix will fix the errors.
See https://github.com/codeigniter4/CodeIgniter4/actions/runs/8343103783/job/22862006485?pr=8633

Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kenjis kenjis merged commit da4861c into codeigniter4:develop Mar 22, 2024
62 checks passed
@kenjis
Copy link
Member

kenjis commented Mar 22, 2024

@Cleric-K Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: esc(..., 'raw') fails on recursive data structures
3 participants