search

in2code-de / in2publish_core

in2publish Community Version
https://www.in2code.de/produkte/content-publisher/
GNU General Public License v3.0
40 stars 23 forks source link

RCE API not used for FilePublisherService::transferTemporaryFile() #24

Closed AndreasA closed 7 years ago

AndreasA commented 7 years ago

FilePublisherService::transferTemporaryFile() still uses SshConnection and not the new RCE API.

Also: RemoteStorage still creates a SshConnection instance which isn't used.

vertexvaar commented 7 years ago

Hey @AndreasA

it's awesome that you had such an deep look into the code. The RCE API is however not meant to transfer files. Its solely purpose is to execute commands on the CLI. I did this separation on purpose, because the RCE API is used in an early state and must not rely on the createMasks as SshConnection did. This also helps to prevent unrelated errors. There is currently not replacement for SshConnection::transferTemporaryFile because i did not come up with a full concept (which should also be bullet proof) on how to replace it with a better implementation. Therefore it's still used, as it has been and is still working (kind of 😄 ).

Concerning SshConnection and the RemoteStorage: Thank you for the hint, i removed it.

AndreasA commented 7 years ago

OK. However, SshConnection doesn't "sanitize" the SSH response. Like is done here: https://github.com/in2code-de/in2publish_core/blob/master/Classes/Communication/RemoteCommandExecution/RemoteCommandResponse.php#L139

This might result in invalid characters being in the response of FileCreateMask during setCreateMasks

EDIT: created own ticket for that: https://github.com/in2code-de/in2publish_core/issues/26