Skip to content

Added support for Subresource Integrity #315

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
Merged
merged 8 commits into from
Feb 17, 2022
Merged

Added support for Subresource Integrity #315

merged 8 commits into from
Feb 17, 2022

Conversation

thejoeejoee
Copy link
Contributor

  • added support for SRI via INTEGRITY configuration option
  • integrity: true configuration for the bundle tracker is needed

@fjsj fjsj requested review from rvlb and fjsj February 16, 2022 12:58
@fjsj fjsj mentioned this pull request Feb 16, 2022
Closed
fjsj
fjsj reviewed Feb 16, 2022
View reviewed changes
fjsj
fjsj reviewed Feb 16, 2022
View reviewed changes
fjsj
fjsj reviewed Feb 16, 2022
View reviewed changes
fjsj
fjsj reviewed Feb 16, 2022
View reviewed changes
tests/app/tests/test_webpack.py
self.compile_bundles('webpack.config.integrity.js')

loader = get_loader(DEFAULT_CONFIG)
with patch.dict(loader.config, {'INTEGRITY': True}):
Copy link
Member

Choose a reason for hiding this comment

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

Please add another test that uses a loader.config without any 'INTEGRITY' key. It should work as 'INTEGRITY': False. This will ensure backwards compatibility.

Copy link
Contributor Author

@thejoeejoee thejoeejoee Feb 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Added test_integrity_missing_config:

https://github.com/thejoeejoee/django-webpack-loader/blob/9c36bbe7718c8963e620e7613dc4bbd002119e00/tests/app/tests/test_webpack.py#L232-L253

fjsj
fjsj requested changes Feb 16, 2022
View reviewed changes
Copy link
Member

@fjsj fjsj left a comment

Choose a reason for hiding this comment

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

Thanks, PR looks great, but I think we need some changes to keep backwards compatibility on integrity config.

@thejoeejoee
Copy link
Contributor Author

thejoeejoee commented Feb 16, 2022 •
edited
Loading

glad to contribute:

  • added test case for missing INTEGRITY config key
  • replaced direct config indexing by dict.get()
  • moved whitespace attr separator to method constructing the attribute itself
  • rebased onto master

@thejoeejoee thejoeejoee requested a review from fjsj February 16, 2022 21:58
@fjsj fjsj merged commit 2c1d92c into django-webpack:master Feb 17, 2022
@fjsj
Copy link
Member

fjsj commented Feb 17, 2022

Thanks @thejoeejoee . I made a small fix at 6a5a880 and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvlb rvlb rvlb approved these changes

@fjsj fjsj Awaiting requested review from fjsj

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

Successfully merging this pull request may close these issues.
3 participants
@thejoeejoee @fjsj @rvlb