Skip to content

Commit 211bfc9

Browse files
committed
TASK: 7.3 upmerges
2 parents b6bed6e + 9528c0b commit 211bfc9

File tree

9 files changed

+169
-103
lines changed

9 files changed

+169
-103
lines changed

Neos.Flow/Classes/Http/Helper/ResponseInformationHelper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ public static function makeStandardsCompliant(ResponseInterface $response, Reque
228228
}
229229
}
230230

231-
if (!$response->hasHeader('Content-Length')) {
231+
if (!$response->hasHeader('Content-Length') && $response->getBody()->getSize() !== null) {
232232
$response = $response->withHeader('Content-Length', $response->getBody()->getSize());
233233
}
234234

Neos.Flow/Classes/Mvc/ActionResponse.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ public function buildHttpResponse(): ResponseInterface
347347
*/
348348
private function hasContent(): bool
349349
{
350-
return $this->content->getSize() > 0;
350+
$contentSize = $this->content->getSize();
351+
return $contentSize === null || $contentSize > 0;
351352
}
352353
}

Neos.Flow/Classes/ResourceManagement/ResourceManager.php

Lines changed: 5 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,10 @@
1111
* source code.
1212
*/
1313

14-
use enshrined\svgSanitize\Sanitizer;
1514
use Neos\Flow\Annotations as Flow;
1615
use Neos\Flow\Log\Utility\LogEnvironment;
1716
use Neos\Flow\ObjectManagement\ObjectManagerInterface;
1817
use Neos\Flow\Persistence\PersistenceManagerInterface;
19-
use Neos\Utility\MediaTypes;
2018
use Neos\Utility\ObjectAccess;
2119
use Neos\Flow\ResourceManagement\Storage\StorageInterface;
2220
use Neos\Flow\ResourceManagement\Storage\WritableStorageInterface;
@@ -162,30 +160,14 @@ public function importResource($source, $collectionName = ResourceManager::DEFAU
162160
$collection = $this->collections[$collectionName];
163161

164162
try {
165-
if (is_resource($source)) {
166-
$mediaType = MediaTypes::getMediaTypeFromResource($source);
167-
if ($this->isSanitizingRequired($mediaType)) {
168-
$content = stream_get_contents($source);
169-
$resource = $this->importResourceFromContent($content, '', $collectionName, $forcedPersistenceObjectIdentifier);
170-
} else {
171-
$resource = $collection->importResource($source);
172-
}
173-
} else {
174-
$resource = fopen($source, 'rb');
175-
$mediaType = MediaTypes::getMediaTypeFromResource($resource);
176-
fclose($resource);
177-
if ($this->isSanitizingRequired($mediaType)) {
178-
$content = file_get_contents($source);
179-
$resource = $this->importResourceFromContent($content, '', $collectionName, $forcedPersistenceObjectIdentifier);
180-
} else {
181-
$resource = $collection->importResource($source);
182-
}
183-
$pathInfo = UnicodeFunctions::pathinfo($source);
184-
$resource->setFilename($pathInfo['basename']);
185-
}
163+
$resource = $collection->importResource($source);
186164
if ($forcedPersistenceObjectIdentifier !== null) {
187165
ObjectAccess::setProperty($resource, 'Persistence_Object_Identifier', $forcedPersistenceObjectIdentifier, true);
188166
}
167+
if (!is_resource($source)) {
168+
$pathInfo = UnicodeFunctions::pathinfo($source);
169+
$resource->setFilename($pathInfo['basename']);
170+
}
189171
} catch (Exception $exception) {
190172
throw new Exception(sprintf('Importing a file into the resource collection "%s" failed: %s', $collectionName, $exception->getMessage()), 1375197120, $exception);
191173
}
@@ -224,11 +206,6 @@ public function importResourceFromContent($content, $filename, $collectionName =
224206
throw new Exception(sprintf('Tried to import a file into the resource collection "%s" but no such collection exists. Please check your settings and the code which triggered the import.', $collectionName), 1380878131);
225207
}
226208

227-
$mediaType = MediaTypes::getMediaTypeFromFileContent($content);
228-
if ($this->isSanitizingRequired($mediaType)) {
229-
$content = $this->sanitizeImportedFileContent($mediaType, $content, $filename);
230-
}
231-
232209
/* @var CollectionInterface $collection */
233210
$collection = $this->collections[$collectionName];
234211

@@ -631,44 +608,6 @@ protected function initializeCollections()
631608
}
632609
}
633610

634-
/**
635-
* Decide weather the given media-type has to be sanitized
636-
* for now this only checks svg file to solve the issue here https://nvd.nist.gov/vuln/detail/CVE-2023-37611
637-
*
638-
* @todo create a feature from this and allow to register code for sanitizing file content before importing
639-
*/
640-
protected function isSanitizingRequired(string $mediaType): bool
641-
{
642-
return $mediaType === 'image/svg+xml';
643-
}
644-
645-
/**
646-
* Sanitize file content and remove content that is suspicious
647-
* for now this only checks svg file to solve the issue here https://nvd.nist.gov/vuln/detail/CVE-2023-37611
648-
*
649-
* @todo create a feature from this and allow to register code for sanitizing file content before importing
650-
*/
651-
protected function sanitizeImportedFileContent(string $mediaType, string $content, $filename = ''): string
652-
{
653-
if ($mediaType === 'image/svg+xml') {
654-
// @todo: Simplify again when https://github.com/darylldoyle/svg-sanitizer/pull/90 is merged and released.
655-
$previousXmlErrorHandling = libxml_use_internal_errors(true);
656-
$sanitizer = new Sanitizer();
657-
$sanitizedContent = $sanitizer->sanitize($content);
658-
libxml_clear_errors();
659-
libxml_use_internal_errors($previousXmlErrorHandling);
660-
$issues = $sanitizer->getXmlIssues();
661-
if ($issues && count($issues) > 0) {
662-
if ($sanitizedContent === false) {
663-
throw new Exception('Sanitizing of suspicious file "' . $filename . '" failed during import.', 1695395560);
664-
}
665-
$content = $sanitizedContent;
666-
$this->logger->warning(sprintf('Imported file "%s" contained suspicious content and was sanitized.', $filename), $issues);
667-
}
668-
}
669-
return $content;
670-
}
671-
672611
/**
673612
* Prepare an uploaded file to be imported as resource object. Will check the validity of the file,
674613
* move it outside of upload folder if open_basedir is enabled and check the filename.
Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
`7.3.17 (2023-11-22) <https://github.com/neos/flow-development-collection/releases/tag/7.3.17>`_
2+
================================================================================================
3+
4+
Overview of merged pull requests
5+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
6+
7+
`BUGFIX: Set InvalidHashException status code to 400 <https://github.com/neos/flow-development-collection/pull/3234>`_
8+
----------------------------------------------------------------------------------------------------------------------
9+
10+
``InvalidHashException`` now declares ``400`` as it's status code (not the inherited ``500`` it has now), as that is clearly a case of a "bad request".
11+
12+
* See: `#3159 <https://github.com/neos/flow-development-collection/issues/3159>`_
13+
14+
**Upgrade instructions**
15+
16+
This might need adjustment, if you rely on the ``InvalidHashException`` throwing a status code of ``500`` somewhere.
17+
18+
19+
* Packages: ``Flow``
20+
21+
`BUGFIX: Return the expected result for `is_dir('resource://sha1')` <https://github.com/neos/flow-development-collection/pull/3226>`_
22+
-------------------------------------------------------------------------------------------------------------------------------------
23+
24+
* Fixes: `#3225 <https://github.com/neos/flow-development-collection/issues/3225>`_
25+
26+
27+
* Packages: ``Flow``
28+
29+
`BUGFIX: Use method to set validated instances container <https://github.com/neos/flow-development-collection/pull/3210>`_
30+
--------------------------------------------------------------------------------------------------------------------------
31+
32+
* Fixes: `#3205 <https://github.com/neos/flow-development-collection/issues/3205>`_
33+
34+
35+
* Packages: ``Flow``
36+
37+
`BUGFIX: Require collection packages as `self.version` again <https://github.com/neos/flow-development-collection/pull/3206>`_
38+
------------------------------------------------------------------------------------------------------------------------------
39+
40+
* See: `#3035 <https://github.com/neos/flow-development-collection/issues/3035>`_ for the original change
41+
42+
43+
* Packages: ``Flow`` ``Eel`` ``FluidAdaptor`` ``Kickstarter``
44+
45+
`BUGFIX: Only set distinct on count clause if explicitely set to improve performance <https://github.com/neos/flow-development-collection/pull/3140>`_
46+
------------------------------------------------------------------------------------------------------------------------------------------------------
47+
48+
F.e. Postgres has performance issues with large datasets and the DISTINCT clause. In a test this change reduced the query time of a count query for ~900.000 entities by >80%.
49+
50+
In a custom project this affected their Neos Media.UI in which the following results were found:
51+
52+
* Count all assets | 580ms -> 260ms
53+
* Query 20 assets | 690ms -> 350ms
54+
* Query 100 assets | 990ms -> 650ms
55+
* Module load | 1900ms -> 1400ms
56+
57+
**Review instructions**
58+
59+
Everything should work the same, as https://github.com/neos/flow-development-collection/pull/415 already sets the distinct flag where (possibly) necessary.
60+
61+
62+
* Packages: ``Flow``
63+
64+
`BUGFIX: Sanitize uploaded svg files from suspicious content <https://github.com/neos/flow-development-collection/pull/3172>`_
65+
------------------------------------------------------------------------------------------------------------------------------
66+
67+
Adding an internal methods ``isSanitizingRequired`` and ``sanitizeImportedFileContent`` to the resourceManager. The import is adjusted to first determine the mediaType of an imported resource to decide wether sanitizing is needed which for now happens only for SVG files. If no sanitizing is needed the code will perform as before by passing streams or filenames around.
68+
69+
If suspicious content was removed from a warning is logged that mentions the remove data and line. The sanitizing is done using "enshrined/svg-sanitize" that is used by other cms aswell.
70+
71+
The initial implementation will only sanitize SVG files as those can contain malicious scripts. In future this should be expanded to a feature that allows registering of custom sanitizing functions.
72+
73+
The sanitizing logic itself ist basically the same as what is done by typo3 here: https://github.com/TYPO3/typo3/blob/`357b07064cf2c7f1735cfb8f73ac4a7248ab040e <https://github.com/neos/flow-development-collection/commit/357b07064cf2c7f1735cfb8f73ac4a7248ab040e>`_/typo3/sysext/core/Classes/Resource/Security/SvgSanitizer.php
74+
75+
This addresses the issue described here: https://nvd.nist.gov/vuln/detail/CVE-2023-37611
76+
77+
**Review Instructions**
78+
79+
The change adds quite a bit of complexity to the importResource method to avoid loading the file content into ram whenever possible. As this method accepts filenames and resources this leads to quite some nested checking. I consider this kindoff necessary as one does not want to read a full video file into php ram to check wether it may be an svg.
80+
81+
Better suggestions are welcome.
82+
83+
84+
* Packages: ``Utility.MediaTypes``
85+
86+
`TASK: Update default .htaccess for _Resources <https://github.com/neos/flow-development-collection/pull/3238>`_
87+
----------------------------------------------------------------------------------------------------------------
88+
89+
PHP 5 is a thing of the past, but for PHP 8 the module is name just ``mod_php.c``, so that needs to be added.
90+
91+
**Upgrade instructions**
92+
93+
Depending in the way you deploy and whether you have that file even in version control, the change might need to be applied manually to your setup.
94+
95+
**Review instructions**
96+
97+
98+
* Packages: ``Flow``
99+
100+
`TASK: Routing Documentation Adjustment <https://github.com/neos/flow-development-collection/pull/3231>`_
101+
----------------------------------------------------------------------------------------------------------
102+
103+
Correction of an erroneous path in routing documentation.
104+
105+
* Packages: ``Flow``
106+
107+
`TASK: PEG Parser declares properties <https://github.com/neos/flow-development-collection/pull/3215>`_
108+
-------------------------------------------------------------------------------------------------------
109+
110+
Prevents deprecation warnings for dynamic properties.
111+
112+
* Packages: ``Flow`` ``Eel``
113+
114+
`TASK: Clean up stored throwable dumps <https://github.com/neos/flow-development-collection/pull/3187>`_
115+
--------------------------------------------------------------------------------------------------------
116+
117+
Whenever a new dump is written, check the existing dumps and remove those that are older than allowed or exceed the maximum count.
118+
119+
By default nothing is cleaned up.
120+
121+
* Resolves: `#3158 <https://github.com/neos/flow-development-collection/issues/3158>`_
122+
123+
**Review instructions**
124+
125+
Should remove old dump files as configured…
126+
127+
128+
* Packages: ``Flow``
129+
130+
`TASK: Fix overlooked dependency… <https://github.com/neos/flow-development-collection/pull/3207>`_
131+
-----------------------------------------------------------------------------------------------------
132+
133+
* See: `#3035 <https://github.com/neos/flow-development-collection/issues/3035>`_ for the original change
134+
135+
136+
* Packages: ``Flow``
137+
138+
`TASK: Fix cache RedisBackend unittest <https://github.com/neos/flow-development-collection/pull/3196>`_
139+
--------------------------------------------------------------------------------------------------------
140+
141+
A test failed due to a missing return value from a method not being mocked (correctly),
142+
143+
144+
* Packages: ``Cache``
145+
146+
`TASK: Fix documentation builds <https://github.com/neos/flow-development-collection/pull/3195>`_
147+
-------------------------------------------------------------------------------------------------
148+
149+
… by pinning updated dependencies.
150+
151+
**Review instructions**
152+
153+
Best is to see if the builds succeed on RTD again with this merged…
154+
155+
156+
* Packages: ``Flow``
157+
158+
`Detailed log <https://github.com/neos/flow-development-collection/compare/7.3.16...7.3.17>`_
159+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Neos.Flow/Documentation/requirements.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,5 @@ sphinxcontrib-qthelp==1.0.6
5858
# via sphinx
5959
sphinxcontrib-serializinghtml==1.1.9
6060
# via sphinx
61-
urllib3==2.0.6
61+
urllib3==2.0.7
6262
# via requests

Neos.Flow/composer.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@
5151

5252
"composer/composer": "^2.2.8",
5353

54-
"egulias/email-validator": "^2.1.17 || ^3.0",
55-
"enshrined/svg-sanitize": "^0.16.0"
54+
"egulias/email-validator": "^2.1.17 || ^3.0"
5655
},
5756
"require-dev": {
5857
"mikey179/vfsstream": "^1.6.10",

Neos.Utility.MediaTypes/Classes/MediaTypes.php

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1830,21 +1830,6 @@ public static function getMediaTypeFromFileContent(string $fileContent): string
18301830
return isset(self::$mediaTypeToFileExtension[$mediaType]) ? $mediaType : 'application/octet-stream';
18311831
}
18321832

1833-
/**
1834-
* Returns a Media Type based on the given resource
1835-
*
1836-
* @param resource $resource The resource to determine the media type from
1837-
* @return string The IANA Internet Media Type
1838-
*/
1839-
public static function getMediaTypeFromResource($resource): string
1840-
{
1841-
if (!is_resource($resource)) {
1842-
throw new \TypeError('Argument "resource" has to be a resource');
1843-
}
1844-
$mediaType = self::trimMediaType(mime_content_type($resource));
1845-
return isset(self::$mediaTypeToFileExtension[$mediaType]) ? $mediaType : 'application/octet-stream';
1846-
}
1847-
18481833
/**
18491834
* Returns the primary filename extension based on the given Media Type.
18501835
*

Neos.Utility.MediaTypes/Tests/Unit/MediaTypesTest.php

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,22 +68,6 @@ public function getMediaTypeFromFileContent(string $filename, string $expectedMe
6868
self::assertSame($expectedMediaType, MediaTypes::getMediaTypeFromFileContent($fileContent));
6969
}
7070

71-
/**
72-
* @test
73-
* @dataProvider filesAndMediaTypes
74-
*/
75-
public function getMediaTypeFromResource(string $filename, string $expectedMediaType)
76-
{
77-
$filePath = __DIR__ . '/Fixtures/' . $filename;
78-
$resource = is_file($filePath) ? fopen($filePath, 'rb') : fopen('data://text/plain,', 'rb');
79-
if ($resource !== false) {
80-
self::assertSame($expectedMediaType, MediaTypes::getMediaTypeFromResource($resource));
81-
fclose($resource);
82-
} else {
83-
$this->fail('fixture ' . $filePath . ' could not be read');
84-
}
85-
}
86-
8771
/**
8872
* Data Provider
8973
*/

composer.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
"neos/composer-plugin": "^2.0",
3434
"composer/composer": "^2.2.8",
3535
"egulias/email-validator": "^2.1.17 || ^3.0",
36-
"enshrined/svg-sanitize": "^0.16.0",
3736
"typo3fluid/fluid": "~2.7.0",
3837
"guzzlehttp/psr7": "^1.8.4 || ^2.1.1",
3938
"ext-mbstring": "*"

0 commit comments

Comments
 (0)