Bug 95849 – [EFL] Sanitize URLs in EWebLauncher / Minibrowser

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Chris Dumez]

The URL argument to EFL's EWebLauncher / Minibrowser is not sanitized and therefore, passing an argument such as a local path will not work.

[Chris Dumez]

Created attachment 162237 [details]
Patch

The following now works:
WebKitBuild/Release/bin/MiniBrowser ../tts-webapi/webapi-w3c-css3-tests/WOFF/woff.html

Yay!

[Kenneth Rohde Christiansen]

Comment on attachment 162237 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162237&action=review

> Tools/EWebLauncher/url_utils.c:31
> +    return eina_str_has_prefix(url, "file://") || eina_str_has_prefix(url, "http://") || eina_str_has_prefix(url, "https://") || eina_str_has_prefix(url, "ftp://");

So why is data: out?

This doesn't seem very trustworthy... Doesnt WebKit have somethign like this?

> Tools/EWebLauncher/url_utils.c:44
> +    /* Convert local path to an URI if needed. */
> +    if (eina_strbuf_length_get(buf) && eina_strbuf_string_get(buf)[0] == '/')
> +        eina_strbuf_prepend(buf, "file://");
> +    char *url = eina_strbuf_string_steal(buf);

Ddi you look at Qt QUrl::fromUserInput ? You should :-)

[Chris Dumez]

Comment on attachment 162237 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162237&action=review

>> Tools/EWebLauncher/url_utils.c:31
>> +    return eina_str_has_prefix(url, "file://") || eina_str_has_prefix(url, "http://") || eina_str_has_prefix(url, "https://") || eina_str_has_prefix(url, "ftp://");
> 
> So why is data: out?
> 
> This doesn't seem very trustworthy... Doesnt WebKit have somethign like this?

I understand this isn't perfect but it should cover the cases needed in MiniBrowser / EWebLauncher I think. Otherwise, I would probably need to use regular expressions.
Regarding WebKit, well, there is related code in KURL but it is quite complex and I cannot reuse it directly here.

>> Tools/EWebLauncher/url_utils.c:44
>> +    char *url = eina_strbuf_string_steal(buf);
> 
> Ddi you look at Qt QUrl::fromUserInput ? You should :-)

No, I haven't. What cases are you worried about? eina_file_path_sanitize() already does a lot of things for us (e.g. make relative a path absolute).

[Chris Dumez]

Comment on attachment 162237 [details]
Patch

Clearing flags. Will make another iteration.

[Chris Dumez]

Created attachment 162426 [details]
Patch

Improved patch based on QUrl::fromUserInput() implementation and our existing implementation in url_bar.c.

The URLs are now sanitized not only for program argument but also in the URL bar.

[Chris Dumez]

Comment on attachment 162426 [details]
Patch

Need to fix a warning.

[Chris Dumez]

Created attachment 162427 [details]
Patch

Fix compiler warning.

[WebKit Review Bot]

Comment on attachment 162427 [details]
Patch

Clearing flags on attachment: 162427

Committed r127729: <http://trac.webkit.org/changeset/127729>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dominik Röttsches (drott)]

Nice, finally the pain is gone. Thanks.

[Chris Dumez]

(In reply to comment #10)
> Nice, finally the pain is gone. Thanks.

Glad I'm not the only one who was annoyed by this :)

[Thiago Marcos P. Santos]

(In reply to comment #11)
> (In reply to comment #10)
> > Nice, finally the pain is gone. Thanks.
> 
> Glad I'm not the only one who was annoyed by this :)

Hooray!

[Kenneth Rohde Christiansen]

Yeah ! finally! It has only annoyed me for one day or so, but that is enough!