Repository navigation
Edge cases looping forever or failing with a TypeError: FileSplitterTask, PropertySetterTask, SlugifyTransformer, InputFolderBrowserTask, XmlFile #232
Description
Description
Several edge cases found while fixing #205, #203, #204, #208 and #202 either loop forever or fail with a raw PHP TypeError instead of an explicit error:
| # | Where | Problem |
|---|---|---|
| 1 | FileSplitterTask |
max_lines lower than 1 is accepted and makes the task loop forever (each produced file is empty and the source file is never consumed). Values given as input ({ file_path: ..., max_lines: ... }) are merged after the option resolution and never validated: max_lines: 0 also loops forever, max_lines: 'abc' throws TypeError: FileSplitterTask::splitFile(): Argument #2 ($maxLines) must be of type int, string given |
| 2 | PropertySetterTask |
Only \Exception is caught: on a scalar input, the TypeError thrown by the PropertyAccessor (setValue(): Argument #1 ($objectOrArray) must be of type object|array, string given) bypasses the error handling of the task, so the property / value error context is not set |
| 3 | SlugifyTransformer |
No setAllowedTypes(): transliterator: 123 throws TypeError: Transliterator::create(): Argument #1 ($id) must be of type string, int given, separator: [] throws TypeError: trim(): Argument #2 ($characters) must be of type string, array given (same for replace) |
| 4 | InputFolderBrowserTask |
An empty input (null, '') with no folder being browsed calls is_dir(null): TypeError: is_dir(): Argument #1 ($filename) must be of type string, null given |
| 5 | XmlFile::write() (XmlWriterTask) |
The result of DOMDocument::saveXML(), which can be false, is passed as is to fwrite(): TypeError: SplFileObject::fwrite(): Argument #1 ($data) must be of type string, false given instead of the intended RuntimeException |
All reproduced on main (6ebfb14), PHP 8.5, Symfony 7.4, by calling the tasks / transformers directly (the unit tests added by the PR reproduce each case).
Proposed fix
FileSplitterTask:max_linesmust be an integer greater than 0 (setAllowedValues()), and the options given as input are validated like the task options (resolved again after the merge; input keys that are not options are ignored).PropertySetterTask: catch\Throwable, so that the error context is set for any failure.SlugifyTransformer:setAllowedTypes('string')ontransliterator,replaceandseparator(as documented).InputFolderBrowserTask: throw an explicit\UnexpectedValueException('No folder path given as input').XmlFile::write(): throw a\RuntimeException('Could not generate the XML content')whensaveXML()fails.- Update the reference documentation (
file_splitter_task.md,property_setter_task.md,slugify_transformer.md,input_folder_browser_task.md,xml_writer_task.md), add tests.
Requirements
- Documentation updates
- Reference
- Cookbooks
- Changelog
- Unit tests
Breaking changes
None: every affected case currently loops forever or throws a TypeError; it now throws an explicit exception (an InvalidOptionsException when resolving the options, for 1 and 3). The only other change: for FileSplitterTask, input keys that are not options are no longer merged into the options (they were not used).
Activity
Merge pull request #233 from cleverage/232