Various tidy up and linting of contentcollector.js and domline.js.
3 Tests disabled which are not due to be covered.
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
* remove IE and add strict headers
* linting: kids are back, need to stop for today
* linting: farbtastic fix
* lint: more lint fixes
* more lint fixes
* linting: sub 100 errors
* comments where I need help
* ready to be helped :)
* small fixes
* fixes
* linting: all errors resolved
* linting: remove note to self
* fix as per nulli/wezz000li suggestion
* fix as per nulli/wezz000li suggestion
* resolve merge conflicts
* better use if to silence eslint
* Use `for..of` with `Object.keys` instead of `for..in`
* lint: move setSelection to before call
Co-authored-by: webzwo0i <webzwo0i@c3d2.de>
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
This will make the pages gracefully handle HTTP server restart events,
which happen whenever a plugin is installed or uninstalled via the
`/admin/plugins` page.
* lint: collab-client
* Undo incorrect lint fixes
These will be re-fixed in a future commit.
* Properly fix guard-for-in error
* Properly fix prefer-rest-params errors
* Move some code back to where it was
Moving the code makes it hard to review the diff.
* Delete DISCONNECT_REASON case
Someone reading the code won't understand what "used to handle
appLevelDisconnectReason" means until they dig through the Git
history. Given the server never sends messages of type
DISCONNECT_REASON anyway, just delete the case.
* Refine lint fixes
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
Squashed changes from rhansen@rhansen.org:
* Move code back to where it was. (It's easier to review changes
when the code isn't moved. This causes some no-use-before-define
warnings to reappear, but those are just warnings.)
* Move eslint-disable comment to same line
* Use `window.clientvars` to resolve no-global-assign
* Undo changes that aren't about fixing lint errors
* lint: pluginfw tsort.js
* Don't comment out the `console.log()` call
Disabling the log message is out of scope for the pull request.
* Put const and let on separate lines
* Convert `tsort` from function to arrow function
ESLint doesn't complain about this due to a bug in
prefer-arrow/prefer-arrow-functions rule:
https://github.com/TristonJ/eslint-plugin-prefer-arrow/issues/24
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
* fix accidental write to global variable
properly show pending tests
log test name in suite
better log output for received/expected strings
* cc tests: enable second nestedOL test
* ignore the head tag on import
Missing await in call to this._pad.getInternalRevisionAText(rev). Function returns a promise. This bug breaks the createDiffHTML API call (how I discovered it).
These characters are in the RFC3986 reserved set.
These characters are added to the set of characters that cannot be the
last character of a URL to avoid mislinkification.
It should be the client's responsibility to handle null name or color.
In the case of author names, passing null to the client allows users
to fill in the names of other users (via a suggestUserName
CLIENT_MESSAGE).
When a new client opens a socket.io connection and sends a
CLIENT_READY message, Etherpad sends the new client a bunch of
USER_NEWINFO messages, one per other user already connected to the
pad. When iterating over the other users, filter out those without an
author ID or missing from the global authors database.
Normally I would let `eslint --fix` do this for me, but there's a bug
that causes:
const x = function ()
{
// ...
};
to become:
const x = ()
=> {
// ...
};
which ESLint thinks is a syntax error. (It probably is; I don't know
enough about the automatic semicolon insertion rules to be confident.)
* caching_middleware: fix gzip compression not triggered
* packages: If a client sets `Accept-Encoding: gzip`, the responseCache will
include `Content-Encoding: gzip` in all future responses, even
if a subsequent request does not set `Accept-Encoding` or another client
requests the file without setting `Accept-Encoding`.
Fix that.
* caching_middleware: use `test` instead of `match`
* add tests
* make code easier to understand
* make the regex more clear
* Fix bad paren placement in `/javascript` handler
This fixes a bug introduced in commit
ed5a635f4c.
* add regression test for #4495
* Move `/javascript` test to `specialpages.js`
Co-authored-by: webzwo0i <webzwo0i@c3d2.de>
Some authentication plugins use the users defined in the `users`
object but ignore the `password` and `hash` properties.
This change deletes all of the filtering logic, including the logic
that filters out users that have both `password` and `hash` properties
defined. I could have kept that check, but decided to remove it
because:
* There's no harm in defining both `hash` and `password`.
* Allowing both makes it easier to transition from one scheme to
another.
* It's fewer lines of code to maintain.
If `settings.json` contains a user without a `password` property then
nobody should be able to log in as that user using the built-in HTTP
basic authentication. This is true both with and without this change,
but before this change it wasn't immediately obvious that a malicious
user couldn't use an empty or null password to log in as such a user.
This commit adds an explicit nullish check and some unit tests to
ensure that an empty or null password will not work if the `password`
property is null or undefined.
This makes it possible to disable `contentEditable` for certain
elements in some circumstances (e.g., on links so that users can click
on them normally).
if animationState evaluates to -1 or 0, it would end up in a conditional that assign its value to itself. Since this is redundant, it is better to remove this conditional, to avoid an extra check
Rewrite the `callAll` and `aCallAll` functions to support all
reasonable hook behaviors and to report errors for unreasonable
behaviors (e.g., calling the callback twice).
Now a hook function like the following works as expected when invoked
by `aCallAll`:
```
exports.myHookFn = (hookName, context, cb) => {
cb('some value');
return;
};
```
If a hook function neither calls the callback nor returns a
(non-undefined) value then there's no way for the hook system to know
if/when the hook function has finished.
* Use jQuery to build the message HTML so that special characters in
the error message, URL, etc. are properly escaped. This helps
avoid XSS vulnerabilities.
* Use bold text for the error message to make it stand out.
* Add a line break between the error message and "in <url> at line
<line>" so that the error message stands out more.
* Use `<p>...</p>` instead of `</br>` to separate the parts of the
popup.
* Use CSS for spacing instead of `</br>`.
* Grammar fixes (add a missing comma, "at" instead of "in").
Teach Gritter to accept anything that jQuery's `.append()` method
accepts for the title and text of a popup message. This makes it
easier to safely build HTML messages with proper escaping of special
characters (to prevent XSS vulnerabilities).
The debug statement mostly printed the following useless message over
and over, causing Travis CI logs to become truncated:
[DEBUG] pluginfw - [ undefined ] returning
This will be a breaking change for some people.
We removed all internal password control logic. If this affects you, you have two options:
1. Use a plugin for authentication and use session based pad access (recommended).
1. Use a plugin for password setting.
The reasoning for removing this feature is to reduce the overall security footprint of Etherpad. It is unnecessary and cumbersome to keep this feature and with the thousands of available authentication methods available in the world our focus should be on supporting those and allowing more granual access based on their implementations (instead of half assed baking our own).
We could instead await the results of the hook, but then all callers
and their callers recursively would have to be converted to async, and
that's a huge change.
This currently isn't absolutely necessary because all current callers
of `userCanModify` already check for a read-only pad ID themselves.
However:
* This adds defense in depth.
* This makes it possible to simply replace the import handler's
`allowAnyoneToImport` check with a call to `userCanModify`.
There's no need to perform an authentication check in the socket.io
middleware because `PadMessageHandler.handleMessage` calls
`SecurityMananger.checkAccess` and that now performs authentication
and authorization checks.
This change also improves the user experience: Before, access denials
caused socket.io error events in the client, which `pad.js` mostly
ignores (the user doesn't see anything). Now a deny message is sent
back to the client, which causes `pad.js` to display an obvious
permission denied message.
This also fixes a minor bug: `settings.loadTest` is supposed to bypass
authentication and authorization checks, but they weren't bypassed
because `SecurityManager.checkAccess` did not check
`settings.loadTest`.
