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.

Sign up for GitHub

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

Jump to bottom

Subsume host-sources when union merge #205

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Subsume host-sources when union merge #205

wants to merge 3 commits into from

Conversation

shekyan
Copy link
Collaborator

@shekyan shekyan commented Aug 15, 2018

fixes #200

michaelficarra
michaelficarra reviewed Aug 15, 2018
View reviewed changes
src/main/java/com/shapesecurity/salvation/data/Policy.java
@@ -171,7 +171,7 @@ private void optimise() {
if (directive instanceof SourceListDirective) {
SourceListDirective sourceListDirective = (SourceListDirective) directive;
Optional<SourceExpression> star =
sourceListDirective.values().filter(x -> x instanceof HostSource && ((HostSource) x).isWildcard())
sourceListDirective.values().filter(x -> x instanceof HostSource && ((HostSource) x).isTLDWildcard())
.findAny();
if (star.isPresent()) {
Copy link
Member

Choose a reason for hiding this comment

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

We should change this to also do the subsumption test.

@kingthorin
Copy link
Contributor

Now conflicting.

@bakkot
Copy link
Member

bakkot commented Jun 24, 2019 •
edited

Ideally this would also handle:

  • intersections (e.g. intersecting script-src http://*.atest.com and script-src http://b.c.atest.com should intersect to script-src http://b.c.atest.com, not script-src 'none'; this is a correctness issue)
  • 'self'
  • scheme-sources which subsume host-sources (e.g. script-src https: and script-src https://example.com should union to script-src https:)

Also: unioning script-src 'self', script-src http://example.com/path (on origin example.com) should produce script-src http://example.com, not script-src http://example.com/path.

@michaelficarra michaelficarra changed the base branch from master to main August 12, 2020 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelficarra michaelficarra michaelficarra left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

unioning overlapping host-sources
4 participants
@shekyan @kingthorin @bakkot @michaelficarra