Skip to content

fixed implode arguments & .gitignore for phpstorm#972

Open
iz65535 wants to merge 1 commit into
mike42:developmentfrom
iz65535:development
Open

fixed implode arguments & .gitignore for phpstorm#972
iz65535 wants to merge 1 commit into
mike42:developmentfrom
iz65535:development

Conversation

@iz65535

@iz65535 iz65535 commented Sep 11, 2020

Copy link
Copy Markdown

i've tried to use DummyPrintConnector, but faced with error in implode function

@mike42 mike42 added the invalid label Jul 1, 2026
@mike42

mike42 commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Not shipping this - I think the code is fine - please provide steps to reproduce the error if this is mistaken.

@mike42 mike42 closed this Jul 1, 2026
@iz65535

iz65535 commented Jul 2, 2026

Copy link
Copy Markdown
Author

Hi there,
unfortunately after so many years i don't remember all the details, but i do believe that it would be clear enough, if you just take a look at MemoryPrintConnector ::finalize method:

 public function finalize(): void
    {
        $this -> buffer = null;
    }

this method is public. it puts null to buffer. everybody can call it.
when somebody calls ::getData later, it will throw a warning

php > implode(null);
PHP Warning:  Uncaught TypeError: implode(): If argument #1 ($separator) is of type string, argument #2 ($array) must be of type array, null given in php shell code:1
Stack trace:
#0 php shell code(1): implode()
#1 {main}
  thrown in php shell code on line 1

Another minor detail is, that buffer is declared as an array in class:

/**
     * @var array $buffer
     *  Buffer of accumilated data.
*/
    private $buffer;

which is obviously not true, because practically it can be null under certain circumstances

@mike42 mike42 reopened this Jul 3, 2026
@mike42

mike42 commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Oh yep, definitely could still be a bug there. Thanks for the explanation.

That should be enough information for me to confirm/fix. Re-opening this so it doesn't get lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants