Signed-off-by: Joshua Ashton joshua@froggi.es --- programs/winebrowser/main.c | 40 +++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 11 deletions(-)
On 2/7/20 11:54 AM, Joshua Ashton wrote:
If the user's default browser application is winebrowser then we go into a a state where winebrowser calls winebrowser which calls winebrowser... forever.
Sometimes winebrowser is registered as a potential browser and even as the default when Wine is installed which makes this a genuine issue.
Where are you getting this? I don't see any code in Wine to set winebrowser as the default browser for winebrowser itself, or indeed to set any browser.
I wouldn't expect setting winebrowser as winebrowser's default browser to work, and I'd be personally inclined to say that whoever does that gets to keep the pieces. If distributions are doing this for some reason, well, they shouldn't do that.
Signed-off-by: Joshua Ashton joshua@froggi.es
programs/winebrowser/main.c | 40 +++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/programs/winebrowser/main.c b/programs/winebrowser/main.c index 9cd6812d03..1294ebf54f 100644 --- a/programs/winebrowser/main.c +++ b/programs/winebrowser/main.c @@ -69,31 +69,49 @@ static char *strdup_unixcp( const WCHAR *str ) return ret; }
+/* forbidden apps that we will test for and never launch
- because they could cause us to create new processes indefinitely */
+static const WCHAR forbidden_apps[] =
- {'w','i','n','e','b','r','o','w','s','e','r',0,0};
/* try to launch a unix app from a comma separated string of app names */ static int launch_app( const WCHAR *candidates, const WCHAR *argv1 ) { char *cmdline; int i, count; char **argv_new;
const WCHAR* forbidden_app = forbidden_apps;
if (!(cmdline = strdup_unixcp( argv1 ))) return 1;
while (*candidates) {
WCHAR **args = CommandLineToArgvW( candidates, &count );
/* test for forbidden apps */BOOL valid_candidate = TRUE;while (*forbidden_app) {if (strstrW(candidates, forbidden_app))valid_candidate = FALSE;forbidden_app += strlenW( forbidden_app ) + 1; /* grab the next app */}
if (!(argv_new = HeapAlloc( GetProcessHeap(), 0, (count + 2) * sizeof(*argv_new) ))) break;for (i = 0; i < count; i++) argv_new[i] = strdup_unixcp( args[i] );argv_new[count] = cmdline;argv_new[count + 1] = NULL;
if (valid_candidate){WCHAR **args = CommandLineToArgvW( candidates, &count );if (!(argv_new = HeapAlloc( GetProcessHeap(), 0, (count + 2) * sizeof(*argv_new) ))) break;for (i = 0; i < count; i++) argv_new[i] = strdup_unixcp( args[i] );argv_new[count] = cmdline;argv_new[count + 1] = NULL;
TRACE( "Trying" );for (i = 0; i <= count; i++) TRACE( " %s", wine_dbgstr_a( argv_new[i] ));TRACE( "\n" );
TRACE( "Trying" );for (i = 0; i <= count; i++) TRACE( " %s", wine_dbgstr_a( argv_new[i] ));TRACE( "\n" );
_spawnvp( _P_OVERLAY, argv_new[0], (const char **)argv_new ); /* only returns on error */for (i = 0; i < count; i++) HeapFree( GetProcessHeap(), 0, argv_new[i] );HeapFree( GetProcessHeap(), 0, argv_new );
_spawnvp( _P_OVERLAY, argv_new[0], (const char **)argv_new ); /* only returns on error */for (i = 0; i < count; i++) HeapFree( GetProcessHeap(), 0, argv_new[i] );HeapFree( GetProcessHeap(), 0, argv_new );} candidates += strlenW( candidates ) + 1; /* grab the next app */-- 2.25.0
I agree with that, but it's been an issue for well over 3 years at this point and I've experienced it on multiple distros.
The worst thing we can do for the typical end user is to infinitely spawn processes endlessly... It's painful -- especially when winebrowser keeps becoming the default (not just links, for PDFs, etc) for whatever reason.
- Josh 🐸
---- On Thu, 20 Feb 2020 05:03:35 +0000 Zebediah Figura mailto:z.figura12@gmail.com wrote ----
On 2/7/20 11:54 AM, Joshua Ashton wrote:
If the user's default browser application is winebrowser then we go into a a state where winebrowser calls winebrowser which calls winebrowser... forever.
Sometimes winebrowser is registered as a potential browser and even as the default when Wine is installed which makes this a genuine issue.
Where are you getting this? I don't see any code in Wine to set winebrowser as the default browser for winebrowser itself, or indeed to set any browser.
I wouldn't expect setting winebrowser as winebrowser's default browser to work, and I'd be personally inclined to say that whoever does that gets to keep the pieces. If distributions are doing this for some reason, well, they shouldn't do that.
Signed-off-by: Joshua Ashton mailto:joshua@froggi.es
programs/winebrowser/main.c | 40 +++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/programs/winebrowser/main.c b/programs/winebrowser/main.c index 9cd6812d03..1294ebf54f 100644 --- a/programs/winebrowser/main.c +++ b/programs/winebrowser/main.c @@ -69,31 +69,49 @@ static char *strdup_unixcp( const WCHAR *str ) return ret; }
+/* forbidden apps that we will test for and never launch
- because they could cause us to create new processes indefinitely */
+static const WCHAR forbidden_apps[] =
- {'w','i','n','e','b','r','o','w','s','e','r',0,0};
/* try to launch a unix app from a comma separated string of app names */ static int launch_app( const WCHAR *candidates, const WCHAR *argv1 ) { char *cmdline; int i, count; char **argv_new;
const WCHAR* forbidden_app = forbidden_apps;
if (!(cmdline = strdup_unixcp( argv1 ))) return 1;
while (*candidates) {
WCHAR **args = CommandLineToArgvW( candidates, &count );
/* test for forbidden apps */BOOL valid_candidate = TRUE;while (*forbidden_app) {if (strstrW(candidates, forbidden_app))valid_candidate = FALSE;forbidden_app += strlenW( forbidden_app ) + 1; /* grab the next app */}
if (!(argv_new = HeapAlloc( GetProcessHeap(), 0, (count + 2) * sizeof(*argv_new) ))) break;for (i = 0; i < count; i++) argv_new[i] = strdup_unixcp( args[i] );argv_new[count] = cmdline;argv_new[count + 1] = NULL;
if (valid_candidate){WCHAR **args = CommandLineToArgvW( candidates, &count );if (!(argv_new = HeapAlloc( GetProcessHeap(), 0, (count + 2) * sizeof(*argv_new) ))) break;for (i = 0; i < count; i++) argv_new[i] = strdup_unixcp( args[i] );argv_new[count] = cmdline;argv_new[count + 1] = NULL;
TRACE( "Trying" );for (i = 0; i <= count; i++) TRACE( " %s", wine_dbgstr_a( argv_new[i] ));TRACE( "\n" );
TRACE( "Trying" );for (i = 0; i <= count; i++) TRACE( " %s", wine_dbgstr_a( argv_new[i] ));TRACE( "\n" );
_spawnvp( _P_OVERLAY, argv_new[0], (const char **)argv_new ); /* only returns on error */for (i = 0; i < count; i++) HeapFree( GetProcessHeap(), 0, argv_new[i] );HeapFree( GetProcessHeap(), 0, argv_new );
_spawnvp( _P_OVERLAY, argv_new[0], (const char **)argv_new ); /* only returns on error */for (i = 0; i < count; i++) HeapFree( GetProcessHeap(), 0, argv_new[i] );HeapFree( GetProcessHeap(), 0, argv_new );} candidates += strlenW( candidates ) + 1; /* grab the next app */-- 2.25.0
On 2/20/20 10:38 AM, Joshua Ashton wrote:
I agree with that, but it's been an issue for well over 3 years at this point and I've experienced it on multiple distros.
The worst thing we can do for the typical end user is to infinitely spawn processes endlessly... It's painful -- especially when winebrowser keeps becoming the default (not just links, for PDFs, etc) for whatever reason.
Again, why is this happening in the first place? Wine doesn't set the HKCU\Software\Wine\WineBrowser key to anything by default. If someone is setting it to winebrowser, I think that's on them.
- Josh
---- On Thu, 20 Feb 2020 05:03:35 +0000 *Zebediah Figura <z.figura12@gmail.com mailto:z.figura12@gmail.com>* wrote ----
On 2/7/20 11:54 AM, Joshua Ashton wrote: > If the user's default browser application is winebrowser then we go > into a a state where winebrowser calls winebrowser which calls > winebrowser... forever. > > Sometimes winebrowser is registered as a potential browser and even > as the default when Wine is installed which makes this a genuine > issue. Where are you getting this? I don't see any code in Wine to set winebrowser as the default browser for winebrowser itself, or indeed to set any browser. I wouldn't expect setting winebrowser as winebrowser's default browser to work, and I'd be personally inclined to say that whoever does that gets to keep the pieces. If distributions are doing this for some reason, well, they shouldn't do that. > > Signed-off-by: Joshua Ashton <joshua@froggi.es <mailto:joshua@froggi.es>> > --- > programs/winebrowser/main.c | 40 +++++++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 11 deletions(-) > > diff --git a/programs/winebrowser/main.c b/programs/winebrowser/main.c > index 9cd6812d03..1294ebf54f 100644 > --- a/programs/winebrowser/main.c > +++ b/programs/winebrowser/main.c > @@ -69,31 +69,49 @@ static char *strdup_unixcp( const WCHAR *str ) > return ret; > } > > +/* forbidden apps that we will test for and never launch > + * because they could cause us to create new processes indefinitely */ > +static const WCHAR forbidden_apps[] = > + {'w','i','n','e','b','r','o','w','s','e','r',0,0}; > + > /* try to launch a unix app from a comma separated string of app names */ > static int launch_app( const WCHAR *candidates, const WCHAR *argv1 ) > { > char *cmdline; > int i, count; > char **argv_new; > + const WCHAR* forbidden_app = forbidden_apps; > > if (!(cmdline = strdup_unixcp( argv1 ))) return 1; > > while (*candidates) > { > - WCHAR **args = CommandLineToArgvW( candidates, &count ); > + /* test for forbidden apps */ > + BOOL valid_candidate = TRUE; > + while (*forbidden_app) { > + if (strstrW(candidates, forbidden_app)) > + valid_candidate = FALSE; > + > + forbidden_app += strlenW( forbidden_app ) + 1; /* grab the next app */ > + } > > - if (!(argv_new = HeapAlloc( GetProcessHeap(), 0, (count + 2) * sizeof(*argv_new) ))) break; > - for (i = 0; i < count; i++) argv_new[i] = strdup_unixcp( args[i] ); > - argv_new[count] = cmdline; > - argv_new[count + 1] = NULL; > + if (valid_candidate) > + { > + WCHAR **args = CommandLineToArgvW( candidates, &count ); > + > + if (!(argv_new = HeapAlloc( GetProcessHeap(), 0, (count + 2) * sizeof(*argv_new) ))) break; > + for (i = 0; i < count; i++) argv_new[i] = strdup_unixcp( args[i] ); > + argv_new[count] = cmdline; > + argv_new[count + 1] = NULL; > > - TRACE( "Trying" ); > - for (i = 0; i <= count; i++) TRACE( " %s", wine_dbgstr_a( argv_new[i] )); > - TRACE( "\n" ); > + TRACE( "Trying" ); > + for (i = 0; i <= count; i++) TRACE( " %s", wine_dbgstr_a( argv_new[i] )); > + TRACE( "\n" ); > > - _spawnvp( _P_OVERLAY, argv_new[0], (const char **)argv_new ); /* only returns on error */ > - for (i = 0; i < count; i++) HeapFree( GetProcessHeap(), 0, argv_new[i] ); > - HeapFree( GetProcessHeap(), 0, argv_new ); > + _spawnvp( _P_OVERLAY, argv_new[0], (const char **)argv_new ); /* only returns on error */ > + for (i = 0; i < count; i++) HeapFree( GetProcessHeap(), 0, argv_new[i] ); > + HeapFree( GetProcessHeap(), 0, argv_new ); > + } > candidates += strlenW( candidates ) + 1; /* grab the next app */ > } > WINE_ERR( "could not find a suitable app to open %s\n", debugstr_w( argv1 )); > > -- > 2.25.0 >
-
Joshua Ashton -
Zebediah Figura