Changeset View
Changeset View
Standalone View
Standalone View
src/applications/diffusion/ssh/DiffusionGitSSHWorkflow.php
| <?php | <?php | ||||
| abstract class DiffusionGitSSHWorkflow | abstract class DiffusionGitSSHWorkflow | ||||
| extends DiffusionSSHWorkflow | extends DiffusionSSHWorkflow | ||||
| implements DiffusionRepositoryClusterEngineLogInterface { | implements DiffusionRepositoryClusterEngineLogInterface { | ||||
| private $engineLogProperties = array(); | private $engineLogProperties = array(); | ||||
| private $protocolLog; | private $protocolLog; | ||||
| private $wireProtocol; | |||||
| protected function writeError($message) { | protected function writeError($message) { | ||||
| // Git assumes we'll add our own newlines. | // Git assumes we'll add our own newlines. | ||||
| return parent::writeError($message."\n"); | return parent::writeError($message."\n"); | ||||
| } | } | ||||
| public function writeClusterEngineLogMessage($message) { | public function writeClusterEngineLogMessage($message) { | ||||
| parent::writeError($message); | parent::writeError($message); | ||||
| $this->getErrorChannel()->update(); | $this->getErrorChannel()->update(); | ||||
| ▲ Show 20 Lines • Show All 51 Lines • ▼ Show 20 Lines | protected function newProtocolLog($is_proxy) { | ||||
| // While developing, do this to write a full protocol log to disk: | // While developing, do this to write a full protocol log to disk: | ||||
| // | // | ||||
| // return new PhabricatorProtocolLog('/tmp/git-protocol.log'); | // return new PhabricatorProtocolLog('/tmp/git-protocol.log'); | ||||
| return null; | return null; | ||||
| } | } | ||||
| protected function getProtocolLog() { | final protected function getProtocolLog() { | ||||
| return $this->protocolLog; | return $this->protocolLog; | ||||
| } | } | ||||
| protected function setProtocolLog(PhabricatorProtocolLog $log) { | final protected function setProtocolLog(PhabricatorProtocolLog $log) { | ||||
| $this->protocolLog = $log; | $this->protocolLog = $log; | ||||
| } | } | ||||
| final protected function getWireProtocol() { | |||||
| return $this->wireProtocol; | |||||
| } | |||||
| final protected function setWireProtocol( | |||||
| DiffusionGitWireProtocol $protocol) { | |||||
| $this->wireProtocol = $protocol; | |||||
| return $this; | |||||
| } | |||||
| public function willWriteMessageCallback( | public function willWriteMessageCallback( | ||||
| PhabricatorSSHPassthruCommand $command, | PhabricatorSSHPassthruCommand $command, | ||||
| $message) { | $message) { | ||||
| $log = $this->getProtocolLog(); | $log = $this->getProtocolLog(); | ||||
| if ($log) { | if ($log) { | ||||
| $log->didWriteBytes($message); | $log->didWriteBytes($message); | ||||
| } | } | ||||
| $protocol = $this->getWireProtocol(); | |||||
| if ($protocol) { | |||||
| $message = $protocol->willWriteBytes($message); | |||||
| } | |||||
| return $message; | return $message; | ||||
| } | } | ||||
| public function willReadMessageCallback( | public function willReadMessageCallback( | ||||
| PhabricatorSSHPassthruCommand $command, | PhabricatorSSHPassthruCommand $command, | ||||
| $message) { | $message) { | ||||
| $log = $this->getProtocolLog(); | $log = $this->getProtocolLog(); | ||||
| if ($log) { | if ($log) { | ||||
| $log->didReadBytes($message); | $log->didReadBytes($message); | ||||
| } | } | ||||
| $protocol = $this->getWireProtocol(); | |||||
| if ($protocol) { | |||||
| $message = $protocol->willReadBytes($message); | |||||
epriestley: We can run them simultaneously and check for byte-equivalence by changing this into:
```… | |||||
| } | |||||
| return $message; | return $message; | ||||
| } | } | ||||
| } | } | ||||
We can run them simultaneously and check for byte-equivalence by changing this into:
This only gets us into trouble if someone enables prototypes and does a 2GB+ fetch and we hit the string length limit, and we could compare the outputs as we go to avoid that.
But I'm not sure this is particularly valuable: we don't have a way to collect external telemetry (and even if we did, having a protocol log but no actual repro case probably wouldn't be terribly helpful in many cases), and requests against secure don't touch any of the interesting cases anyway so it seems unlikely that they'd identify issues that don't crop up locally.
This does run "alongside" in the sense that the new code only activates when prototypes are enabled, so secure will see it but production will continue just doing protocol passthru until this feels ready to move forward.