Skip to content
This repository has been archived by the owner on Jan 4, 2019. It is now read-only.

Check args before using it #338

Merged
merged 1 commit into from
Oct 10, 2017
Merged

Check args before using it #338

merged 1 commit into from
Oct 10, 2017

Conversation

darkdh
Copy link
Member

@darkdh darkdh commented Oct 10, 2017

fix brave/browser-laptop#11330

Auditors: @brividr, @bsclifton, @bbondy

@@ -89,7 +89,7 @@ bool Browser::RemoveAsDefaultProtocolClient(const std::string& protocol,
bool Browser::SetAsDefaultProtocolClient(const std::string& protocol,
mate::Arguments* args) {
std::string desktop_name;
if (!args->GetNext(&desktop_name)) {
if (args && !args->GetNext(&desktop_name)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

args should always exist, where are you seeing it as null?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebContents::OnRegisterProtocol in atom/browser/api/atom_api_web_contents.cc

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#309 was merged into 4.4 so this isn't a cr62 issue

@darkdh darkdh modified the milestones: 4.5.4, 4.4.27 Oct 10, 2017
@bridiver bridiver merged commit 9d81a62 into master Oct 10, 2017
bridiver added a commit that referenced this pull request Oct 10, 2017
bridiver added a commit that referenced this pull request Oct 12, 2017
bridiver added a commit that referenced this pull request Oct 12, 2017
@darkdh darkdh deleted the bl-issue-11330 branch October 18, 2017 03:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Browser crash when allowing protocol handler
2 participants