Add new unit tests and update filename validation

MarjovanLier commented 4 months ago

User description

New unit tests for EnableXhprof and ProfilesDir functions have been added. In parallel, the file reading function in Trace has been modified to validate the filename using htmlspecialchars(), ensuring it's a valid file before proceeding. Namespace in IsExcludedClassTest has also been updated.

Type

tests, enhancement

Description

Enhanced filename validation in Trace.php to prevent processing invalid file names.
Added unit tests for enableXhprof and setProfilesDir methods to ensure functionality works as expected.
Corrected the namespace in IsExcludedClassTest to maintain consistency across tests.

Changes walkthrough

Relevant files

Enhancement

Trace.php

Enhance Filename Validation in Trace File Reading

src/Trace.php

Filename validation added using htmlspecialchars.

Checks if filename is not empty and is a file before reading its
contents.

+8/-1

Tests

EnableXhprofTest.php `Add Unit Test for EnableXhprof Method` tests/Unit/EnableXhprofTest.php New unit test for `Trace::enableXhprof` method. Test ensures no errors occur during method execution.	+25/-0
IsExcludedClassTest.php `Namespace Correction in IsExcludedClassTest` tests/Unit/IsExcludedClassTest.php - Namespace corrected from `tests\Unit` to `Unit`.	+1/-1
ProfilesDirTest.php `Add Unit Test for Profiles Directory Setting` tests/Unit/ProfilesDirTest.php New unit test for setting profiles directory in `Trace::setProfilesDir`. Test verifies the profiles directory is set correctly using reflection.	+32/-0

✨ PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

Summary by CodeRabbit

Security Improvements
- Enhanced security by sanitizing file names in the trace report generation process.
Tests
- Added tests for enabling Xhprof profiling and setting the profiles directory in the Trace class.

coderabbitai[bot] commented 4 months ago

Walkthrough

## Walkthrough The recent updates emphasize security enhancements and testing improvements in the codebase. A notable security enhancement involves sanitizing the `filename` parameter in `Trace.php`. Furthermore, new unit tests have been introduced for the `Trace` class, specifically targeting the `enableXhprof` method and the functionality related to setting the profiles directory. These changes aim to bolster security measures and ensure the robustness of the codebase. ## Changes | File | Summary | |------------------------------|-----------------------------------------------------------------------------------------------| | `src/Trace.php` | Sanitizes `filename` using `htmlspecialchars` for security and validation. | | `tests/Unit/EnableXhprofTest.php`
`tests/Unit/ProfilesDirTest.php` | Adds unit tests for `enableXhprof` method and setting the profiles directory in `Trace` class. |

Tips

### Chat There are 3 ways to chat with CodeRabbit: - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit-tests for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit tests for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit tests.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - The JSON schema for the configuration file is available [here](https://coderabbit.ai/integrations/coderabbit-overrides.v2.json). - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json` ### CodeRabbit Discord Community Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback.

sweep-ai[bot] commented 4 months ago

Apply Sweep Rules to your PR?

[ ] Apply: All new business logic should have corresponding unit tests.
[ ] Apply: Refactor large functions to be more modular.
[ ] Apply: Add docstrings to all functions and file headers.

This is an automated message generated by Sweep AI.

sweep-ai[bot] commented 4 months ago

Fixing PR: track the progress here.

I'm currently fixing this PR to address the following:

[Sweep GHA Fix] The GitHub Actions run failed on 689d570 (main) with the following error logs: ``` ```

✨ Created Pull Request: https://github.com/MarjovanLier/XhprofTrace/pull/29

codiumai-pr-agent-pro[bot] commented 4 months ago

PR Description updated to latest commit (https://github.com/MarjovanLier/XhprofTrace/commit/689d570b90e09f2466a2baf4ade6e15ea9a81be4)

[ ] Copy walkthrough table to "Files Changed" Tab

codiumai-pr-agent-pro[bot] commented 4 months ago

Changelog updates:

2024-03-05

Added

New unit tests for enableXhprof and setProfilesDir methods.

Changed

Enhanced filename validation in Trace.php to improve security.
Updated namespace in IsExcludedClassTest for consistency.

to commit the new content to the CHANGELOG.md file, please type: '/update_changelog --pr_update_changelog.push_changelog_changes=true'

codiumai-pr-agent-pro[bot] commented 4 months ago

PR Code Suggestions

Category	Suggestions
Security	Add validation to prevent directory traversal vulnerabilities. ___ Consider validating the `$filename` for directory traversal vulnerabilities. Since you're accepting a filename and using it to read file contents, ensure that the filename cannot be manipulated to access unauthorized files. [src/Trace.php [160]](https://github.com/MarjovanLier/XhprofTrace/pull/28/files#diff-15329b36f4764bd79867bdc947704d9b2afcb82fd7627bd305ae5865be6fb833R160-R160) ```diff -$filename = htmlspecialchars($filename, ENT_QUOTES, 'UTF-8'); +$filename = realpath(htmlspecialchars($filename, ENT_QUOTES, 'UTF-8')); +if (!$filename \|\| !str_starts_with($filename, $expectedDirectory)) { + return []; +} ```
Maintainability	Use early returns to reduce code nesting and improve readability. ___ To improve code readability and reduce nesting, consider using early returns for the condition where `$filename` is empty or not a file. [src/Trace.php [162-166]](https://github.com/MarjovanLier/XhprofTrace/pull/28/files#diff-15329b36f4764bd79867bdc947704d9b2afcb82fd7627bd305ae5865be6fb833R162-R166) ```diff -if (!empty($filename) && is_file($filename)) { - $fileContents = file_get_contents($filename); -} else { +if (empty($filename) \|\| !is_file($filename)) { return []; } +$fileContents = file_get_contents($filename); ```
Maintainability	Ensure consistent namespace declarations across test files. ___ Ensure consistency in namespace declarations across test files. The namespace was changed from `tests\Unit` to `Unit`, which might affect autoloading or test discovery depending on the configuration. [tests/Unit/IsExcludedClassTest.php [5]](https://github.com/MarjovanLier/XhprofTrace/pull/28/files#diff-695aa0f2247ae4d4b8d76563739fb5190e87f9527d2fe1ca160a40bb2fa81dd0R5-R5) ```diff -namespace Unit; +namespace Tests\Unit; ```
Enhancement	Add assertions to the test case to verify expected behavior. ___ The test case `test_enable_xhprof_with_no_errors` does not assert any behavior. Consider adding assertions to verify the expected behavior of `Trace::enableXhprof()`. [tests/Unit/EnableXhprofTest.php [23]](https://github.com/MarjovanLier/XhprofTrace/pull/28/files#diff-e61fbf200dbcc4e83dfaa9d57c590e6d22cb3cf8e18e5d55cf510d9c53f3e4aeR23-R23) ```diff -$this->expectNotToPerformAssertions(); +// Example assertion +$this->assertTrue(function_exists('xhprof_enable'), 'xhprof_enable function should exist after enabling Xhprof.'); ```
Best practice	Clean up or reset changes after the test to avoid side effects. ___ Consider cleaning up the temporary directory or resetting the `profilesDir` property after the test to avoid side effects on other tests. [tests/Unit/ProfilesDirTest.php [30]](https://github.com/MarjovanLier/XhprofTrace/pull/28/files#diff-f1896be17f3534975a72894a5ef562327081232687331acfc3f7b9cdce221b66R30-R30) ```diff $this->assertEquals($tempDir, $property->getValue()); +// Cleanup or reset code +$property->setValue(null); // Reset to default or initial value ```