:?: Patch for ZConnection.Connect

Code patches written by our users to solve certain "problems" that were not solved, yet.

Moderators: gto, cipto_kh, EgonHugeist, mdaems

Ostfriese
Junior Boarder
Junior Boarder
Posts: 25
Joined: 12.05.2009, 10:48
Location: Coburg
Contact:

:?: Patch for ZConnection.Connect

Postby Ostfriese » 12.05.2009, 10:58

Currently the username and password properties are not updated if they were changed in the login dialog. So I've modified

Code: Select all

procedure TZConnection.Connect;
var
//Local variables declared in order to preserve the original property value
//and to avoid the storage of password
  Username, Password: string;
begin
  if FConnection = nil then
  begin
// Fixes Zeos Bug 00056
//    try
      DoBeforeConnect;
//    except
//This is here to support aborting the Connection in BeforeConnect event without fatal errors
//      on E: EAbort do
//        Exit;
//    end;

    UserName := FUser;
    Password := FPassword;

    if FLoginPrompt then
    begin
      { Defines user name }
      if UserName = '' then
        UserName := FProperties.Values['UID'];
      if UserName = '' then
        UserName := FProperties.Values['username'];

      { Defines user password }
      if Password = '' then
        Password := FProperties.Values['PWD'];
      if Password = '' then
        Password := FProperties.Values['password'];

      if Assigned(FOnLogin) then
        FOnLogin(Self, UserName, Password)
      else
      begin
        if Assigned(LoginDialogProc) then
        begin
          if not LoginDialogProc(FDatabase, UserName, Password) then
            Exit
        end
        else
          raise Exception.Create(SLoginPromptFailure);
      end;
    end;

    ShowSqlHourGlass;
    try
      FConnection := DriverManager.GetConnectionWithParams(
        ConstructURL(UserName, Password), FProperties);
      try
        with FConnection do
        begin
          SetAutoCommit(FAutoCommit);
          SetReadOnly(FReadOnly);
          SetCatalog(FCatalog);
          SetTransactionIsolation(FTransactIsolationLevel);
          Open;
        end;
      except
        FConnection := nil;
        raise;
      end;
    finally
      HideSqlHourGlass;
    end;

    if not FConnection.IsClosed then
      DoAfterConnect;
  end;
end;

to

Code: Select all

procedure TZConnection.Connect;
var
//Local variables declared in order to preserve the original property value
//and to avoid the storage of password
  Username, Password: string;
begin
  if FConnection = nil then
  begin
// Fixes Zeos Bug 00056
//    try
      DoBeforeConnect;
//    except
//This is here to support aborting the Connection in BeforeConnect event without fatal errors
//      on E: EAbort do
//        Exit;
//    end;

    UserName := FUser;
    Password := FPassword;

    if FLoginPrompt then
    begin
      { Defines user name }
      if UserName = '' then
        UserName := FProperties.Values['UID'];
      if UserName = '' then
        UserName := FProperties.Values['username'];

      { Defines user password }
      if Password = '' then
        Password := FProperties.Values['PWD'];
      if Password = '' then
        Password := FProperties.Values['password'];

      if Assigned(FOnLogin) then
        FOnLogin(Self, UserName, Password)
      else
      begin
        if Assigned(LoginDialogProc) then
        begin
          if not LoginDialogProc(FDatabase, UserName, Password) then
            Exit
          else
          //HA 090512 Update username and password after login dialog closed
          begin
            FUser := UserName;
            FPassword := Password;
          end;
        end
        else
          raise Exception.Create(SLoginPromptFailure);
      end;
    end;

    ShowSqlHourGlass;
    try
      FConnection := DriverManager.GetConnectionWithParams(
        ConstructURL(UserName, Password), FProperties);
      try
        with FConnection do
        begin
          SetAutoCommit(FAutoCommit);
          SetReadOnly(FReadOnly);
          SetCatalog(FCatalog);
          SetTransactionIsolation(FTransactIsolationLevel);
          Open;
        end;
      except
        FConnection := nil;
        raise;
      end;
    finally
      HideSqlHourGlass;
    end;

    if not FConnection.IsClosed then
      DoAfterConnect;
  end;
end;

In this way the username and password are set to the username and password which are typed into the login dialog.

seawolf
Zeos Dev Team *
Zeos Dev Team *
Posts: 385
Joined: 04.06.2008, 19:50
Contact:

Postby seawolf » 12.05.2009, 20:53

