fix bug with git after update to Plasma 5.9 in neon
ClosedPublic

Authored by idekels on Feb 10 2017, 1:59 AM.

Details

Summary

After an update to Plasma 5.9 in KDE Neon, ksshaskpass stop working.
After diging a little on the web, I found that this bug "https://www.mail-archive.com/kde-bugs-dist@kde.org/msg117934.html"
was the problem and I fixed.

Diff Detail

Repository
R105 KDE SSH Password Dialog
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
idekels retitled this revision from to fix bug with git after update to Plasma 5.9 in neon.Feb 10 2017, 1:59 AM
idekels updated this object.
idekels edited the test plan for this revision. (Show Details)
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 10 2017, 1:59 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
cfeck added a subscriber: cfeck.

Thanks, this looks good so far, but adding more eyes, because it touches privacy sensitive code.

cfeck added a comment.Feb 10 2017, 2:30 AM

One issue: The comments for the change that broke this state that 'sshaskpass' has no translations, so the prompt is the same in any locale.

Could you verify if 'git' uses the same prompt in any language/locale?

I could not tell, I've tried changing git's language/locale but apparently there is no easy way to do that.

bbuch requested changes to this revision.Feb 10 2017, 10:34 AM
bbuch added a reviewer: bbuch.
bbuch added a subscriber: bbuch.

This doesn't work for Usernames and the resulting keyFile is different from the old behavior. (It contains the closing ": ")

Please change the regular expression to:

QRegularExpression re3("^(Password|Username) for (.*?)[:] $");

and map the keyFile to:

keyFile = match3.captured(2);

Apart from that, thank you very much, great work! :-)

I use git with german locals and it does work.

This revision now requires changes to proceed.Feb 10 2017, 10:34 AM
idekels updated this revision to Diff 11164.Feb 10 2017, 12:13 PM
  • changes requested by bbuch
  1. Updating D4540: fix bug with git after update to Plasma 5.9 in neon #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

change regex to also capture username and comply with old behavior

mpyne accepted this revision.Feb 10 2017, 11:54 PM

The change is fine as far as privacy impact, it shouldn't add any issues that aren't already present.

bbuch accepted this revision.Feb 10 2017, 11:57 PM
This revision is now accepted and ready to land.Feb 10 2017, 11:57 PM

The duplicate bug 376318 does not mention 'git', but simply opening a (secure) shell. Do both use the same 'ssh-add' command? If yes, then the comment mentioning 'git' might be misleading.

Can you change the commit title and message to be a little more descriptive than "fix bug after git update to 5.9", like "Fix password extraction from blabla"

idekels updated this revision to Diff 11212.Feb 11 2017, 1:10 PM
  1. Updating D4540: Fix password extraction from git after update to Plasma 5.9 in neon #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

Fix password extraction from git after update to Plasma 5.9 in neon
This bug caused ksshaskpass to ask for the password and/or username every time without storing it in kwallet

cfeck requested changes to this revision.Feb 11 2017, 1:30 PM
cfeck added a reviewer: cfeck.

The patch is borked. Did you swap branches?

This revision now requires changes to proceed.Feb 11 2017, 1:30 PM

This is my first time using this tool, I am so sorry, I was trying to change the commit message.

cfeck added a comment.Feb 17 2017, 1:31 AM

Any update?

I don't know what to do next. should I try change the message again?

cfeck added a comment.Feb 17 2017, 3:15 AM

Use "Update Diff" to upload the correct diff. If you switched between different branches (as the current diff suggests), please make sure you solve the git issues first. If everything else fails, "git reset origin/master", and re-apply your changes.

idekels updated this revision to Diff 11427.Feb 17 2017, 3:39 AM

Fix password extraction from git after update to Plasma 5.9 in neon
This bug caused ksshaskpass to ask for the password and/or username every time without storing it in kwallet

cfeck added inline comments.Feb 17 2017, 3:47 AM
src/main.cpp
70

So are these strings really specific to git? Also, it makes no sense to mention the Plasma version or distribution this bug appears, if you want some explanation, either add the specifics in the comment, or just reference the KDE bug number.

idekels updated this revision to Diff 11428.Feb 17 2017, 4:36 AM
  • Fix password extraction from git. bug number 376228
  • remove Plasma version and distribution from comment

I suppose that it could work for other apps as well but I only use this program for git

cfeck accepted this revision.Feb 17 2017, 4:46 AM

Thanks, looks good now. Do you have commit rights? If not, I will commit it on your behalf.

This revision is now accepted and ready to land.Feb 17 2017, 4:46 AM

I think my commits are right now, but go ahead, it's probably better!

This revision was automatically updated to reflect the committed changes.