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

refactor: prevent using empty() on CURLRequest #9195

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

ddevsr
Copy link
Collaborator

@ddevsr ddevsr commented Sep 20, 2024

Description
Decrease PHPStan baseline.

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

system/HTTP/CURLRequest.php Outdated Show resolved Hide resolved
@@ -469,7 +469,7 @@ protected function applyMethod(string $method, array $curlOptions): array
*/
protected function applyBody(array $curlOptions = []): array
{
if (! empty($this->body)) {
if ($this->body !== '' && $this->body !== null) {
Copy link
Member

Choose a reason for hiding this comment

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

can we just cast to string then check against empty string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean like this?

$ git diff system/HTTP/CURLRequest.php
diff --git a/system/HTTP/CURLRequest.php b/system/HTTP/CURLRequest.php
index c78f088ff7..40e197abb5 100644
--- a/system/HTTP/CURLRequest.php
+++ b/system/HTTP/CURLRequest.php
@@ -469,8 +469,10 @@ class CURLRequest extends OutgoingRequest        
      */
     protected function applyBody(array $curlOptions = []): array
     {
-        if ($this->body !== '' && $this->body !== null) {
-            $curlOptions[CURLOPT_POSTFIELDS] = (string) $this->getBody();
+        $requestBody = (string) $this->getBody();
+
+        if ($requestBody !== '') {
+            $curlOptions[CURLOPT_POSTFIELDS] = $requestBody;
         }

         return $curlOptions;

system/HTTP/CURLRequest.php Outdated Show resolved Hide resolved
system/HTTP/CURLRequest.php Outdated Show resolved Hide resolved
system/HTTP/CURLRequest.php Show resolved Hide resolved
@@ -575,7 +575,7 @@ protected function setCURLOptions(array $curlOptions = [], array $config = [])
}

// Decode Content
if (! empty($config['decode_content'])) {
if (array_key_exists('decode_content', $config) && $config['decode_content']) {
Copy link
Member

Choose a reason for hiding this comment

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

for right side, pls be more specific what we are checking against

system/HTTP/CURLRequest.php Outdated Show resolved Hide resolved
system/HTTP/CURLRequest.php Outdated Show resolved Hide resolved
@@ -650,7 +650,7 @@ protected function setCURLOptions(array $curlOptions = [], array $config = [])
}

// version
if (! empty($config['version'])) {
if (array_key_exists('version', $config) && $config['version']) {
Copy link
Member

Choose a reason for hiding this comment

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

right side change to is_string and !== ''?

Copy link
Collaborator Author

@ddevsr ddevsr Dec 6, 2024

Choose a reason for hiding this comment

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

Based documentation float and string

@github-actions github-actions bot added the stale Pull requests with conflicts label Dec 17, 2024
Copy link

👋 Hi, @ddevsr!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

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

Successfully merging this pull request may close these issues.

2 participants