mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-25 16:22:43 +01:00
Use large text columns to store IP addresses
Summary: Fixes T10259. There was no real reason to do this `ip2long()` stuff in the first place -- it's very slightly smaller, but won't work with ipv6 and the savings are miniscule. Test Plan: - Ran migration. - Viewed logs in web UI. - Pulled and pushed. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10259 Differential Revision: https://secure.phabricator.com/D15165
This commit is contained in:
parent
1d939e0bd8
commit
9d125b459e
10 changed files with 62 additions and 26 deletions
5
resources/sql/autopatches/20160202.ipv6.1.sql
Normal file
5
resources/sql/autopatches/20160202.ipv6.1.sql
Normal file
|
@ -0,0 +1,5 @@
|
|||
ALTER TABLE {$NAMESPACE}_repository.repository_pullevent
|
||||
CHANGE remoteAddress remoteAddress VARBINARY(64);
|
||||
|
||||
ALTER TABLE {$NAMESPACE}_repository.repository_pushevent
|
||||
CHANGE remoteAddress remoteAddress VARBINARY(64);
|
39
resources/sql/autopatches/20160202.ipv6.2.php
Normal file
39
resources/sql/autopatches/20160202.ipv6.2.php
Normal file
|
@ -0,0 +1,39 @@
|
|||
<?php
|
||||
|
||||
$pull = new PhabricatorRepositoryPullEvent();
|
||||
$push = new PhabricatorRepositoryPushEvent();
|
||||
|
||||
$conn_w = $pull->establishConnection('w');
|
||||
|
||||
$log_types = array($pull, $push);
|
||||
foreach ($log_types as $log) {
|
||||
foreach (new LiskMigrationIterator($log) as $row) {
|
||||
$addr = $row->getRemoteAddress();
|
||||
|
||||
$addr = (string)$addr;
|
||||
if (!strlen($addr)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!ctype_digit($addr)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!(int)$addr) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$ip = long2ip($addr);
|
||||
if (!is_string($ip) || !strlen($ip)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$id = $row->getID();
|
||||
queryfx(
|
||||
$conn_w,
|
||||
'UPDATE %T SET remoteAddress = %s WHERE id = %d',
|
||||
$log->getTableName(),
|
||||
$ip,
|
||||
$id);
|
||||
}
|
||||
}
|
|
@ -542,8 +542,12 @@ final class AphrontRequest extends Phobject {
|
|||
return $this->isFormPost() && $this->getStr('__dialog__');
|
||||
}
|
||||
|
||||
public function getRemoteAddr() {
|
||||
return $_SERVER['REMOTE_ADDR'];
|
||||
public function getRemoteAddress() {
|
||||
$address = $_SERVER['REMOTE_ADDR'];
|
||||
if (!strlen($address)) {
|
||||
return null;
|
||||
}
|
||||
return substr($address, 0, 64);
|
||||
}
|
||||
|
||||
public function isHTTPS() {
|
||||
|
|
|
@ -322,6 +322,7 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
|
|||
case 'phid':
|
||||
case 'policy';
|
||||
case 'hashpath64':
|
||||
case 'ipaddress':
|
||||
$column_type = 'varbinary(64)';
|
||||
break;
|
||||
case 'bytes64':
|
||||
|
|
|
@ -76,8 +76,7 @@ final class DiffusionServeController extends DiffusionController {
|
|||
}
|
||||
|
||||
try {
|
||||
$remote_addr = $request->getRemoteAddr();
|
||||
$remote_addr = ip2long($remote_addr);
|
||||
$remote_addr = $request->getRemoteAddress();
|
||||
|
||||
$pull_event = id(new PhabricatorRepositoryPullEvent())
|
||||
->setEpoch(PhabricatorTime::getNow())
|
||||
|
@ -720,11 +719,11 @@ final class DiffusionServeController extends DiffusionController {
|
|||
}
|
||||
|
||||
private function getCommonEnvironment(PhabricatorUser $viewer) {
|
||||
$remote_addr = $this->getRequest()->getRemoteAddr();
|
||||
$remote_address = $this->getRequest()->getRemoteAddress();
|
||||
|
||||
return array(
|
||||
DiffusionCommitHookEngine::ENV_USER => $viewer->getUsername(),
|
||||
DiffusionCommitHookEngine::ENV_REMOTE_ADDRESS => $remote_addr,
|
||||
DiffusionCommitHookEngine::ENV_REMOTE_ADDRESS => $remote_address,
|
||||
DiffusionCommitHookEngine::ENV_REMOTE_PROTOCOL => 'http',
|
||||
);
|
||||
}
|
||||
|
|
|
@ -56,15 +56,6 @@ final class DiffusionCommitHookEngine extends Phobject {
|
|||
return $this->remoteAddress;
|
||||
}
|
||||
|
||||
private function getRemoteAddressForLog() {
|
||||
// If whatever we have here isn't a valid IPv4 address, just store `null`.
|
||||
// Older versions of PHP return `-1` on failure instead of `false`.
|
||||
$remote_address = $this->getRemoteAddress();
|
||||
$remote_address = max(0, ip2long($remote_address));
|
||||
$remote_address = nonempty($remote_address, null);
|
||||
return $remote_address;
|
||||
}
|
||||
|
||||
public function setSubversionTransactionInfo($transaction, $repository) {
|
||||
$this->subversionTransaction = $transaction;
|
||||
$this->subversionRepository = $repository;
|
||||
|
@ -1078,7 +1069,7 @@ final class DiffusionCommitHookEngine extends Phobject {
|
|||
$viewer = $this->getViewer();
|
||||
return PhabricatorRepositoryPushEvent::initializeNewEvent($viewer)
|
||||
->setRepositoryPHID($this->getRepository()->getPHID())
|
||||
->setRemoteAddress($this->getRemoteAddressForLog())
|
||||
->setRemoteAddress($this->getRemoteAddress())
|
||||
->setRemoteProtocol($this->getRemoteProtocol())
|
||||
->setEpoch(time());
|
||||
}
|
||||
|
|
|
@ -42,12 +42,9 @@ final class DiffusionPushLogListView extends AphrontView {
|
|||
$repository = $log->getRepository();
|
||||
|
||||
// Reveal this if it's valid and the user can edit the repository.
|
||||
$remote_addr = '-';
|
||||
$remote_address = '-';
|
||||
if (isset($editable_repos[$log->getRepositoryPHID()])) {
|
||||
$remote_long = $log->getPushEvent()->getRemoteAddress();
|
||||
if ($remote_long) {
|
||||
$remote_addr = long2ip($remote_long);
|
||||
}
|
||||
$remote_address = $log->getPushEvent()->getRemoteAddress();
|
||||
}
|
||||
|
||||
$event_id = $log->getPushEvent()->getID();
|
||||
|
@ -76,7 +73,7 @@ final class DiffusionPushLogListView extends AphrontView {
|
|||
),
|
||||
$repository->getDisplayName()),
|
||||
$handles[$log->getPusherPHID()]->renderLink(),
|
||||
$remote_addr,
|
||||
$remote_address,
|
||||
$log->getPushEvent()->getRemoteProtocol(),
|
||||
$log->getRefType(),
|
||||
$log->getRefName(),
|
||||
|
|
|
@ -41,13 +41,13 @@ final class PhabricatorUserLogView extends AphrontView {
|
|||
$ip = phutil_tag(
|
||||
'a',
|
||||
array(
|
||||
'href' => $base_uri.'?ip='.$log->getRemoteAddr().'#R',
|
||||
'href' => $base_uri.'?ip='.$ip.'#R',
|
||||
),
|
||||
$ip);
|
||||
$session = phutil_tag(
|
||||
'a',
|
||||
array(
|
||||
'href' => $base_uri.'?sessions='.$log->getSession().'#R',
|
||||
'href' => $base_uri.'?sessions='.$ip.'#R',
|
||||
),
|
||||
$session);
|
||||
}
|
||||
|
|
|
@ -30,7 +30,7 @@ final class PhabricatorRepositoryPullEvent
|
|||
self::CONFIG_COLUMN_SCHEMA => array(
|
||||
'repositoryPHID' => 'phid?',
|
||||
'pullerPHID' => 'phid?',
|
||||
'remoteAddress' => 'uint32?',
|
||||
'remoteAddress' => 'ipaddress?',
|
||||
'remoteProtocol' => 'text32?',
|
||||
'resultType' => 'text32',
|
||||
'resultCode' => 'uint32',
|
||||
|
|
|
@ -29,7 +29,7 @@ final class PhabricatorRepositoryPushEvent
|
|||
self::CONFIG_AUX_PHID => true,
|
||||
self::CONFIG_TIMESTAMPS => false,
|
||||
self::CONFIG_COLUMN_SCHEMA => array(
|
||||
'remoteAddress' => 'uint32?',
|
||||
'remoteAddress' => 'ipaddress?',
|
||||
'remoteProtocol' => 'text32?',
|
||||
'rejectCode' => 'uint32',
|
||||
'rejectDetails' => 'text64?',
|
||||
|
|
Loading…
Reference in a new issue