Closes #7142: Sanitize url in HttpIconLoader

[Amejia481]

---

Pull Request checklist

• Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
• Tests: This PR includes thorough tests or an explanation of why it does not
• Changelog: This PR includes a changelog entry or does not need one
• Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

• Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
• Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

[codecov]

Codecov Report

Merging #7160 into master will decrease coverage by 0.45%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #7160      +/-   ##
============================================
- Coverage     77.23%   76.78%   -0.46%     
+ Complexity     5044     4783     -261     
============================================
  Files           674      652      -22     
  Lines         24692    23519    -1173     
  Branches       3643     3435     -208     
============================================
- Hits          19070    18058    -1012     
+ Misses         4113     4023      -90     
+ Partials       1509     1438      -71     

Impacted Files	Coverage Δ	Complexity Δ
.../components/browser/icons/extension/IconMessage.kt	75.00% <100.00%> (ø)	0.00 <0.00> (ø)
...onents/support/sync/telemetry/BaseGleanSyncPing.kt	100.00% <0.00%> (ø)	11.00% <0.00%> (ø%)
...a/components/browser/search/SearchEngineManager.kt
...owser/search/suggestions/SearchSuggestionClient.kt
...la/components/browser/search/suggestions/Parser.kt
.../mozilla/components/browser/search/SearchEngine.kt
...in/java/mozilla/components/lib/jexl/lexer/Token.kt
...zilla/components/lib/jexl/evaluator/JexlContext.kt
...mozilla/components/lib/jexl/parser/StateMachine.kt
...va/mozilla/components/lib/jexl/lexer/LexerInput.kt
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bec28a5...98158c8. Read the comment docs.

[Amejia481]

@pocmo I preferred to not add a IllegalArgumentException to the exception list on HttpIconLoader.load as it is a implementation detail of GeckoViewFetch/GeckoWebExecutor, that is not necessary true for all implementation of Client. What do you think?

[pocmo]

@pocmo I preferred to not add a IllegalArgumentException to the exception list on HttpIconLoader.load as it is a implementation detail of GeckoViewFetch/GeckoWebExecutor, that is not necessary true for all implementation of Client. What do you think?

Hm. I think I would prefer if we sanitize one level higher. This came from BrowserIcons and that code is reading values from the website. This code should take care of sanitizing and probably not blindly trust all values it gets. A Client bailing out if you feed it something that is not a valid URL sounds okay to me tbh. - I think it's good if it fails and the caller has to think about potentially broken/untrusted values being passed to the Client.

[Amejia481]

@pocmo 👍 I updated the pr. Now we are sanitizing in both levels HttpIconLoader and GeckoViewFetch

[Amejia481]

I updated the pr :)

[pocmo]

Great. 👍

bors r+

[bors]

Build failed:

• complete-push

[Amejia481]

bors retry

[bors]

Build succeeded:

• complete-push