Looks like similar to Zeos bug 173, never checked

Problem is related to the TLoginEvent where user and password seems to be declared as readonly.

User avatar
mdaems
Zeos Project Manager
Zeos Project Manager
Posts: 2766
Joined: 20.09.2005, 15:28
Location: Brussels, Belgium
Contact:

Postby mdaems » 13.05.2009, 20:38

var
//Local variables declared in order to preserve the original property value
//and to avoid the storage of password
  Username, Password: string;


Doesn't this look like this is done on purpose? In my opinion (but who am I??) this shouldn't be changed. If one really wants to set the component properties to whatever input comes from the user, he can always call LoginDialogProc himself and set the property values.

So, to all others reading : let us know your opinion on this. I'm not applying this patch to the official sources for now.

Mark
Image

Ostfriese
Junior Boarder
Junior Boarder
Posts: 25
Joined: 12.05.2009, 10:48
Location: Coburg
Contact:

Postby Ostfriese » 14.05.2009, 09:59

From my point of view I expect that the properties user and password allways contain the current values which are either set by the program or modified by the login prompt. Otherwise they are useless because it's impossible to determine which username is currently used.
Thomas Jefferson wrote:Information ist die Währung der Demokratie

Ostfriese
Junior Boarder
Junior Boarder
Posts: 25
Joined: 12.05.2009, 10:48
Location: Coburg
Contact:

Postby Ostfriese » 14.05.2009, 14:25

Okay, I've reworked my patch in that way that I've additionaly removed the protected FUser and FPassword variables. The User and Password properties are now setting and getting the coresponding connection properties.

After login dialog is closed the private procedure which set the username and password properties are called to update the connection properties.

All changes are surrounded by

Code: Select all

//HA 090514

comments for easier finding
You do not have the required permissions to view the files attached to this post.
Thomas Jefferson wrote:Information ist die Währung der Demokratie

User avatar
mdaems
Zeos Project Manager
Zeos Project Manager
Posts: 2766
Joined: 20.09.2005, 15:28
Location: Brussels, Belgium
Contact:

Postby mdaems » 14.05.2009, 15:11

Ohhh... Stop... wait... my message must have been a little unclear to you. Sorry for that.

The problem is not that the Username and Pasword properties exist. They only provide an alternative way to provide the connection with a username and password to connect. This is good for those who want to avoid using a login prompt or OnLogin event handler. And it is way easier to understand how to set a user/password this way than to use those primitive string settings used by Delphi's standard components. But it's similar in that way that it's a plain text field defined at design time or in the user program. So I certainly did not want to change this current behaviour.

What worried me more was the fact that your initial patch did change the programmatically readable/modifiable component settings. This isn't similar to the way Delphi works AFAIK. Using the Login Dialog doesn't change eventually preset values and it is 'impossible' to get the user provided username and password easily.
In general I believe it's bad practice to keep a user provided username/password combination longer around than is strictly necessary. Apparently Delphi didn't like this either. So I don't like to promote this.

I hope some others want to comment on this as well. As your signature says : just to get the information necessary to guarantee a democratic decision.

Mark
Image

Ostfriese
Junior Boarder
Junior Boarder
Posts: 25
Joined: 12.05.2009, 10:48
Location: Coburg
Contact:

Postby Ostfriese » 14.05.2009, 15:34

mdaems: Just take a look at my modified unit. The properties are still there but they use the connection properties username and password instead of extra variables. :wink:

This is IMHO more consequent because there will be only one location where the username and password are stored or is there a reason I didn't see for this behaviour?
Thomas Jefferson wrote:Information ist die Währung der Demokratie

gto
Zeos Dev Team
Zeos Dev Team
Posts: 278
Joined: 11.11.2005, 18:35
Location: Porto Alegre / Brasil
Contact:

Postby gto » 14.05.2009, 19:13

Leaving my 0.05 cents:

The original behaviour was designed to keep the typed settings just into the login procedure and don't propagate them through ZConnection properties. The patch from Ostfriese make it save the typed user and password over the ZConnection values, right?

Well, if this is a security point, I really don't know. Indeed, seems more logic, as you're storing user/pass in one place only, and keeping them updated through running of application.

But, looking for the login dialog box, it need the user to input user/pass everytime. I don't know people which use the default dialog (seems better to build your own), but storing the values over the ZConnection values have no mean, the user will be prompted for them everytime it's used again.

