Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed log constants #836

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@andreybolonin
Copy link
Contributor

andreybolonin commented Oct 1, 2015

PHP Notice 'yii\base\ErrorException' with message 'Use of undefined constant NET_SFTP_LOG_REALTIME - assumed 'NET_SFTP_LOG_REALTIME''

in /data/cporder-2-0/vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:2682

Stack trace:
#0 /data/cporder-2-0/vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php(2682): yii\base\ErrorHandler->handleError(8, 'Use of undefine...', '/data/cporder-2...', 2682, Array)
#1 /data/cporder-2-0/vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php(467): phpseclib\Net\SFTP->_send_sftp_packet(1, '\x00\x00\x00\x03')
#2 /data/cporder-2-0/components/Transports/TransportSFTP.php(29): phpseclib\Net\SFTP->login('CentralPoint', 'VoOe8TVK4QW5mB9...')
#3 /data/cporder-2-0/components/Transports/TransportBase.php(55): app\components\Transports\TransportSFTP->getChannel()
#4 /data/cporder-2-0/commands/ServiceController.php(741): app\components\Transports\TransportBase->__construct(Array)
#5 [internal function]: app\commands\ServiceController->actionTie()
#6 /data/cporder-2-0/vendor/yiisoft/yii2/base/InlineAction.php(55): call_user_func_array(Array, Array)
#7 /data/cporder-2-0/vendor/yiisoft/yii2/base/Controller.php(151): yii\base\InlineAction->runWithParams(Array)
#8 /data/cporder-2-0/vendor/yiisoft/yii2/console/Controller.php(91): yii\base\Controller->runAction('tie', Array)
#9 /data/cporder-2-0/vendor/yiisoft/yii2/base/Module.php(455): yii\console\Controller->runAction('tie', Array)
#10 /data/cporder-2-0/vendor/yiisoft/yii2/console/Application.php(167): yii\base\Module->runAction('service/tie', Array)
#11 /data/cporder-2-0/vendor/yiisoft/yii2/console/Application.php(143): yii\console\Application->runAction('service/tie', Array)
#12 /data/cporder-2-0/vendor/yiisoft/yii2/base/Application.php(375): yii\console\Application->handleRequest(Object(yii\console\Request))
#13 /data/cporder-2-0/yii(15): yii\base\Application->run()
#14 {main}
PHP Notice 'yii\base\ErrorException' with message 'ob_flush(): failed to flush buffer. No buffer to flush'

in /data/cporder-2-0/vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:2757

Stack trace:
#0 [internal function]: yii\base\ErrorHandler->handleError(8, 'ob_flush(): fai...', '/data/cporder-2...', 2757, Array)
#1 /data/cporder-2-0/vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php(2757): ob_flush()
#2 /data/cporder-2-0/vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php(472): phpseclib\Net\SFTP->_get_sftp_packet()
#3 /data/cporder-2-0/components/Transports/TransportSFTP.php(29): phpseclib\Net\SFTP->login('CentralPoint', 'VoOe8TVK4QW5mB9...')
#4 /data/cporder-2-0/components/Transports/TransportBase.php(55): app\components\Transports\TransportSFTP->getChannel()
#5 /data/cporder-2-0/commands/ServiceController.php(741): app\components\Transports\TransportBase->__construct(Array)
#6 [internal function]: app\commands\ServiceController->actionTie()
#7 /data/cporder-2-0/vendor/yiisoft/yii2/base/InlineAction.php(55): call_user_func_array(Array, Array)
#8 /data/cporder-2-0/vendor/yiisoft/yii2/base/Controller.php(151): yii\base\InlineAction->runWithParams(Array)
#9 /data/cporder-2-0/vendor/yiisoft/yii2/console/Controller.php(91): yii\base\Controller->runAction('tie', Array)
#10 /data/cporder-2-0/vendor/yiisoft/yii2/base/Module.php(455): yii\console\Controller->runAction('tie', Array)
#11 /data/cporder-2-0/vendor/yiisoft/yii2/console/Application.php(167): yii\base\Module->runAction('service/tie', Array)
#12 /data/cporder-2-0/vendor/yiisoft/yii2/console/Application.php(143): yii\console\Application->runAction('service/tie', Array)
#13 /data/cporder-2-0/vendor/yiisoft/yii2/base/Application.php(375): yii\console\Application->handleRequest(Object(yii\console\Request))
#14 /data/cporder-2-0/yii(15): yii\base\Application->run()
#15 {main}

