Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: command-line scheme switch values' spillover #19912

Merged
merged 2 commits into from Aug 26, 2019

Conversation

ckerr
Copy link
Member

@ckerr ckerr commented Aug 23, 2019

Description of Change

Fixes #19911: don't spill command-line switch scheme values into other switches.

In addition, this PR uses move semantics to move the tokens from a temporary container into their destination container.

Verified by adding this temporary code to the end of AtomContentClient::AddAdditionalSchemes():

#define DUMP_SCHEMES(name) \
  std::cout << #name << std::endl; \
  for (const auto& scheme : schemes->name) std::cout << scheme << ", "; \
  std::cout <<std::endl;

  DUMP_SCHEMES(service_worker_schemes)
  DUMP_SCHEMES(standard_schemes)
  DUMP_SCHEMES(secure_schemes)
  DUMP_SCHEMES(csp_bypassing_schemes)
  DUMP_SCHEMES(cors_enabled_schemes)

and invoking Electron with --standard-schemes=foo,bar,mum.

Without PR:

service_worker_schemes
file, 
standard_schemes
foo, bar, mum, chrome-extension, 
secure_schemes
foo, bar, mum, 
csp_bypassing_schemes
foo, bar, mum, 
cors_enabled_schemes
foo, bar, mum, 

With PR:

service_worker_schemes
file, 
standard_schemes
foo, bar, mum, chrome-extension, 
secure_schemes

csp_bypassing_schemes

cors_enabled_schemes

(file and chrome-extension are added automatically and unrelated to this PR).

ChecklistAtomContentClient::AddAdditionalSchemes

Release Notes

Notes: Fixed command-line scheme arguments from spilling over into each other.

The value of one of the scheme command-line switches
shouldn't spill over into other switches.

Fixes #19911
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 23, 2019
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

Copy link
Member

@codebytere codebytere left a comment

Choose a reason for hiding this comment

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

rad catch 🌟

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 24, 2019
@zcbenz zcbenz merged commit 080fdb3 into master Aug 26, 2019
@release-clerk
Copy link

release-clerk bot commented Aug 26, 2019

Release Notes Persisted

Fixed command-line scheme arguments from spilling over into each other.

@zcbenz zcbenz deleted the fix-command-line-schemes-splillover branch August 26, 2019 00:40
@trop
Copy link
Contributor

trop bot commented Aug 26, 2019

I have automatically backported this PR to "5-0-x", please check out #19939

@trop
Copy link
Contributor

trop bot commented Aug 26, 2019

I have automatically backported this PR to "6-0-x", please check out #19940

@trop trop bot removed the target/5-0-x label Aug 26, 2019
@trop
Copy link
Contributor

trop bot commented Aug 26, 2019

I have automatically backported this PR to "7-0-x", please check out #19941

ckerr added a commit that referenced this pull request Aug 26, 2019
@trop
Copy link
Contributor

trop bot commented Aug 26, 2019

A maintainer has manually backported this PR to "4-2-x", please check out #19954

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
5.0.x
Fixed in 5.0.11
6.1.x
Fixed in 6.0.5
7.2.x
Fixed in 7.0.0-beta.4
Development

Successfully merging this pull request may close these issues.

command-line scheme switches' values spill over into other switches
4 participants