Repository: servo/rust-url (rust)

Issue:
Corrupted data URL output from <rect style='fill:%23000000;' width='100%' height='100%'/>
# Version: 
data-url 0.2.0 & master

# Description
The following data URL:
```
data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg'><rect style='fill:%23000000;' width='100%' height='100%'/></svg>
```
produces a corrupted result with duplicate fields and unexpected values with `DataUrl::process`:
```
<svg xmlns='http://www.w3.org/2000/svg'><rect style='fill:#000000;' width='100000000;' width='100%' height='100000000;' width='100%' height='100%'/></svg>
```

Note that the data URL produces a full window black rectangle on Firefox and keeps the initial SVG intact.

# SSCCE
```
use data_url::DataUrl;    
    
static SVG: &str = r#"data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg'><rect style='fill:%23000000;' width='100%' height='100%'/></svg>"#;    
    
fn main() {    
    println!("{}", SVG);    
    let url = DataUrl::process(SVG).unwrap();    
    let (body, _) = url.decode_to_vec().unwrap();    
    let utf8 = std::str::from_utf8(&body).unwrap();    
    println!("{}", utf8);                                                                                                                                                     
} 
```





Discussion:
I'm not sure if it is the only problem,  but you need to escape the spaces and % characters in your data url.
> I'm not sure if it is the only problem, but you need to escape the spaces and % characters in your data url.

It's a user submitted one based on browser support. It may not be correct, but this does not explain the result as far as I can tell reading the URL standard.
Spaces are not the issue here. The problem can be reproduced with the following even simpler example:

```
use data_url::DataUrl;    
    
static URL: &str = r#"data:text/plain;utf8,100%"#;    
    
fn main() {    
    println!("{}", URL);    
    let url = DataUrl::process(URL).unwrap();    
    let (body, _) = url.decode_to_vec().unwrap();    
    let utf8 = std::str::from_utf8(&body).unwrap();    
    println!("{}", utf8);    
}  
```
Which outputs:
```
data:text/plain;utf8,100%
100100%
```
So the `%` character is replaced by the whole input string (in the top example by a substring starting before the % character).

According to the URL standard:

https://url.spec.whatwg.org/#percent-encoded-bytes

> 
> 1. If byte is not 0x25 (%), then append byte to output.
> 
> 2. Otherwise, if byte is 0x25 (%) and the next two bytes after byte in input are not in the ranges 0x30 (0) to 0x39 (9), 0x41 (A) to 0x46 (F), and 0x61 (a) to 0x66 (f), all inclusive, append byte to output.

Here for `100%`, the 0x25 (%) character is not followed by 2 bytes in range `[0-9A-Fa-f]`, therefore if I understand correctly the spec it should be included in the output without more processing.
The problem comes from the `decode_without_base64` function.

When a `%` character is encountered, the function "Write everything (if anything) "non-special" we’ve accumulated before this special byte":
https://github.com/servo/rust-url/blob/359bc90a4f07224f79cc79c45dc873d44bcd6f14/data-url/src/lib.rs#L268-L272

But `slice_start` is not updated, and because the characters following `%` do not match hex digits, it is not updated either in the following `else` condition:
https://github.com/servo/rust-url/blob/359bc90a4f07224f79cc79c45dc873d44bcd6f14/data-url/src/lib.rs#L278-L286

Therefore when at the end, bytes from `slice_start` to the end are written:
https://github.com/servo/rust-url/blob/359bc90a4f07224f79cc79c45dc873d44bcd6f14/data-url/src/lib.rs#L300-L302

we write again all accumulated characters, which duplicates all non-special characters accumulated.

I believe that `slice_start` should be set to `i` after writing accumulated "non-special" characters to avoid issues in later conditionals not updating it like the empty `else` described above:
```
              if i > slice_start {    
                  write_bytes(&bytes[slice_start..i])?;    
+                 slice_start = i;    
              } 
```
An alternative would be to update it in the `else` condition when `i > slice_start`, but there is the risk of adding new conditionals not updating it later.

PS: Maybe some code could be common with `PercentDecode` in `percent_encoding` lib to avoid duplicate (and different) implementations.

Patch:
diff --git a/data-url/src/lib.rs b/data-url/src/lib.rs
--- a/data-url/src/lib.rs
+++ b/data-url/src/lib.rs
@@ -298,6 +298,7 @@ where
             // before this special byte
             if i > slice_start {
                 write_bytes(&bytes[slice_start..i])?;
+                slice_start = i;
             }
             // Then deal with the special byte.
             match byte {


Test patch:
diff --git a/data-url/tests/data-urls.json b/data-url/tests/data-urls.json
--- a/data-url/tests/data-urls.json
+++ b/data-url/tests/data-urls.json
@@ -52,6 +52,24 @@
   ["data:text/plain;Charset=UTF-8,%C2%B1",
    "text/plain;charset=UTF-8",
    [194, 177]],
+  ["data:text/plain,%",
+   "text/plain",
+   [37]],
+  ["data:text/plain,X%",
+   "text/plain",
+   [88, 37]],
+  ["data:text/plain,X%%",
+   "text/plain",
+   [88, 37, 37]],
+  ["data:text/plain;Charset=UTF-8,X%X",
+   "text/plain;charset=UTF-8",
+   [88, 37, 88]],
+  ["data:text/plain;Charset=UTF-8,X%0",
+   "text/plain;charset=UTF-8",
+   [88, 37, 48]],
+  ["data:text/plain;Charset=UTF-8,X%0X",
+   "text/plain;charset=UTF-8",
+   [88, 37, 48, 88]],
   ["data:text/plain;charset=windows-1252,áñçə💩",
    "text/plain;charset=windows-1252",
    [195, 161, 195, 177, 195, 167, 201, 153, 240, 159, 146, 169]],