andreybolonin added some commits Oct 1, 2015

@terrafrost

This comment has been minimized.

Copy link

terrafrost commented on 3ff929c Oct 1, 2015

If you're going to comment this line out you ought to do this:

                //ob_flush();

...instead of...

//                ob_flush();

That said, I'm not convinced that this is a good change. Quoting http://php.net/flush ,

flush() may not be able to override the buffering scheme of your web server and it has no effect on any client-side buffering in the browser. It also doesn't affect PHP's userspace output buffering mechanism. This means you will have to call both ob_flush() and flush() to flush the ob output buffers if you are using those.

To fix the errors you're getting I think we'd be better off using the error suppression operator (@).

@terrafrost

This comment has been minimized.

Copy link
Member

terrafrost commented Oct 1, 2015

Nice find.

This should be merged into the 2.0 branch, however, and not into master. Once it's merged into the 2.0 branch the 2.0 branch can be merged into master.

Also, a unit test would be nice. I wouldn't say it's a requirement but it'd be nice :)

fixed ob_flush with @,
php-cs-fixer fixes
@andreybolonin

This comment has been minimized.

Copy link
Contributor Author

andreybolonin commented Oct 2, 2015

Fixed

@ob_flush()

What about https://github.com/FriendsOfPHP/PHP-CS-Fixer code style fixes ?

@bantu

This comment has been minimized.

Copy link
Member

bantu commented Oct 2, 2015

Please keep this PR to bugfix only.

fixed ob_flush with @,
revert php-cs-fixer fixes
@andreybolonin

This comment has been minimized.

Copy link
Contributor Author

andreybolonin commented Oct 3, 2015

All is ok?

@terrafrost

This comment has been minimized.

Copy link
Member

terrafrost commented Oct 4, 2015

Can you squash the commits? Like right now you modified a whole bunch of unrelated lines and then undid those changes. But since the commits are still there git blame will still show that they were changed and merge conflicts will still be able to occur on those lines. Squashing the commits should make it so those changes were never made in the first place!

echo "<pre>\r\n" . $this->_format_log(array($packet), array($packet_type)) . "\r\n</pre>\r\n";
flush();
ob_flush();
@ob_flush();

This comment has been minimized.

@bantu

bantu Mar 27, 2016

Member

Unrelated change

echo "<pre>\r\n" . $this->_format_log(array($data), array($packet_type)) . "\r\n</pre>\r\n";
flush();
ob_flush();
@ob_flush();

This comment has been minimized.

@bantu

bantu Mar 27, 2016

Member

Unrelated change

@terrafrost

This comment has been minimized.

Copy link
Member

terrafrost commented Apr 12, 2016

I've just made my own commit of this:

b3171cc

@@ -3482,7 +3482,7 @@ function _append_log($message_number, $message)
case self::LOG_REALTIME_FILE:
if (!isset($this->realtime_log_file)) {
// PHP doesn't seem to like using constants in fopen()
$filename = self::LOG_REALTIME_FILENAME;
$filename = self::LOG_REALTIME_FILE;

This comment has been minimized.

@terrafrost

terrafrost Apr 12, 2016

Member

This should never have been made a constant in the first place - rather, it should have been NET_SFTP_LOGGING_REALTIME_FILE. The reason being so that you can define the name yourself.

But overall I don't like the whole logging system anyway and at some point in the master branch I intend to redo the whole thing.

@terrafrost terrafrost closed this Apr 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.