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

Support librejs browser plugin. #1058

Merged
merged 2 commits into from Mar 7, 2016

Conversation

Projects
None yet
3 participants
@mnd
Contributor

mnd commented Dec 3, 2014

For client javascript code written by E14N add stylized comments around license notice. For 3rd party code verbatim copied from other projects create JavaScript Web Labels table and keep scripts code unchanged.

With this changes LibreJS plugin would not block scripts on pump.io servers.

@strugee

This comment has been minimized.

Show comment
Hide comment
@strugee

strugee Feb 19, 2016

Member

@mnd can you rebase this on top of current master? This will allow Travis to run with fixed tests.

Member

strugee commented Feb 19, 2016

@mnd can you rebase this on top of current master? This will allow Travis to run with fixed tests.

@mnd

This comment has been minimized.

Show comment
Hide comment
@mnd

mnd Feb 21, 2016

Contributor

@strugee: For now I at business trip. I'll do it on next week.

Contributor

mnd commented Feb 21, 2016

@strugee: For now I at business trip. I'll do it on next week.

@mnd

This comment has been minimized.

Show comment
Hide comment
@mnd

mnd Mar 3, 2016

Contributor

Rebased. Checks passed. And my instance works with latest LibreJS.

Contributor

mnd commented Mar 3, 2016

Rebased. Checks passed. And my instance works with latest LibreJS.

@strugee

This comment has been minimized.

Show comment
Hide comment
@strugee

strugee Mar 3, 2016

Member

@mnd thanks! The code looks good, but I'm not sure this will work properly with CDN'd instances. Specifically, if CloudFlare doesn't serve license metadata, doesn't that mean that this won't work properly? (Maybe I'm misremembering how LibreJS works, though.)

Thoughts?

Member

strugee commented Mar 3, 2016

@mnd thanks! The code looks good, but I'm not sure this will work properly with CDN'd instances. Specifically, if CloudFlare doesn't serve license metadata, doesn't that mean that this won't work properly? (Maybe I'm misremembering how LibreJS works, though.)

Thoughts?

@mnd

This comment has been minimized.

Show comment
Hide comment
@mnd

mnd Mar 3, 2016

Contributor

This will work properly with any value for config.noCDN. In patch I add license notes only in pump.io own javascript code. All 3rd party code was not changed. Instead I added next string in public/template/layout.utml file:
<p><a href="/javascript/about.html" data-jslicense="1">JavaScript license information</a></p>
Where about.html file contain license information for both: local 3rd party .js files and javascript files at cdnjs.cloudflare.com

You can look at LibreJS reports for noCDN = true and for noCDN = false cases

Contributor

mnd commented Mar 3, 2016

This will work properly with any value for config.noCDN. In patch I add license notes only in pump.io own javascript code. All 3rd party code was not changed. Instead I added next string in public/template/layout.utml file:
<p><a href="/javascript/about.html" data-jslicense="1">JavaScript license information</a></p>
Where about.html file contain license information for both: local 3rd party .js files and javascript files at cdnjs.cloudflare.com

You can look at LibreJS reports for noCDN = true and for noCDN = false cases

@strugee

This comment has been minimized.

Show comment
Hide comment
@strugee

strugee Mar 5, 2016

Member

Ah, okay. Forgot that LibreJS could parse files like about.html.

Final note that I just thought of: should there be a way to turn off the LibreJS support in case the person running the instance wants to patch the software, but not release their code? I can easily imagine someone just assuming that they could patch JS files without bothering to read the license header...

Member

strugee commented Mar 5, 2016

Ah, okay. Forgot that LibreJS could parse files like about.html.

Final note that I just thought of: should there be a way to turn off the LibreJS support in case the person running the instance wants to patch the software, but not release their code? I can easily imagine someone just assuming that they could patch JS files without bothering to read the license header...

@mnd

This comment has been minimized.

Show comment
Hide comment
@mnd

mnd Mar 5, 2016

Contributor

@strugee I am not sure if such solution really required. If somebody change code and provide it in source form (i.e. not minified) in file with license mention on top of file then you can use code under this license. If he provide code in minified form then minifier deleted license label from file together with other comments. And if he does not change code, but instead he used additional script files (added in source or enabled through config.scripts) then this files would not be mentioned in "public/javascript/about.html" file.

Just now I committed changes that apply license only to standard inlined javascript code, so server owner now can add non-free inlined javascript code.

If you really think that server owner must be able to disable license labels through config you can use next solution: mnd@d0c06f3

Contributor

