Closed Bug 453623 Opened 16 years ago Closed 16 years ago

Spaces stripped out of data uri when pasting in location bar

Categories

(Camino Graveyard :: Location Bar & Autocomplete, defect)

Product:
Camino Graveyard
Component:
Location Bar & Autocomplete
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: phiw2, Assigned: bugzilla-graveyard)

References

Details

(Keywords: regression)

Attachments

(1 file, 3 obsolete files)

Really ready now
7.51 KB, patch
Details | Diff | Splinter Review 
Copy paste in the location bar the following:

data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" id="canvas"> <rect id="background-rect" x="25%" y="25%" width="50%" height="50%" fill="#aaaaaa"/></svg>

(as seen in the url field of bug 407495)

AR: a yellow page :-(
ER: a white rectangle with a grey rectangular hole in the middle

or this:
data:application/xhtml+xml,<html xmlns="http://www.w3.org/1999/xhtml"><body id="x"><ol><li>one and really one</li><li>two</li><li>three</li></ol></body></html>

all spaces are stripped out of the data uri, resulting in <svgxmlns="h.....

Version 2.0a1pre (1.9.0.3pre 2008090100) works fine
Version 2.0a1pre (1.9.0.3pre 2008090200) fails

--> bug 411165

note: (curiously to me) this one works fine (note mime-type):
data:text/html,<html><body id="x"><ol><li>one and really one</li><li>two</li><li>three</li></ol></body></html>
Not only xml, this fails as well:

data:text/plain,<html xmlns="http://www.w3.org/1999/xhtml"><body id="x"><ol><li>one and really one</li><li>two</li><li>three</li></ol></body></html>
Summary: Spaces stripped out of data uri with xml mime type when pasting in location bar → Spaces stripped out of data uri when pasting in location bar
We need to add 'data:' to the whitelist, like 'javascript:'
Yeah, oops, I forgot about data: URIs when I wrote that patch. Sorry 'bout that. I'll fix it when I get home this weekend.

cl
Hardware: PC → All
Assignee: nobody → cl-bugs-new
Status: NEW → ASSIGNED
Fixing dependencies; 411165 was not what broke this (which caused me some serious confusion when I went looking for the problem code).
Blocks: 452697
No longer blocks: 411165
Attached patch fix v1 (obsolete) — — Splinter Review 
That oughta do it.
Attachment #337171 - Flags: superreview?(stuart.morgan+bugzilla)
Attachment #337171 - Flags: review?(trendyhendy2000)
Comment on attachment 337171 [details] [diff] [review]
fix v1

Lets get a private isValidURLString: method in here, as this test gets more complex.
Attachment #337171 - Flags: superreview?(stuart.morgan+bugzilla)
Attachment #337171 - Flags: superreview-
Attachment #337171 - Flags: review?(trendyhendy2000)
The |cleanPastedText| method of BrowserWindowController will strip all newlines in any string with "://" in it. This means the first example, with the namespace url in it, will end up being not well-formed. The method needs to be modified to prevent this.
Blocks: 454087
Attached patch fix v1.1 (obsolete) — — Splinter Review 
Addresses comment 6 and comment 7.
Attachment #337171 - Attachment is obsolete: true
Attachment #337336 - Flags: superreview?(stuart.morgan+bugzilla)
Attachment #337336 - Flags: review?(trendyhendy2000)
Maybe |isValidURIString| could be moved to NSString+Utils, as it may be useful to other methods. At the moment it can only be used by NSPasteboard.
Comment on attachment 337336 [details] [diff] [review]
fix v1.1

>+// Utility method to ensure validity of URI strings
>+// NSURL is used to validate most of them,
>+// but the NSURL test fails for |javascript:| and |data:| URIs

s/fails [...] URIs/may fail [...] URIs because they often contain invalid characters (such as " ")/.

>+  // Bookmarklets, whose URLs start with "javascript:", won't pass the NSURL test.
>+  // Neither will data: URIs.

Similar change needed here.

>+- (BOOL)isJavascriptOrDataURIString;

Drop the String part, since this is an NSString method (all NSStings are strings).

The name makes me sad in general; I'd rather we name a concept than a list of checks. But since I don't have any suggestions (isTechnicallyInvalidURIThatWeWillPretendIsOkay is a bit much), I guess we can stick with this.

sr=smorgan with the comment fixes and s/String//. I'm fine with moving the the validity method no NSString+Utils if it's needed elsewhere (just drop the String if you do it that way)
Attachment #337336 - Flags: superreview?(stuart.morgan+bugzilla) → superreview+
How about isLooselyValidatedURI, and give it a header comment explaining that it checks to see if it's a string that we treat as a valid URI even though it isn't technically valid.
Attached patch fix v1.11 (obsolete) — — Splinter Review 
sr comments addressed. This needs to land ASAP so we can get the security bug it's blocking landed as well.
Attachment #337336 - Attachment is obsolete: true
Attachment #337336 - Flags: review?(trendyhendy2000)
Attached patch Really ready now — — Splinter Review 

        Attachment #337475 -
        Attachment is obsolete: true

    
    

    
  
"Really ready now" landed on cvs trunk.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED

    
    

    
  
"Really ready now" verified as working with
2.0a1pre (1.9.0.3pre 2008090900).
Thank you.
Status: RESOLVED → VERIFIED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: