> On des. 15, 2014, 10:45 p.m., Àlex Fiestas wrote:
> > Code looks good. 
> > 
> > Could you perhaps add an integration test for this? Since we are 
> > "abstracted" by the socket it should be possible. If it is too much work 
> > feel free to push it.
> 
> Martin Gräßlin wrote:
>     what do you want the integration test to test? I certainly can start the 
> daemon but I'm not sure what it would give us as the only way to return from 
> it requires a password. And that's what the test application in tests already 
> does.

Well, this patch adds a lot of new logic that can be tested, since it does not 
have unit test (and doing them in ksmserver migh prove difficult) we can test 
the code with an integraiton test.

I see lots of socket logic
I see logic in addAllowedWindow
And also the biggest method setKsldSocket which has 2 lambdas that (afaik) 
can't be tested in any other way.


- Àlex


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


On des. 15, 2014, 9:29 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121429/
> -----------------------------------------------------------
> 
> (Updated des. 15, 2014, 9:29 a.m.)
> 
> 
> Review request for Plasma, Àlex Fiestas and David Edmundson.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> The screenlocker_greet needs to tell the parent ksld process which
> windows it created. Ksld sends input events to these windows. So
> far this was based on an X property on the window. Unfortunately
> ksld didn't validate whether the windows tagged with this property
> belong to the screenlocker_greet process it started.
> 
> With this change the communication for announcing windows is moved
> away from the X11 protocol and instead a custom Wayland protocol is
> used.
> 
> Ksld starts a KWaylandServer when the greet process gets started. It
> creates anonymous unix sockets for the connection and passes one
> filedescriptor to the started greeter process.
> 
> The check for the X property is removed in ksld and instead only
> windows ids passed through the Wayland socket connection are
> accepted.
> 
> 
> Diffs
> -----
> 
>   ksmserver/screenlocker/ksldapp.cpp 22698ce37e9d4be17126111b3ded8133f7c3baa6 
>   ksmserver/screenlocker/lockwindow.h 
> 9938d201269c89a24c9c0bd6275aa5f731bb5535 
>   ksmserver/screenlocker/lockwindow.cpp 
> 3aa963a59e21636862f5ca59e220bbea3bd41ff9 
>   ksmserver/screenlocker/protocols/ksld.xml PRE-CREATION 
>   ksmserver/screenlocker/waylandserver.h PRE-CREATION 
>   ksmserver/screenlocker/waylandserver.cpp PRE-CREATION 
>   ksmserver/screenlocker/greeter/greeterapp.h 
> b92b13b63365a9026dba5d71b772dcd8c9ee3d3b 
>   ksmserver/screenlocker/greeter/greeterapp.cpp 
> 30d1821bdba38028959f3457e900a1b32e628192 
>   ksmserver/screenlocker/greeter/main.cpp 
> 12e570107d0cba851b8978131d730b27924529bb 
>   ksmserver/screenlocker/ksldapp.h 095424c9845c134aa156917aeb6c8ddf31e8d25a 
>   CMakeLists.txt c6d89c14b05f5639937aee5692d305fa2faed974 
>   ksmserver/screenlocker/CMakeLists.txt 
> 5378a10df2be70cee95b5612c23046eae639f610 
>   ksmserver/screenlocker/greeter/CMakeLists.txt 
> 10c473488f08354096f68784b9240392a444af5f 
> 
> Diff: https://git.reviewboard.kde.org/r/121429/diff/
> 
> 
> Testing
> -------
> 
> Running ksmserver with the patch. Lock/unlock working, my exploit is failing.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

_______________________________________________
Plasma-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to