I can suggest the follow options:
- Save user and pass typed in LoginDialog under ZConnection params. Something like params["logindialog_user" = "john"].
- In future, develop our own LoginDialog box, which save typed passwords between sessions of a running application.
Use the FU!!!!!IN Google !

gto's Zeos Quick Start Guide

Te Amo Taís!

User avatar
mdaems
Zeos Project Manager
Zeos Project Manager
Posts: 2766
Joined: 20.09.2005, 15:28
Location: Brussels, Belgium
Contact:

Postby mdaems » 18.05.2009, 23:36

Okay,

I had a look again at your patch and gto's comments.

Some new conclusions/questions about your (last) patch.
- What's the use of deprecate-ing FUser and FPassword when they are just ignored? In that case I propose to just drop them.
- Why parsing the Properties object in your own code? Isn't the standard TStrings interface sufficient for access by name? Please correct when you can't justify.
- Changes can not be done to 6.6 for this issue. Let's keep them in the 7.X versions becauseof backward compatibility. Iagine someonwe writing his ZConnection.properties to an ini file. At once the password would be visible to everybody who has access to this file. This is one of the reasons to stick with current implementation using private variables THis is my most mportant reason to keep these variables private/protected.
- Please work startting from testing branch when making patches, unless this branch is too different from 6.6 and the patch is for 6.6 only.

Mark
Image

Ostfriese
Junior Boarder
Junior Boarder
Posts: 25
Joined: 12.05.2009, 10:48
Location: Coburg
Contact:

Postby Ostfriese » 19.05.2009, 07:16

mdaems wrote:- What's the use of deprecate-ing FUser and FPassword when they are just ignored? In that case I propose to just drop them.

It's my case of removing used variables: Mark them as deprecate and remove them some versions later

mdaems wrote:- Why parsing the Properties object in your own code? Isn't the standard TStrings interface sufficient for access by name? Please correct when you can't justify.

IMHO a TStrings can't look for partial strings so I've to it on my own.
Thomas Jefferson wrote:Information ist die Währung der Demokratie

User avatar
mdaems
Zeos Project Manager
Zeos Project Manager
Posts: 2766
Joined: 20.09.2005, 15:28
Location: Brussels, Belgium
Contact:

Postby mdaems » 19.05.2009, 21:44

Ostfriese,

From your patched version:

Code: Select all

    //HA 090514 retrieve username and password from the connection properties ..
    //UserName := FUser;
    //Password := FPassword;
    UserName := GetUsername;
    Password := GetPassword;
    //..

    if FLoginPrompt then
    begin
      { Defines user name }
      if UserName = '' then
        UserName := FProperties.Values['UID'];
      if UserName = '' then
        UserName := FProperties.Values['username'];

      { Defines user password }
      if Password = '' then
        Password := FProperties.Values['PWD'];
      if Password = '' then
        Password := FProperties.Values['password'];



When you look a little below your change you'll notice GetPassword can be replaced by FProperties.Values['password']. Here it's used to prefill the login dialog fields.

Or am I overlooking something?

Mark
Image

seawolf
Zeos Dev Team *
Zeos Dev Team *
Posts: 385
Joined: 04.06.2008, 19:50
Contact:

Postby seawolf » 19.05.2009, 21:47

Hi, loginprompt property does not works (as described in zeos bug 173)
because of TLoginEvent

User avatar
mdaems
Zeos Project Manager
Zeos Project Manager
Posts: 2766
Joined: 20.09.2005, 15:28
Location: Brussels, Belgium
Contact:

Postby mdaems » 19.05.2009, 21:51

Yes, I know, but that wasn't really the subject of this topic ;) . Here we're talking about the goods and/or bads of the proposed patch. We welcome your opinion on the subect, however. Please give it a little study.

Mark
Image

Ostfriese
Junior Boarder
Junior Boarder
Posts: 25
Joined: 12.05.2009, 10:48
Location: Coburg
Contact:

Postby Ostfriese » 11.08.2009, 08:15

I've modified login event declaration in ZConnection.pas based on the description in Zeos Bug #173 to make the username and password persistent. I hope this will also help to close the corresponding bug.

My patched ZConnection.pas is based on ZeosDBO 6.6.5
You do not have the required permissions to view the files attached to this post.
Thomas Jefferson wrote:Information ist die Währung der Demokratie


Return to “User Patches”

Who is online

Users browsing this forum: No registered users and 1 guest