Re: [Kolab-devel] Guam patches

Christoph Erhardt <[email protected]> Mon, 07 May 2018 12:52:19 +0200

Newsgroups	gmane.comp.kde.devel.kolab
Message-ID	<1541574.zAoRmQbfkq__40742.0934564017$1525690251$gmane$org@delle>

Quick follow-up: I just saw that hede has already analysed that issue with 
Kolab Notes and posted a patch to this list in December 2017.

I suppose we should use his patch (attached for reference) as a starting point 
for further discussion. I'll try it out in the next few days and see whether 
it still applies cleanly and fixes the problem for me.

Cheers,
Christoph

On Monday, 7 May 2018 12:22:28 CEST Christoph Erhardt wrote:
> Hi Aaron,
> 
> thanks for your answer! Sorry about the "sucks" part; it's just that I'm
> currently a bit frustrated that Guam is still not working reliably enough,
> after all the effort I've put into packaging it. :-/
> 
> As far as I can tell, I'm already using all the patches you mentioned: I'm
> on Guam 0.9.4 (released February 2018) with a patched erlang-eimap 0.4.0
> (the diff being the one you posted to this mailing list in December 2017).
> 
> Here are the links to my respective package sources:
> https://obs.kolabsys.com/package/show/home:sicherha:branches:Kolab:16/guam
> https://obs.kolabsys.com/package/show/home:sicherha:branches:Kolab:16/erlang
> -eimap
> 
> The only relevant upstream change to Guam since your departure has been this
> one:
> https://git.kolab.org/rG7db53d9c3fc00baa1fe4fb8ca7c9ae63c61e853b ("Allow
> empty lines in commands.")
> 
> Are there any other patches still floating around or have I hit a new issue
> indeed? In the latter case, I'd be happy to provide you with all the
> information necessary to debug this.
> 
> Cheers,
> Christoph

_______________________________________________
devel mailing list
[email protected]
https://lists.kolab.org/mailman/listinfo/devel

guam-0.9.2-stalling-client-buffer-and-split-command-handling.patch (text/x-patch, 5 KB)

diff -rupN guam-0.9.2/apps/kolab_guam/src/kolab_guam_session.erl guam-0.9.2-mic2/apps/kolab_guam/src/kolab_guam_session.erl
--- guam-0.9.2/apps/kolab_guam/src/kolab_guam_session.erl	2017-03-21 10:20:48.000000000 +0000
+++ guam-0.9.2-mic2/apps/kolab_guam/src/kolab_guam_session.erl	2017-11-30 19:16:12.994244345 +0000
@@ -195,25 +195,47 @@ process_client_data(Socket, Data, #state
             { TLS, Socket, NewInflator, NewDeflator, UndecidedRules, ActiveRules, <<>>, undefined, undefined };
         nochange ->
             %%lager:debug("... now applying rules"),
+            %lager:info("now applying rules..."),
             { ModifiedData, NewSplitCommand, NewSplitResetTrigger, NewUndecidedRules, NewActiveRules, PostAction } = apply_ruleset_clientside(ImapSession, Socket, PreprocessData, CurrentCommandSplit, UndecidedRules, ActiveRules),
             %%lager:info("The modified data is: ~s", [ModifiedData]),
             %lager:info("The post-processed data is: ~s", [PostProcessed]),
             BufferThisData =
             case PostAction of
                 perform_passthrough ->
+                    %lager:info("sending (no buffer): ~s", [ModifiedData]),
                     eimap:passthrough_data(ImapSession, ModifiedData),
                     <<>>;
                 buffer_data ->
-                    Data
+                    %Data
+                    % Originally Aaron uses Data here, but later on this buffer is assumed to be
+                    % already decoded, so we do have to use PreprocessData here, I think.
+                    case binary:matches(PreprocessData, <<"\r\n">>) of
+                        [] -> 
+                            lager:info("buffering: ~s", [PreprocessData]),
+                            PreprocessData;
+                        List ->
+                            {FoundPos, _} = lists:last(List),
+                            % I would like to have some binary:match for the last instead of the
+                            % first occurrence; but I'm really inexperienced in erlang so I don't 
+                            % know how to solve this efficient, so I'm using binary:matches with
+                            % using the last element only
+                            SplitPos = FoundPos + 2,
+                            eimap:passthrough_data(ImapSession, binary:part(PreprocessData, 0, SplitPos)),
+                            %lager:info("sending first part: ~s", [binary:part(PreprocessData, 0, SplitPos)] ),
+                            %lager:info("buffering second part: ~s", [binary:part(PreprocessData, SplitPos, size(PreprocessData)-SplitPos)]),
+                            binary:part(PreprocessData, SplitPos, size(PreprocessData)-SplitPos)
+                    end
             end,
             { TLS, Socket, Inflator, Deflator, NewUndecidedRules, NewActiveRules, BufferThisData, NewSplitCommand, NewSplitResetTrigger }
     end,
     set_socket_active(TLSActive, CurrentSocket),
-    PrevBuffered = State#state.buffered_client_data,
+    %buffered_client_data is already in DataToBuffer via preprocess_client_data
+    %PrevBuffered = State#state.buffered_client_data,
     { noreply, State#state{ rules_deciding = CurrentUndecidedRules, rules_active = CurrentActiveRules,
                             socket = CurrentSocket, client_tls_active = TLSActive,
                             inflator = CurrentInflator, deflator = CurrentDeflator,
-                            buffered_client_data = <<PrevBuffered/binary, DataToBuffer/binary>>,
+                            %buffered_client_data = <<PrevBuffered/binary, DataToBuffer/binary>>,
+                            buffered_client_data = <<DataToBuffer/binary>>,
                             current_command_split = SplitCommand,
                             command_split_reset_trigger = SplitResetTrigger } }.
 
@@ -261,7 +283,7 @@ apply_next_rule_serverside(ImapSession,
     apply_next_rule_serverside(ImapSession, ModifiedData, [{ Module, ModifiedRuleState } | ActiveRulesAcc], ActiveRules).
 
 apply_ruleset_clientside(_ImapSession, _Socket, ClientData, _CurrentCommandSplit, [], []) ->
-    { ClientData, [], [], [], [], perform_passthrough };
+    { ClientData, undefined, [], [], [], perform_passthrough }; 
 apply_ruleset_clientside(ImapSession, Socket, ClientData, CurrentCommandSplit, UndecidedRules, CurrentlyActiveRules) ->
     { PostAction, SplitCommand, SplitResetTrigger } =
         case CurrentCommandSplit of
diff -rupN guam-0.9.2/debian/changelog guam-0.9.2-mic2/debian/changelog
--- guam-0.9.2/debian/changelog	2017-12-01 10:23:48.000000000 +0000
+++ guam-0.9.2-mic2/debian/changelog	2017-11-30 17:32:36.954428454 +0000
@@ -1,3 +1,15 @@
+guam (0.9.2-3mic2) unstable; urgency=medium
+
+  * split command handling
+
+ -- hede <[email protected]>  Thu, 30 Nov 2017 18:36:43 +0100
+
+guam (0.9.2-3mic1) unstable; urgency=medium
+
+  * workaround for stalling client buffer 
+
+ -- hede <[email protected]>  Thu, 30 Nov 2017 17:30:23 +0100
+
 guam (0.9.2-3) unstable; urgency=medium
 
   * Rebuild

signature.asc (application/pgp-signature, 833 B)