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

Generate integrity hash for scripts/link tags on the index. #442

Closed
wants to merge 10 commits into from

Conversation

T4rk1n
Copy link
Contributor

@T4rk1n T4rk1n commented Oct 31, 2018

  • Generate a sha384 hash from the local copy of the file.
  • Format the tags with integrity and crossorigin attributes.
  • Flattened the resources collection from the dist.
  • Map external unpkg to local host file.

As the version of plotly.js hosted locally and externally was not the same, this breaks dash-core-components Graph component.

The files hosted locally must be downloaded from the external source so they match 1:1. Same for the component libs bundles, do npm publish first so you have the builds with the prepublish hook, then do python setup.py sdist and twine upload dist/* without rebuilding the bundles so they have the same hash, all version of dcc I tested had the right bundles, but it needs to be said somewhere.

Closes #422

@T4rk1n T4rk1n requested review from ned2 and rmarren1 October 31, 2018 17:13
@chriddyp
Copy link
Member

This is very cool. We should note in our documentation and the changelog the browser support https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity#Browser_compatibility (not supported in IE. partial support in Edge, not sure what that means)

@T4rk1n
Copy link
Contributor Author

T4rk1n commented Oct 31, 2018

@chriddyp I tested from ie11 and edge, it loaded, I think it just ignore the attribute.

@T4rk1n T4rk1n added the dash 2.0 label Nov 1, 2018
@T4rk1n
Copy link
Contributor Author

T4rk1n commented Nov 1, 2018

Tagging this as dash 2.0 as it breaks old versions of dcc that did not have the same plotly.js files. It's a nice feature, but not essential.

dash/_utils.py Outdated

@functools.wraps(func)
def wrapper(*args, **kwargs):
key = hash((args, frozenset(kwargs)))
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be hash((args, frozenset(kwargs.keys()), frozenset(kwargs.values()))?

frozenset(kwargs) gets a set of the keys, so function(1, 2, a=1) and function(1, 2, a=2) would produce the same hash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, the function it wraps doesn't have kwargs so it didn't impact it. I think frozenset(kwargs.items()) so the values and keys are grouped together in a tuple?

@rmarren1
Copy link
Contributor

rmarren1 commented Nov 3, 2018 via email

@T4rk1n T4rk1n changed the base branch from master to 1.0.0-release December 5, 2018 17:17
@alexcjohnson
Copy link
Collaborator

This has gotten stale - closing. Will likely need to be reimplemented if we pick this feature up again.

@alexcjohnson alexcjohnson deleted the sri branch August 13, 2019 01:19
HammadTheOne pushed a commit to HammadTheOne/dash that referenced this pull request May 28, 2021
…y#442)

* 🎉 initial commit of dashTable R package

* border is none if not define

* test cases for css border overwrite bugs

* edit CHANGELOG.md

* Update CHANGELOG.md

Co-Authored-By: Marc-André Rivet <Marc-Andre-Rivet@users.noreply.github.com>

* Fix monorepo build (plotly#443)

monorepo changes + shallow clone + v0/v1 clean up

* Create FUNDING.yml

* 📝 correct "dash green" to "dashed blue"
@mkhorton
Copy link

Tagging this as dash 2.0 as it breaks old versions of dcc that did not have the same plotly.js files. It's a nice feature, but not essential.

With Dash 2.0 on the horizon, has this feature been re-evaluated? It would give a lot more confidence if using third-party CDNs like unpkg.

@alexcjohnson
Copy link
Collaborator

It's not on our roadmap ATM, but if anyone wants to tackle it we'd be happy to review a PR!

@mkhorton
Copy link

@alexcjohnson Thanks for the response. Is the approach taken by this PR no longer feasible? It looks like a sensible approach.

HammadTheOne pushed a commit that referenced this pull request Jul 23, 2021
* 🎉 initial commit of dashTable R package

* border is none if not define

* test cases for css border overwrite bugs

* edit CHANGELOG.md

* Update CHANGELOG.md

Co-Authored-By: Marc-André Rivet <Marc-Andre-Rivet@users.noreply.github.com>

* Fix monorepo build (#443)

monorepo changes + shallow clone + v0/v1 clean up

* Create FUNDING.yml

* 📝 correct "dash green" to "dashed blue"
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.

5 participants