Re: msi: Dont crash on installation of services without component KeyPath

[Hans Leidekker]

On Wed, 2014-02-19 at 14:49 +0900, Aric Stewart wrote:
> diff --git a/dlls/msi/action.c b/dlls/msi/action.c
> index 07322fe..a004225 100644
> --- a/dlls/msi/action.c
> +++ b/dlls/msi/action.c
> @@ -5794,27 +5794,30 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param)
>          goto done;
>      }
>      key = MSI_RecordGetString(row, 6);
> - file = msi_get_loaded_file(package, key);
> - msiobj_release(&row->hdr);
> - if (!file)
> - {
> -
>    ERR("Failed to load the service file\n");
> -
>    goto done;
> - }
> -
> - if (!args || !args[0]) image_path = file->TargetPath;
> - else
> - if (key)
> {
> -
>    int len = strlenW(file->TargetPath) + strlenW(args) + 2;
> -
>    if (!(image_path = msi_alloc(len * sizeof(WCHAR))))
> -
>    file = msi_get_loaded_file(package, key);
> -
>    msiobj_release(&row->hdr);
> -
>    if (!file)
>    {
> -
>        ret = ERROR_OUTOFMEMORY;
> -
>        ERR("Failed to load the service file\n");
>        goto done;
>    }
>
> -
>    strcpyW(image_path, file->TargetPath);
> -
>    strcatW(image_path, szSpace);
> -
>    strcatW(image_path, args);
> -
>    if (!args || !args[0]) image_path = file->TargetPath;
> -
>    else
> -
>    {
> -
>        int len = strlenW(file->TargetPath) + strlenW(args) + 2;
> -
>        if (!(image_path = msi_alloc(len * sizeof(WCHAR))))
> -
>        {
> -
>            ret = ERROR_OUTOFMEMORY;
> -
>            goto done;
> -
>        }
> -
> -
>        strcpyW(image_path, file->TargetPath);
> -
>        strcatW(image_path, szSpace);
> -
>        strcatW(image_path, args);
> -
>    }
> }
>    service = CreateServiceW(hscm, name, disp, GENERIC_ALL, serv_type,
>                             start_type, err_control, image_path, load_order,
>
> @@ -5831,7 +5834,7 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param)
>              WARN("failed to set service description %u\n", GetLastError());
>      }
> - if (image_path != file->TargetPath) msi_free(image_path);
> - if (image_path && image_path != file->TargetPath) msi_free(image_path);
>
> done:
>      CloseServiceHandle(service);
>      CloseServiceHandle(hscm);

Should we still create the service if the installer doesn't specify the image path?

msi: Dont crash on installation of services without component KeyPath

[Aric Stewart]

Well the image path in the component is optional and looking at the documentation for CreateServiceW lpBinaryPathName is also optional.  So it seemed correct.

If you feel like we should just exit then that is acceptable as well.

-aric

On 2014/02/19 17:51, Hans Leidekker wrote:
> On Wed, 2014-02-19 at 14:49 +0900, Aric Stewart wrote:
> > diff --git a/dlls/msi/action.c b/dlls/msi/action.c
> > index 07322fe..a004225 100644
> > --- a/dlls/msi/action.c
> > +++ b/dlls/msi/action.c
> > @@ -5794,27 +5794,30 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param)
> >           goto done;
> >       }
> >       key = MSI_RecordGetString(row, 6);
> > - file = msi_get_loaded_file(package, key);
> > - msiobj_release(&row->hdr);
> > - if (!file)
> > - {
> > -
> >    ERR("Failed to load the service file\n");
> > -
> >    goto done;
> > - }
> > -
> > - if (!args || !args[0]) image_path = file->TargetPath;
> > - else
> > - if (key)
> >  {
> > -
> >    int len = strlenW(file->TargetPath) + strlenW(args) + 2;
> > -
> >    if (!(image_path = msi_alloc(len * sizeof(WCHAR))))
> > -
> >    file = msi_get_loaded_file(package, key);
> > -
> >    msiobj_release(&row->hdr);
> > -
> >    if (!file)
> >     {
> > -
> >        ret = ERROR_OUTOFMEMORY;
> > -
> >        ERR("Failed to load the service file\n");
> >         goto done;
> >     }
> >
> > -
> >    strcpyW(image_path, file->TargetPath);
> > -
> >    strcatW(image_path, szSpace);
> > -
> >    strcatW(image_path, args);
> > -
> >    if (!args || !args[0]) image_path = file->TargetPath;
> > -
> >    else
> > -
> >    {
> > -
> >        int len = strlenW(file->TargetPath) + strlenW(args) + 2;
> > -
> >        if (!(image_path = msi_alloc(len * sizeof(WCHAR))))
> > -
> >        {
> > -
> >            ret = ERROR_OUTOFMEMORY;
> > -
> >            goto done;
> > -
> >        }
> > -
> > -
> >        strcpyW(image_path, file->TargetPath);
> > -
> >        strcatW(image_path, szSpace);
> > -
> >        strcatW(image_path, args);
> > -
> >    }
> > }
> > service = CreateServiceW(hscm, name, disp, GENERIC_ALL, serv_type,
> >                          start_type, err_control, image_path, load_order,
> >
> > @@ -5831,7 +5834,7 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param)
> >               WARN("failed to set service description %u\n", GetLastError());
> >       }
> > - if (image_path != file->TargetPath) msi_free(image_path);
> > - if (image_path && image_path != file->TargetPath) msi_free(image_path);
> > done:
> >  CloseServiceHandle(service);
> >  CloseServiceHandle(hscm);
>
> Should we still create the service if the installer doesn't specify the image path?

