jraebrown / growl

Automatically exported from code.google.com/p/growl
0 stars 0 forks source link

GNTP URL Callback handling of % escapes #555

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Send a URL without % escapes in it via GNTP's Notification-Callback-Target 
header
2. We fail to create the URL
3. We fail to open the URL

What is the expected output?
That we should try to get the a URL regardless of % escape state of the string

What do you see instead?
We silently fail

Putting this on 2.1, 2.0.1 is about to ship, and this will need testing.  Can 
easily be replicated using the --url argument of GrowlNotify trying to make it 
open any file:// url with a space in it (GrowlNotify should try to cover this 
as well).
As a workaround, users can put their own % escapes in, but we need to be aware 
when fixing it that users might send % escaped or not % escaped strings and 
figure out how to work accordingly.  

Original issue reported on code.google.com by dan...@growl.info on 17 Nov 2012 at 8:27

GoogleCodeExporter commented 8 years ago
Alternatively, we could simple make sure we document this all better, both in 
GrowlNotify and the GNTP spec (URL's must conform to whichever RFC defines 
URL's for instance), and put in some error logging when we try to use a string 
that won't turn into an NSURL.  

Original comment by dan...@growl.info on 19 Nov 2012 at 12:41

GoogleCodeExporter commented 8 years ago
I've gone ahead and added an error message to GrowlNotify when a URL is 
invalid, along with an explanation of what we mean by valid.  Im going to flag 
as fixed in source, we don't crash or anything bad in Growl from a badly 
formatted URL string, it just doesn't work.  If someone at some point wants to 
improve our handling of this somehow, that can be discussed later

Original comment by dan...@growl.info on 28 Nov 2012 at 8:38