mnd commented Mar 5, 2016

@strugee I am not sure if such solution really required. If somebody change code and provide it in source form (i.e. not minified) in file with license mention on top of file then you can use code under this license. If he provide code in minified form then minifier deleted license label from file together with other comments. And if he does not change code, but instead he used additional script files (added in source or enabled through config.scripts) then this files would not be mentioned in "public/javascript/about.html" file.

Just now I committed changes that apply license only to standard inlined javascript code, so server owner now can add non-free inlined javascript code.

If you really think that server owner must be able to disable license labels through config you can use next solution: mnd@d0c06f3

mnd added some commits Dec 2, 2014

Support librejs browser plugin.
For client javascript code written by E14N add stylized comments around license
notice. For 3rd party code verbatim copied from other projects create JavaScript
Web Labels table and keep scripts code unchanged.
Apply license only to specific javascript code
We can not be sure that other inlined javascript code is free
@strugee

This comment has been minimized.

Show comment
Hide comment
@strugee

strugee Mar 7, 2016

Member

@mnd you're right! Seems like that covers all cases. LGTM, thanks!

Member

strugee commented Mar 7, 2016

@mnd you're right! Seems like that covers all cases. LGTM, thanks!

strugee added a commit that referenced this pull request Mar 7, 2016

Merge pull request #1058 from mnd/librejs-compatible
Support librejs browser plugin.

@strugee strugee merged commit 5d225c1 into pump-io:master Mar 7, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@larjona

This comment has been minimized.

Show comment
Hide comment
@larjona

larjona Mar 18, 2016

Collaborator

The corresponding issue: #462

Collaborator

larjona commented Mar 18, 2016

The corresponding issue: #462

strugee added a commit that referenced this pull request Aug 27, 2016

1.0.0
This release adds many security features. It's recommended that admins
upgrade as soon as possible.

Please note that while we're not doing so _yet_, we're planning to
deprecate running under Node.js 0.10 and 0.12 very soon. Additionally,
upgrading to Node.js 4.x early will enable the new, better XSS scrubber
- _however_, be aware that pump.io is far less tested under Node.js 4.x
and you are likely to run into more bugs than you would under 0.10 or
0.12.

See #1184 for details.

* [API] Send the `Content-Length` header in Dialback requests
* Add support for [LibreJS][librejs] (#1058)
* Node.js 4.x is officially supported (#1184)
* Browser MIME type sniffing is disabled via `X-Content-Type-Options:
  nosniff` ([#1184][security-headers])
* Protect some versions of Internet Explorer from a confused deputy
  attack via `X-Download-Options: noopen` ([#1184][security-headers])
* Make sure Internet Explorer's built-in XSS protection is as secure as
  possible with `X-XSS-Protection: 1;
  mode=block` ([#1184][security-headers])
* Versions of Internet Explorer the XSS scrubber can't protect are
  presented with a security error instead of any content (#1184)
* Clickjacking is prevented via `X-Frame-Options: DENY` header (in
  addition to Content Security Policy) ([#1184][security-headers])
* A `Content-Security-Policy` header is sent with every response (#1184)
  * Scripts are forbidden from everywhere except the application domain
    and (if CDNs are enabled) `cdnjs.cloudflare.com` and
    `ajax.googleapis.com`
  * Styles are forbidden from everywhere except the application domain
    and inline styles
  * `<object>`, `<embed>`, and `<applet>`, as well as all plugins, are
    forbidden * Embedding the web UI via `<frame>`, `<iframe>`,
    `<object>`, `<embed>`, and `<applet>` is forbidden
  * Connecting to anything other than the application domain via
    `XMLHttpRequest`, WebSockets or `EventSource` is forbidden
  * Loading Web Workers or nested browsing contexts (i.e. `<frame>`,
    `<iframe>`) is forbidden except from the application domain
  * Fonts are forbidden from everywhere except the application domain
  * Form submission is limited to the application domain

* [API] Don't return `displayName` properties if they're empty (#1149)
* Upgraded from Connect 1.x to Connect 2.x
* Upgraded various minor dependencies
* All files pass style checking and most pass JSHint
* Add links to the user guide on the homepage and welcome
  message (#1125)
* Add a new XSS scrubber implementation (#1184)
  * DOMPurify-based scrubber is used on Node.js 4.x or better
  * Otherwise, a more intrusive, less precise one is used

* Fix a crash upon access of an activity without any replies (#1135)
* Disable registration link if registration is disabled (#853)
* `package.json` now uses a valid SPDX license identifier (#1112)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment