Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2020393002:
WebUI: only crash Chrome for invalid chrome:// URLs in debug (Closed)
DescriptionWebUI: only crash Chrome for invalid chrome:// URLs in debug
R=groby@chromium.org
BUG=615847
Committed: https://crrev.com/e3081ca02d004b57750869a2425fc8bf6c19e1e4
Cr-Commit-Position: refs/heads/master@{#397027}
Patch Set 1 #Patch Set 2 : debug-only #Messages
Total messages: 16 (6 generated)
Description was changed from ========== WebUI: crash earlier for invalid chrome:// URLs missing file extensions R=groby@chromium.org BUG=615847 ========== to ========== WebUI: crash earlier for invalid chrome:// URLs missing file extensions R=groby@chromium.org BUG=615847 ==========
/cc creis@
creis@chromium.org changed reviewers: + creis@chromium.org
The bug seems to show that it's possible to get here in practice by interacting with DevTools, which means that we'd crash the browser process after the user enters some input. That's very disruptive, and we should avoid it. If we want to treat this as illegal input, can we have DevTools catch it earlier so that we don't get as far as the CHECK crash?
On 2016/05/31 19:37:36, Charlie Reis wrote: > The bug seems to show that it's possible to get here in practice by interacting > with DevTools, which means that we'd crash the browser process after the user > enters some input. That's very disruptive, and we should avoid it. > > If we want to treat this as illegal input, can we have DevTools catch it earlier > so that we don't get as far as the CHECK crash? any funky URL can hit this, i.e. typing chrome://resources/blah.map in the URL bar
On 2016/05/31 21:19:49, Dan Beam wrote: > On 2016/05/31 19:37:36, Charlie Reis wrote: > > The bug seems to show that it's possible to get here in practice by > interacting > > with DevTools, which means that we'd crash the browser process after the user > > enters some input. That's very disruptive, and we should avoid it. > > > > If we want to treat this as illegal input, can we have DevTools catch it > earlier > > so that we don't get as far as the CHECK crash? > > any funky URL can hit this, i.e. typing chrome://resources/blah.map in the URL > bar Wow, it's definitely not OK to crash the browser process on that. We should be showing an error URL.
Description was changed from ========== WebUI: crash earlier for invalid chrome:// URLs missing file extensions R=groby@chromium.org BUG=615847 ========== to ========== WebUI: only crash Chrome for invalid chrome:// URLs in debug R=groby@chromium.org BUG=615847 ==========
On 2016/05/31 21:24:41, Charlie Reis wrote: > On 2016/05/31 21:19:49, Dan Beam wrote: > > On 2016/05/31 19:37:36, Charlie Reis wrote: > > > The bug seems to show that it's possible to get here in practice by > > interacting > > > with DevTools, which means that we'd crash the browser process after the > user > > > enters some input. That's very disruptive, and we should avoid it. > > > > > > If we want to treat this as illegal input, can we have DevTools catch it > > earlier > > > so that we don't get as far as the CHECK crash? > > > > any funky URL can hit this, i.e. typing chrome://resources/blah.map in the URL > > bar > > Wow, it's definitely not OK to crash the browser process on that. We should be > showing an error URL. Done. (though I wrote this intentional /to/ crash "on that", I'm also happy to get less bugs assigned to me ;))
Thanks, LGTM.
The CQ bit was checked by dbeam@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2020393002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2020393002/20001
Message was sent while issue was closed.
Description was changed from ========== WebUI: only crash Chrome for invalid chrome:// URLs in debug R=groby@chromium.org BUG=615847 ========== to ========== WebUI: only crash Chrome for invalid chrome:// URLs in debug R=groby@chromium.org BUG=615847 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== WebUI: only crash Chrome for invalid chrome:// URLs in debug R=groby@chromium.org BUG=615847 ========== to ========== WebUI: only crash Chrome for invalid chrome:// URLs in debug R=groby@chromium.org BUG=615847 Committed: https://crrev.com/e3081ca02d004b57750869a2425fc8bf6c19e1e4 Cr-Commit-Position: refs/heads/master@{#397027} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/e3081ca02d004b57750869a2425fc8bf6c19e1e4 Cr-Commit-Position: refs/heads/master@{#397027} |
