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

A few code issues #5

Open
Dimasmagadan opened this issue Feb 6, 2023 · 0 comments
Open

A few code issues #5

Dimasmagadan opened this issue Feb 6, 2023 · 0 comments

Comments

@Dimasmagadan
Copy link

Dimasmagadan commented Feb 6, 2023

https://github.com/ddev-app/feditor/blob/9f7bc988e308f020d4e31c6cf8c6f3175a24fa80/feditor.php#L13
I'd suggest not to use glob and require_once in a loop, cause it can cause several issues:

  • Security: The directory contents are not verified, any malicious files can be included and potentially harm the system.
  • Performance: require_once is slower compared to other file inclusion methods and reading all files through a loop can slow down the application, especially if the directory has many files. Better to use autoloader or require with a conditional check like if_function_exists() inside
  • Maintainability: more files are longer to read and harder to maintain. But could be ok for a small project

https://github.com/ddev-app/feditor/blob/9f7bc988e308f020d4e31c6cf8c6f3175a24fa80/feditor.php#L35
An ob_start inside a WordPress shortcode could lead to a few issues:

  • it can cause unexpected behavior: If a shortcode starts an output buffer, any output generated by other plugins or themes that runs before the shortcode's buffer is flushed will be captured and discarded, leading to unexpected behavior.
  • Buffering can cause unexpected nesting: If multiple shortcodes start output buffers, the buffers can become nested, causing problems when trying to flush them in the correct order.
  • Buffering can cause memory leaks: Output buffering can cause memory leaks if the buffer is not properly flushed, causing the memory usage of the application to increase over time.

https://github.com/ddev-app/feditor/blob/9f7bc988e308f020d4e31c6cf8c6f3175a24fa80/feditor.php#L53
intval() would return 0 for empty value, no need to for an extra empty() check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant