? ? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
10 Jun 2020

Pull Request for Issue #29519 .

Summary of Changes

This Pull Request (PR) changes the special security check when using a remote database server to allow port numbers to be used in the host name.

The database drivers already seem to support that at least for hostnames and IPv4 addresses.

With IPv6 I'm not sure yet (the address should be enclosed in square brackets to distinguish the colon to separate the port from the colons in the IPv6 address).

Testing Instructions

Requirements

  • You need a clean, current staging or 3.9.19 or latest 3.9 nightly build.
  • You need a local database server, i.e. "localhost"/"127.0.0.1"/"::1".
  • You may test with any kind of database type which can be used.
    If you have MySQL or MariaDB, plese test both the "MySQLi" and the "MySQL (PDO)" type.
  • Please report back with which kind of database server and type you have tested.

Test Execution

  1. On a clean, current staging or 3.9.19 or latest 3.9 nightly build, apply the patch for this PR.

  2. Make a new installation.

  3. When coming to the database part, fill in correct data and use either "localhost", "127.0.0.1" or "::1" (the latter only if IPv6 works) as database host, together with the port number on which the database server works, which normally is 3306 for MySQL or MariaDB and 5432 for PostgreSQL, i.e. use as database host

  • for MySQL and MariaDB: "localhost:3306" or "127.0.0.1:3306" or "[::1]:3306"
  • for PostgreSQL: "localhost:5432" or "127.0.0.1:5432" or "[::1]:5432"
    or different ports if your servers are set up not to use the standard ports.
  1. Start the installation.
    Result: There is no extra security check using a temporary file, the installation works as usual when using a local database host.

  2. Clear the session cookie or close the browser window so the next test starts with a new session.

  3. Repeat the previous steps 1 to 4, i.e. make again a new installation using another empty database or creating another nerw one, but this time don't use a port number, and in case of IPv6 leave away the square brackets.
    Result: There is again no extra security check using a temporary file, the installation works.

  4. Clear the session cookie or close the browser window so the next test starts with a new session.

  5. Repeat step 6, i.e. make again a new installation using another empty database or creating another nerw one, but this time use something else than "localhost" or "127.0.0.1"or "::1", e.g. use the real computer name of that server and make sure it can be resolved to an IP address e.g. by adding it to the local hosts file ("c:\windows\system32\drivers\etc\hosts" on Windows or "/etc/hosts" on Linux). It's ok if this resolves to 127.0.0.1, too, it just needs a different name than the ones listed before. Use a port number like in the first installation.
    Result: This time there is extra security check using a temporary file, the installation works.

  6. Clear the session cookie or close the browser window so the next test starts with a new session.

  7. Repeat step 8, but this time don't use a port number.
    Result: Again there is extra security check using a temporary file, the installation works.

Expected result

No security check when using "localhost:1234", "127.0.0.1:1234" or "[::1]:1234" as database host, with "1234" being the port number on which that server works.

No security check when using "localhost", "127.0.0.1" or "::1" as database host.

Security check when using something else than "localhost", "127.0.0.1" or "::1" with or without port number as database host.

Actual result

Security check when using "localhost:1234", "127.0.0.1:1234" or "[::1]:1234" as database host, with "1234" being the port number on which that server works, as if it was a remote host.

No security check when using "localhost", "127.0.0.1" or "::1" as database host.

Security check when using something else than "localhost", "127.0.0.1" or "::1" with or without port number as database host.

Documentation Changes Required

Don't think so, but am not 100% sure.

avatar richard67 richard67 - open - 10 Jun 2020
avatar richard67 richard67 - change - 10 Jun 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 10 Jun 2020
Category Installation
avatar richard67 richard67 - change - 10 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 10 Jun 2020
avatar richard67 richard67 - change - 10 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 10 Jun 2020
avatar richard67 richard67 - change - 10 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 10 Jun 2020
avatar toivo toivo - test_item - 11 Jun 2020 - Tested successfully
avatar toivo
toivo - comment - 11 Jun 2020

I have tested this item successfully on 03676bd

Tested successfully in Joomla 3.9.20-dev from 11 May. Using IPv4, no PostgreSQL.
Environment: Wampserver 3.2.2 Apache 2.4.43a MySQL 8.0.20 MariaDB 10.4.13 PHP 7.4.7
MySQL: MySQLi, MySQL (PDO) localhost:3308, localhost
MariaDB: localhost, localhost:3306, databaseserver:3306, databaseserver


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29567.
avatar richard67
richard67 - comment - 12 Jun 2020

@Quy Could you test this one, too? It's does the same thing at the same time as #29568 does for J4 (only at a different place because the installation has been restructured in J4).

avatar Quy Quy - test_item - 12 Jun 2020 - Tested successfully
avatar Quy
Quy - comment - 12 Jun 2020

I have tested this item successfully on 03676bd


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

avatar Quy Quy - change - 12 Jun 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 12 Jun 2020

RTC


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

avatar richard67 richard67 - change - 12 Jun 2020
Labels Added: ? ?
avatar richard67
richard67 - comment - 12 Jun 2020

Thanks guys for testing.

avatar PhilETaylor
PhilETaylor - comment - 25 Jun 2020

If Im right with a quick glance, this restricts the mysql port to 4 digits right ?

The documentation for Mysql port allows port numbers from 0 to 65535

Surely we would allow any valid mysql port and not just a 4 digit port number?

avatar PhilETaylor
PhilETaylor - comment - 25 Jun 2020

Actually ignore me, I missed the first required int before the optional 4... that all validates:

Unit test: https://3v4l.org/qAagS

<?php

$localhost = '/^(((localhost|127\.0\.0\.1|\[\:\:1\])(\:[1-9]{1}[0-9]{0,4})?)|(\:\:1))$/';

var_dump(preg_match($localhost, '127.0.0.1:80'));		
var_dump(preg_match($localhost, '127.0.0.1:80'));		
var_dump(preg_match($localhost, '127.0.0.1:8000'));			
var_dump(preg_match($localhost, '127.0.0.1:65535'));
var_dump(preg_match($localhost, '127.0.0.1:6553511111111'));
int(1)
int(1)
int(1)
int(1)
int(0)

So all good :)

avatar richard67
richard67 - comment - 25 Jun 2020

If Im right with a quick glance, this restricts the mysql port to 4 digits right ?

Wrong. It is "[1-9]{1}[0-9]{0,4}", which means at least one digit between 1 and 9 followed by zero to four digits from zero to nine, i.e. it can have 1 to 5 digts without leading zero. I did not limit it to high ports only, that's why I allow also less than 4 digits.

avatar PhilETaylor
PhilETaylor - comment - 25 Jun 2020

Correct - just like I corrected myself. Nothing to see here.. .move along :) we all make mistakes :)

avatar zero-24 zero-24 - close - 28 Jun 2020
avatar zero-24 zero-24 - merge - 28 Jun 2020
avatar zero-24 zero-24 - change - 28 Jun 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-06-28 19:20:04
Closed_By zero-24
Labels
avatar zero-24
zero-24 - comment - 28 Jun 2020

Merged thanks

Add a Comment

Login with GitHub to post a comment