? ? ? Pending

User tests: Successful: Unsuccessful:

avatar SharkyKZ
SharkyKZ
11 Jul 2019

Pull Request for Issue #25518 .

Summary of Changes

Use IpHelper to get IP in Joomla\CMS\Log\Logger\FormattedtextLogger.

Testing Instructions

Install Joomla behind a reverse proxy which provides both remote_addr and http_x_forwarded_for headers.
Perform a login failure on /administrator
Check the /logs/error.php log

Expected result

To see the ip address from http_x_forwarded_for reported as the offending IP address.

Actual result

To see the ip address from remote_addr reported as the offending IP address.

Documentation Changes Required

No.

avatar SharkyKZ SharkyKZ - open - 11 Jul 2019
avatar SharkyKZ SharkyKZ - change - 11 Jul 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 11 Jul 2019
Category Libraries
avatar SharkyKZ SharkyKZ - change - 12 Jul 2019
Labels Added: ?
avatar SharkyKZ
SharkyKZ - comment - 12 Jul 2019

Failing tests related to framework. Submitted PR there joomla-framework/utilities#23.

avatar richard67
richard67 - comment - 16 Jul 2019

I can't test because I don't have a reverse proxy.

@twrhills As you had the issue handled by this PR, could you test?

avatar twrhills
twrhills - comment - 16 Jul 2019

I can't test because I don't have a reverse proxy.

@twrhills As you had the issue handled by this PR, could you test?

@richard67 I have tested this and it is the correct solution for my case. I am unfamiliar with github so if you need me to confirm in another way I would be grateful if you can link documentation / instructions.

avatar Quy
Quy - comment - 16 Jul 2019

Thank you for testing!

Please mark your test here: https://issues.joomla.org/tracker/joomla-cms/25520

avatar richard67
richard67 - comment - 16 Jul 2019

@twrhills Follow the link posted in comment before by Quy, then click the "Test this" button at the top left corner above the description, then in the "Submit test result" select your test result with the check box, then click "Submit test result" button below the text field for the (otional) comment.

avatar twrhills
twrhills - comment - 16 Jul 2019

I have tested this item successfully on 6ed695e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25520.

avatar twrhills twrhills - test_item - 16 Jul 2019 - Tested successfully
avatar alikon
alikon - comment - 5 Aug 2019

I have tested this item successfully on 6ed695e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25520.

avatar alikon alikon - test_item - 5 Aug 2019 - Tested successfully
avatar franz-wohlkoenig franz-wohlkoenig - change - 5 Aug 2019
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 5 Aug 2019

Status "Ready To Commit".

avatar HLeithner HLeithner - change - 8 Aug 2019
Labels Added: ?
avatar HLeithner
HLeithner - comment - 8 Aug 2019

@SharkyKZ can you please fix the error messages found by travis?

  1. JLogLoggerFormattedTextTest::testAddEntry
    Undefined index: REMOTE_ADDR
    /home/travis/build/joomla/joomla-cms/tests/unit/core/helper.php:55
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:488
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:440
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:57
    /home/travis/build/joomla/joomla-cms/libraries/src/Log/Logger/FormattedtextLogger.php:195
    /home/travis/build/joomla/joomla-cms/libraries/src/Log/Logger/FormattedtextLogger.php:171
    /home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/log/loggers/JLogLoggerFormattedTextTest.php:174
  2. JLogLoggerFormattedTextTest::testDeferWritingEntry
    Undefined index: REMOTE_ADDR
    /home/travis/build/joomla/joomla-cms/tests/unit/core/helper.php:55
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:488
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:440
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:57
    /home/travis/build/joomla/joomla-cms/libraries/src/Log/Logger/FormattedtextLogger.php:195
    /home/travis/build/joomla/joomla-cms/libraries/src/Log/Logger/FormattedtextLogger.php:139
    /home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/log/loggers/JLogLoggerFormattedTextTest.php:237
  3. JLogLoggerW3CTest::testAddEntry
    Undefined index: REMOTE_ADDR
    /home/travis/build/joomla/joomla-cms/tests/unit/core/helper.php:55
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:488
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:440
    /home/travis/build/joomla/joomla-cms/libraries/vendor/joomla/utilities/src/IpHelper.php:57
    /home/travis/build/joomla/joomla-cms/libraries/src/Log/Logger/FormattedtextLogger.php:195
    /home/travis/build/joomla/joomla-cms/libraries/src/Log/Logger/FormattedtextLogger.php:171
    /home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/log/loggers/JLogLoggerW3CTest.php:37
avatar SharkyKZ
SharkyKZ - comment - 8 Aug 2019

@HLeithner need to update Framework's Utilities package to fix the notice. Submitted a separate PR #25601.

avatar HLeithner
HLeithner - comment - 8 Aug 2019

@SharkyKZ thx for the info so this is RTC when the composer update is merged.

avatar HLeithner
HLeithner - comment - 23 Aug 2019

I created pr #26000 please test.

avatar richard67
richard67 - comment - 23 Aug 2019

@HLeithner Ist this a duplicate then to @SharkyKZ 's PR #25601 ?

avatar SharkyKZ
SharkyKZ - comment - 23 Aug 2019

@richard67 almost. But just realized my PR reverts some changes to Composer (I was using older version). So I'll close it.

avatar HLeithner
HLeithner - comment - 12 Oct 2019

@SharkyKZ IpHelper::getIp(); seams to behave different then the current code in the testing. Could you please have a look?

avatar SharkyKZ
SharkyKZ - comment - 13 Oct 2019

@HLeithner should I change the PR so it does not break tests or update tests?

avatar mbabker
mbabker - comment - 13 Oct 2019

The tests should be updated. I’m going to take a wild guess that superglobal values aren’t being properly set and reset.

avatar SharkyKZ
SharkyKZ - comment - 13 Oct 2019

There's also discrepancy between empty IP values. Before it was null, now it's an empty string 😕

avatar joomla-cms-bot joomla-cms-bot - change - 13 Oct 2019
Category Libraries Libraries Unit Tests
avatar SharkyKZ SharkyKZ - change - 13 Oct 2019
Labels Added: ?
avatar HLeithner
HLeithner - comment - 13 Oct 2019

The w3c logger returned - with the old code now it shows an empty stringand I think thats wrong. So the null seams to be right.

avatar SharkyKZ
SharkyKZ - comment - 13 Oct 2019

Added empty string check in the logger.

avatar HLeithner HLeithner - change - 29 Oct 2019
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-10-29 14:26:36
Closed_By HLeithner
avatar HLeithner HLeithner - close - 29 Oct 2019
avatar HLeithner HLeithner - merge - 29 Oct 2019
avatar HLeithner
HLeithner - comment - 29 Oct 2019

Thanks

Add a Comment

Login with GitHub to post a comment