brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.91k stars 2.34k forks source link

Migrate uses of GURL to url::Origin when possible #19849

Open mariospr opened 2 years ago

mariospr commented 2 years ago

Description

As explained in the Chromium documentation [1], "URLs are often not sufficient for security decisions, since the origin may not be present in the URL (e.g., about:blank), may be tricky to parse (e.g., blob: or filesystem: URLs), or may be opaque despite a normal-looking URL (e.g., the security context may be sandboxed)".

Unfortunately, Brave uses at the moment GURL in several places where url::Origin should be used instead (e.g. in brave::BraveRequestInfo::tab_origin, and other places), which is becoming an increasingly bigger problem as upstream Chromium is progressively migrating away from GURL as well.

Also, note that this not only affects C++ files but also mojo definitions as well, see for instance:

module brave_wallet.mojom;

[...]

interface EthJsonRpcController {
  AddEthereumChain(EthereumChain chain, url.mojom.Url origin) => (string chain_id, bool accepted);
  [...]
}

Thus, filing this issue to track down the effort of converting as much as possible to url::Origin instead of using GURL (and maybe cases using strings as well, such as this one).

In summary, as mentioned in [1], we should use the following datatypes to represent origins:

[1] https://chromium.googlesource.com/chromium/src/+/main/docs/security/origin-vs-url.md

mariospr commented 2 years ago

cc @pes10k @diracdeltas @iefremov @goodov

pes10k commented 2 years ago

cc @pilgrim-brave