Re: [tortoisesvn] r24285 committed - Smart-Bear Code collaborator integration, step 1: ...

On 04.06.2013 06:14, tortoisesvn_at_googlecode.com wrote:
> Revision: 24285
> Author: brunzefb
> Date: Mon Jun 3 21:14:13 2013
> Log: Smart-Bear Code collaborator integration, step 1:
> - Add resources for new menu
> - Create a helper class that will read persisted registry values that are
> used
> to build a command line string for ccollabgui.exe
> (this code has been tested and seems to work fine, it is not yet used in
> the log dialog)
> http://code.google.com/p/tortoisesvn/source/detail?r=24285

Some small problems here:

> +CodeCollaboratorInfo::CodeCollaboratorInfo(CString revisions)
> +{
> + PathToCollabGui =
> +
> CRegString(_T("Software\\TortoiseSVN\\CodeCollaborator\\PathToCollabGui"),
> _T(""));
> + CollabUser =

Don't use the _T() macro for new changes: just use L"" for strings.
There's no chance we're ever going to compile without _UNICODE defined.

> + PathToCollabGui.read();
> + CollabUser.read();
> + CollabPassword.read();
> + RepoUrl.read();
> + SvnUser.read();
> + SvnPassword.read();

Calling .read() is not necessary here. That's only necessary if you
think the registry value has changed between when you created the
registry object and the time you need the value.

> +CString CodeCollaboratorInfo::GetCommandLineArguments()
> +{
> + CString arguments;
> + arguments.Format(_T("--user %s --password %s --scm svn
> --svn-repo-url %s --svn-user %s --svn-passwd %s addchangelist new %s"),
> + (CString)CollabUser, (CString)CollabPassword, (CString)RepoUrl,
> (CString)SvnUser, (CString)SvnPassword, m_Revisions);
> + return arguments;
> +}

The Format() Method uses void parameters. So you must cast the CString
to an LPCWSTR, otherwise you pass the CString object as the %s instead
of the string: this works for win32 builds but it can fail for x64
builds since there the string data doesn't start at the CString object
start in RAM.
So you have to write:
(LPCWSTR)(CString)CollabUser
instead of just
(CString)CollabUser

> + CRegString PathToCollabGui;
> + CRegString CollabUser;
> + // passwords visible on cmd line anyways, so we don't encrypt :-(
> + CRegString CollabPassword;
> + CRegString SvnPassword;
> + CRegString RepoUrl;
> + CRegString SvnUser;

Storing username in the registry might be ok, but the password doesn't
belong there, at least not in plain text.
Use the CryptoAPI to encrypt/decrypt the password as needed. Have a look
at TSVNAuth.cpp, Creds::Decrypt() and Creds::Encrypt().

You should then ask the user for username/password if there's nothing
yet in the registry and then store the user data there, but encrypt the
password first.

And the repourl should best be stored in a project property instead: I
guess that's the same for all users but dependent on the working
copy/project.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=3057198
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].