Unexpected output: Affected paths: relative - resubmit patch.
⚙ D278 INP-1653 - Emulate "CURLINFO_REDIRECT_URL" option in "kCurlHelper" class
Page MenuHomeIn-Portal Phabricator

INP-1653 - Emulate "CURLINFO_REDIRECT_URL" option in "kCurlHelper" class
AcceptedPublic

Authored by alex on Jan 3 2017, 2:58 AM.

Details

Reviewers
erik
Test Plan

Test Plan (technical)

  • anywhere
    • apply patch from the D422 (if not applied already and committed)
    • apply patch from the D423 (if not applied already and committed)
  • in IDE:
    • open /index.php file for editing
    • replace $application->Run(); line with the following code:
/** @var kCurlHelper $curl_helper */
$curl_helper = $application->recallObject('CurlHelper');

$curl_helper->followLocation = false;
$curl_helper->Send('http://www.google.com', false);
echo 'Redirect URL (not following): [' . $curl_helper->getInfo(CURLINFO_REDIRECT_URL) . ']' . PHP_EOL;
echo 'Effective URL (not following): [' . $curl_helper->getInfo(CURLINFO_EFFECTIVE_URL) . ']' . PHP_EOL;
$curl_helper->CloseConnection();

$curl_helper->followLocation = true;
$curl_helper->Send('http://www.google.com', false);
echo 'Redirect URL (following): [' . $curl_helper->getInfo(CURLINFO_REDIRECT_URL) . ']' . PHP_EOL;
echo 'Effective URL (following): [' . $curl_helper->getInfo(CURLINFO_EFFECTIVE_URL) . ']' . PHP_EOL;
$curl_helper->CloseConnection();
    • save changes
  • in Web Browser:
    • open /index.php
    • confirm, that the View Source of page output contains this text:
Redirect URL (not following): [https://www.google.com/?gws_rd=ssl]
Effective URL (not following): [http://www.google.com/]
Redirect URL (following): []
Effective URL (following): [https://www.google.com/?gws_rd=ssl]

Test Plan (human)

  1. run test from D251

Diff Detail

Repository
rINP In-Portal
Branch
/in-portal/branches/5.2.x
Lint
Lint ErrorsExcuse: Not fixing, because haven't broken anything.
SeverityLocationCodeMessage
Errorcore/units/helpers/curl_helper.php:592PHPCS.E.CodingStandard.Classes.ClassDeclaration.SpaceBeforeCloseBraceCodingStandard.Classes.ClassDeclaration.SpaceBeforeCloseBrace
Unit
No Unit Test Coverage
Build Status
Buildable 1113
Build 1113: arc lint + arc unit

Event Timeline

glebs updated this revision to Diff 672.Jan 3 2017, 2:58 AM
glebs retitled this revision from to INP-1653 Emulate "CURLINFO_REDIRECT_URL" option in "kCurlHelper" class.
glebs updated this object.
glebs edited the test plan for this revision. (Show Details)
glebs added 1 JIRA issue(s): INP-1653.
alex requested changes to this revision.Jan 3 2017, 5:00 AM
alex edited edge metadata.

Also issues with Test Plan:

  1. fact, that CSS placed in inc/styles.css is actually used inside CKEditor in these cases isn't tested:
    • on initial load
    • after clicking Source button twice (this is when CKEditor reloads CSS)
  2. when kCurlHelper::$followLocation option is disabled the CURLINFO_REDIRECT_URL option would always return redirect url (if found)
  3. when kCurlHelper::$followLocation option is enabled and kCurlHelper::followLocationLimited returns false the location following happens and CURLINFO_REDIRECT_URL option value would be empty/false (as per docs)
  4. when kCurlHelper::$followLocation option is enabled and kCurlHelper::followLocationLimited returns true the location following happens and CURLINFO_REDIRECT_URL option value would be empty/false (as per docs)
core/units/helpers/curl_helper.php
503–509

When kCurlHelper::$followLocation is enabled we should immediately return an empty string.


Doesn't work according to docs from http://php.net/manual/en/function.curl-getinfo.php page.

509

Return the value, that curl_getinfo($ch, CURLINFO_REDIRECT_URL) returns in real life when no Location: header was found.


Maybe it really returns false, but double check just in case.

This revision now requires changes to proceed.Jan 3 2017, 5:00 AM
glebs updated this revision to Diff 698.Jan 25 2017, 4:30 AM
glebs edited edge metadata.

Requested changes

alex requested changes to this revision.Jan 25 2017, 5:00 AM
alex edited edge metadata.
alex added inline comments.
core/units/helpers/curl_helper.php
502–504
  1. Please move this IF inside next IF.
  2. Update test plan to reflect regular usage of getInfo method (not for getting url).

This causes a side effect, that regular class to getInfo method (not asking redirect url) would fail.

This revision now requires changes to proceed.Jan 25 2017, 5:00 AM
alex retitled this revision from INP-1653 Emulate "CURLINFO_REDIRECT_URL" option in "kCurlHelper" class to INP-1653 - Emulate "CURLINFO_REDIRECT_URL" option in "kCurlHelper" class.Aug 3 2022, 4:40 AM
alex commandeered this revision.Aug 3 2022, 4:44 AM
alex edited reviewers, added: glebs; removed: alex.
This revision now requires review to proceed.Aug 3 2022, 4:44 AM
alex planned changes to this revision.Aug 3 2022, 4:46 AM
alex edited reviewers, added: erik; removed: glebs.
alex updated this revision to Diff 1078.Aug 3 2022, 5:44 AM

Fixed "kCurlHelper::getInfo" method to support working with other than "CURLINFO_REDIRECT_URL" info.

alex planned changes to this revision.Aug 3 2022, 5:46 AM

Test plan is incomplete.

alex updated this revision to Diff 1081.Aug 5 2022, 6:54 AM

Removed code, that was extracted as D422 and D423.

alex edited the test plan for this revision. (Show Details)Aug 5 2022, 6:58 AM
alex updated this revision to Diff 1082.Aug 5 2022, 6:58 AM

Removed debug code.

alex edited the test plan for this revision. (Show Details)Nov 27 2023, 1:54 AM
erik accepted this revision.Nov 27 2023, 4:24 AM
This revision is now accepted and ready to land.Nov 27 2023, 4:24 AM