Bug #94422
closedwrong handling in TYPO3\CMS\Core\Http\SelfEmittableLazyOpenStream
100%
Description
The class SelfEmittableLazyOpenStream has one issue like reported in the comments here https://forge.typo3.org/issues/91826 but furthermore another one.
In bug 91826 I mention the context and won't repeat it here.
Here is the additional bug:
The method write($string) is not according to the interface Psr\Http\Message\StreamInterface and must return int.
Therefore it must not throw any exception. As the class is used for Ajax requests exceptions are not shown and make debugging quite hard anyway.
Having tried to return just 0 I never got the installtool running like expected.
Returning just strlen($string) instead worked so far that the install-process finished by writing in LocalConfiguration.php and the backend could be opened.
I don't know which string is coming in, if it could include multi-byte-signs and therefore return a wrong strlen.
Also I never verified the usage of the returned value.
Therefore my solution is a workaround for my own case but it has to be verified if it could be used in general.
Updated by David Bruchmann over 3 years ago
- Related to Bug #91826: Install issue in TYPO3 10/11 added
Updated by David Bruchmann over 3 years ago
The Exception in the method write($string) seems being correct according to the StreamInterface, nevertheless it had to be thrown conditional and not always.
Here is the method in the StreamInterface:
/** * Write data to the stream. * * @param string $string The string that is to be written. * @return int Returns the number of bytes written to the stream. * @throws \RuntimeException on failure. */ public function write($string);
and below is shown how it's implemented in TYPO3:
/** * @param string $string * @throws \RuntimeException on failure. */ public function write($string) { throw new \RuntimeException('Cannot write to a ' . self::class, 1538331833); }
Following is my workaround to get it running:
/** * @param string $string * @return int Returns the number of bytes written to the stream. * @throws \RuntimeException on failure. */ public function write($string) : int { // throw new \RuntimeException('Cannot write to a ' . self::class, 1538331833); return strlen($string); }
Updated by David Bruchmann over 3 years ago
Having discovered the trait https://github.com/guzzle/psr7/blob/148492f9e2e1524317a8aa60875f5b59517a9374/src/StreamDecoratorTrait.php which I did miss before, I see that the implementation in TYPO3 just by extending GuzzleHttp\Psr7\LazyOpenStream might be wrong.
The class LazyOpenStream serves as stream-wrapper and just returns what a stream-class is returning. Therefore any streams should be transferred as argument and not by extending the class.
Updated by Christian Kuhn over 3 years ago
Ok, guzzle/psr7 2.0-dev declares LazyOpenStream final. That's why it fails - we can't extend that class anymore.
This did not pop up in core testing yet, since we don't install -dev dependencies in our 'composer max' suites, and you probably shouldn't, either. Please stick to non-dev dependencies, core gives no guarantees whatsoever on this.
Of course, we should still change our SelfEmittableLazyOpenStream to deal with this dependency change, but it's not a blocker (currently, at least until 2.0.0 is out). For v10, we might be better off with defining guzzle/psr7 2.0 as conflict and adapt only v11 to be later compatible with guzzle/psr7 when 2.0.0 is released.
Updated by Christian Kuhn over 3 years ago
Easiest solution for both v10 and v11:
v10:
composer --req guzzle/psr7:^1.4.0
v11:
composer --req guzzle/psr7:^1.7.0
Both are the current lower bounds given by current v10 / v11 guzzlehttp/guzzle dependency.
This will deny 2.0 and also declares that we actively use guzzle/psr7 as dependency, which we do, since we extend one of its classes.
Agreed?
Updated by Christian Kuhn over 3 years ago
Above notes are more for the 'final' report from https://forge.typo3.org/issues/91826#note-3 - still unsure if there is more todo. I'd say we should block 2.0 for now, then see what happens. ok?
Updated by David Bruchmann over 3 years ago
Sounds reasonable to block v2.0 first as quick fix.
I think an update should be target in mid-term though.
I haven't verified the differences between the versions so I don't know if v1.x has really disadvanteges.
Christian, you described it more detailed how to handle it, that's completely fine I think.
Updated by Gerrit Code Review over 3 years ago
- Status changed from New to Under Review
Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/69666
Updated by Gerrit Code Review over 3 years ago
Patch set 1 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/69667
Updated by Gerrit Code Review over 3 years ago
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/69668
Updated by Christian Kuhn over 3 years ago
Pushed patches for master, v10 and even v9 since a guzzlehttp/psr7 2.0.0 would break v9 otherwise.
David, would you like to test that? It should actively downgrade your guzzlehttp/psr7 to 1.something, probably 1.8.2.
Updated by Christian Kuhn over 3 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 2b1d7e62e597337c14d1f69af8d5494d16f29c02.
Updated by Christian Kuhn over 3 years ago
- Related to Bug #94454: SelfEmittableLazyOpenStream is not compatible with guzzlehttp/psr7 2.0.0 added
Updated by Christian Kuhn over 3 years ago
- Related to Task #94800: Update codeception dependencies added
Updated by David Bruchmann over 3 years ago
The method write($string) still has to be changed.
Btw. according to the interface it must throw an exception but return int if I remember correctly.