Skip to content

call curl_close after last use of the curl object#550

Merged
tenzap merged 1 commit into
kalkun-sms:develfrom
tenzap:feature-ncpr-curl
Nov 21, 2025
Merged

call curl_close after last use of the curl object#550
tenzap merged 1 commit into
kalkun-sms:develfrom
tenzap:feature-ncpr-curl

Conversation

@tenzap

@tenzap tenzap commented Nov 21, 2025

Copy link
Copy Markdown
Collaborator

As suggested at
#549 (comment)

Summary by CodeRabbit

  • Bug Fixes
    • Improved HTTP response handling to ensure reliable status code retrieval and prevent potential edge-case failures.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Nov 21, 2025

Copy link
Copy Markdown

Walkthrough

The change relocates HTTP response code retrieval in a cURL request to occur immediately after curl_exec(), before the curl handle is closed, and removes the duplicate retrieval that previously occurred afterward. This prevents potential use-after-close issues.

Changes

Cohort / File(s) Summary
cURL Response Code Retrieval Fix
application/helpers/kalkun_helper.php
Moved curl_getinfo($request, CURLINFO_HTTP_CODE) call to immediately after curl_exec() execution and removed duplicate retrieval that occurred after curl_close logic. Ensures HTTP status is captured while the CURL handle remains valid.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify that moving the curl_getinfo() call does not break existing logic that may depend on the previous timing
  • Confirm that the HTTP code is being used correctly in subsequent code paths following the relocation
  • Ensure the removed duplicate call had no side effects or was genuinely redundant

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: moving curl_close to after the last use of the curl object, which aligns with the refactoring to prevent use-after-close issues.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ddd4fe4 and 19fb1be.

📒 Files selected for processing (1)
  • application/helpers/kalkun_helper.php (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: test (ubuntu-latest, 7.1)
  • GitHub Check: test (ubuntu-latest, 7.4)
  • GitHub Check: test (ubuntu-latest, 5.6)
  • GitHub Check: test (ubuntu-latest, 7.3)
  • GitHub Check: test (ubuntu-latest, 7.2)
  • GitHub Check: test (ubuntu-latest, 8.2)
  • GitHub Check: test (ubuntu-latest, 8.4)
  • GitHub Check: test (ubuntu-latest, 8.5)
  • GitHub Check: test (ubuntu-latest, 7.0)
  • GitHub Check: test (ubuntu-latest, 8.3)
  • GitHub Check: test (ubuntu-latest, 8.0)
  • GitHub Check: test (ubuntu-latest, 8.1)
  • GitHub Check: check-code (ubuntu-latest, 8.2)
🔇 Additional comments (1)
application/helpers/kalkun_helper.php (1)

33-43: curl_getinfo correctly moved before potential curl_close; change looks good

Capturing the HTTP status immediately after curl_exec() and before the conditional curl_close() avoids using the handle after close on pre‑8.0 PHP and keeps behavior correct across versions. No issues spotted with the new ordering or the existing status check.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tenzap tenzap merged commit 1d574f9 into kalkun-sms:devel Nov 21, 2025
16 checks passed
@tenzap tenzap deleted the feature-ncpr-curl branch November 21, 2025 20:33
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

Successfully merging this pull request may close these issues.

1 participant