[Kde-pim] Review Request 124179: Kleo: Improve error handling of SignEncryptFilesTask

[Sandro Knauß]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124179/#review81824
-----------------------------------------------------------


It would be nice, if you would also add tests for it. Especially because your changes are done for 4.14 and need to be ported to KF5.


kleopatra/crypto/signencryptfilestask.cpp (line 523)
<https://git.reviewboard.kde.org/r/124179/#comment56162>

    I think the following is better understandable, because we dont have this if,elseif,else structure:
      
        if ( sresult.error().code() || eresult.error().code() ) {
            output->cancel();
            outputCreated = false;
        } else {
            kleo_assert( !sresult.isNull() || !eresult.isNull() );
        }
        
        if ( !outputCreated || finishIO() ) {     // emit result if either no output where created or the IO operation are finished
            q->emitResult( shared_ptr<Result>( new SignEncryptFilesResult( sresult, eresult, input, output, removeInput, outputCreated, auditLog ) ) );
        }
        
    maybe also rename outputCreated with canceled, because we wanna mark that the operationwas canceled.


- Sandro Knauß


On Juni 25, 2015, 5:45 nachm., Andre Heinecke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124179/
> -----------------------------------------------------------
> 
> (Updated Juni 25, 2015, 5:45 nachm.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Repository: kdepim
> 
> 
> Description
> -------
> 
> This deduplicates the IO Finalization and additionally checks
> for input failures.
> 
> As errors from preprocessing commands can lead to an
> empty input the crypto jobs appearently succeed (as they work
> on empty input) but create empty files. 
> 
> With removeInput set to true there is the possibility of
> data loss in that case.
> 
> The new method finishIO handles those errors. Additionally
> errors that occur when removing input files are no longer
> silently discarded.
> 
> See: https://bugs.g10code.com/gnupg/issue1624 for
> the report on input error issue
> 
> 
> Diffs
> -----
> 
>   kleopatra/crypto/signencryptfilestask.cpp abfa6e4 
>   kleopatra/utils/input.h 0a4a930 
>   kleopatra/utils/input.cpp 751e6f1 
> 
> Diff: https://git.reviewboard.kde.org/r/124179/diff/
> 
> 
> Testing
> -------
> 
> The input error handling has been part of the last gpg4win version (relased in march).
> 
> I've also tested this with KDE/4.14 under linux by trying to archive / encrypt files without read permission.
> Tested removeInput error handling by creating a read only folder and trying to remove files after encryption in there.
> 
> 
> Thanks,
> 
> Andre Heinecke
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/