msi: Dont crash on installation of services without component KeyPath

[Hans Leidekker]

On Wed, 2014-02-19 at 17:57 +0900, Aric Stewart wrote:
> Well the image path in the component is optional and looking at the documentation
> for CreateServiceW lpBinaryPathName is also optional.  So it seemed correct.
>
> If you feel like we should just exit then that is acceptable as well.

What happens on Windows?

msi: Dont crash on installation of services without component KeyPath

[Aric Stewart]

I have not dug into the details, but I know it does not crash and the overall install does succeed.

I can try investigating further on windows, Is there an easy way to determine what service this is and if it gets successfully installed on windows?

-aric

On 2014/02/19 18:11, Hans Leidekker wrote:
> On Wed, 2014-02-19 at 17:57 +0900, Aric Stewart wrote:
> > Well the image path in the component is optional and looking at the documentation
> > for CreateServiceW lpBinaryPathName is also optional.  So it seemed correct.
> >
> > If you feel like we should just exit then that is acceptable as well.
>
> What happens on Windows?

msi: Dont crash on installation of services without component KeyPath

[Hans Leidekker]

On Wed, 2014-02-19 at 20:03 +0900, Aric Stewart wrote:
> I have not dug into the details, but I know it does not crash and the
> overall install does succeed.
>
> I can try investigating further on windows, Is there an easy way to
> determine what service this is and if it gets successfully installed on windows?

A +msi,+service trace should reveal the service name. "sc query" will list
installed services, for example.

msi: Dont crash on installation of services without component KeyPath

[Aric Stewart]

Ok, I will try that tomorrow and report if windows successfully installs the service or not.

-aric

On 2014/02/19 20:28, Hans Leidekker wrote:
> On Wed, 2014-02-19 at 20:03 +0900, Aric Stewart wrote:
> > I have not dug into the details, but I know it does not crash and the
> > overall install does succeed.
> >
> > I can try investigating further on windows, Is there an easy way to
> > determine what service this is and if it gets successfully installed on windows?
>
> A +msi,+service trace should reveal the service name. "sc query" will list
> installed services, for example.

msi: Dont crash on installation of services without component KeyPath

[Aric Stewart]

On 2014/02/19 20:28, Hans Leidekker wrote:
> On Wed, 2014-02-19 at 20:03 +0900, Aric Stewart wrote:
> > I have not dug into the details, but I know it does not crash and the
> > overall install does succeed.
> >
> > I can try investigating further on windows, Is there an easy way to
> > determine what service this is and if it gets successfully installed on windows?
>
> A +msi,+service trace should reveal the service name. "sc query" will list
> installed services, for example.

Unfortunately it only tries to install the service on windows 2000 or older. (this is an old applications that I was installing in a windows 2000 prefix)  I do not have a windows 2000 machine to test the install on.

Using an XP prefix gets around the problem by not trying to install the service.  But I am unable to determine what is the correct method to fix this issue. You are more of the expert Hans, so I will leave it up to you if we should fail to install the service or install the service without an image path.

-aric

msi: Dont crash on installation of services without component KeyPath

[Francois Gouget]

On Thu, 20 Feb 2014, Aric Stewart wrote:
[...]
> Unfortunately it only tries to install the service on windows 2000 or 
> older. (this is an old applications that I was installing in a windows 
> 2000 prefix)  I do not have a windows 2000 machine to test the install 
> on.

You're in luck then. The new testbot now has a Windows 2000 VM...

msi: Dont crash on installation of services without component KeyPath

[Dmitry Timoshkov]

Francois Gouget  wrote:
> You're in luck then. The new testbot now has a Windows 2000 VM...

Probably it's time then to make testbot process the tasks slightly
faster, currently it's speed is completely unacceptable. For that
reason I was using old testbot instead, but since it's no longer
available this creates additional problems and prevents productive
work.