Skip to content

pseudopatterns polluting base patterns #744

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

Merged
merged 3 commits into from
Nov 10, 2017
Merged

Conversation

bmuenzenmeyer
Copy link
Member

@bmuenzenmeyer bmuenzenmeyer commented Nov 10, 2017

Addresses #711

Summary of changes:

pseudopattern_hunter is performing a dangerous lodash merge against the base pattern data

//extend any existing data with variant data
variantFileData = _.merge(currentPattern.jsonFileData, variantFileData);

resulting in the mutation of the currentPattern.jsonFileData object.

Luckily, we get around this elsewhere by using jsonCopy on the merged objects.

I also verified this now using starterkit-mustache-demo.

FIX NOT APPLIED, DASHBOARD

image

(wrong data)

FIX NOT APPLIED, DASHBOARD ~ HACKED

image

(wrong data)

FIX APPLIED, DASHBOARD

image

(correct default data)

FIX APPLIED, DASHBOARD ~ HACKED

image

(correct override data)

@bmuenzenmeyer bmuenzenmeyer self-assigned this Nov 10, 2017
@bmuenzenmeyer bmuenzenmeyer changed the title fix(pseudopattern)hunter): Add failing test fix(pseudopattern_hunter): Add failing test Nov 10, 2017
@bmuenzenmeyer bmuenzenmeyer changed the title fix(pseudopattern_hunter): Add failing test pseudopatterns polluting base patterns Nov 10, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 72.528% when pulling 2c8592a on fix/711-pseudopatterns into fca4d03 on dev.

1 similar comment
@coveralls
Copy link

coveralls commented Nov 10, 2017

Coverage Status

Coverage decreased (-0.2%) to 72.528% when pulling 2c8592a on fix/711-pseudopatterns into fca4d03 on dev.

@coveralls
Copy link

coveralls commented Nov 10, 2017

Coverage Status

Coverage decreased (-0.2%) to 72.528% when pulling 2c8592a on fix/711-pseudopatterns into fca4d03 on dev.

@bmuenzenmeyer
Copy link
Member Author

(code coverage decrease is likely due to deleting duplicate code, not the fix itself)

@bmuenzenmeyer bmuenzenmeyer merged commit c964462 into dev Nov 10, 2017
@bmuenzenmeyer bmuenzenmeyer deleted the fix/711-pseudopatterns branch November 10, 2017 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants