Also converted the handler functions that depend on checkAccess() into async
functions too.
NB: this commit needs specific attention to it because it touches a lot of
security related code!
This patch also contains significant refactoring relating to error checking of
arguments supplied to the functions (e.g. rev) facilitated by use of `throw`
instead of nodeback errors.
1. This module was not migrated to Promises, because it is only used via
express-session, which can't actually use promises anyway.
2. all(), clear() and length() depend on the presence of the `db.forEach()`
function, which in ueberdb2 doesn't even exist.
Fortunately those three methods are optional, so I made their existence
conditional on the presence of `db.forEach`.
3. in SessionStore.clear(), replaced a call to db.db.remove() with db.remove()
Changed two occurrences of:
process.nextTick(function() {
if (fn) fn();
});
with
if (fn) {
process.nextTick(fn);
}
i.e. such that no function even gets added to the `nextTick` queue unless
there's actually a function to be called.
Extracted from Ray's work.
`getPadAccess()` (src/node/padaccess.js) is now "promise only", resolving to
`true` or `false` as appropriate, and throwing an exception if there's an
error.
The two call sites (padreadonly.js and importexport.js) updated to match.
Use real `async` instead of async.js where applicable.
The `getPluginTests()` function was never truly async anyway because it only
contains calls to synchronous `fs` modules.
This change is in preparation of the future async refactoring by Ray. It tries
to extract as many changes in boolean conditions as possible, in order to make
more evident identifying eventual logic bugs in the future work.
This proved already useful in at least one case.
BEWARE: this commit exposes an incoherency in the DB API, in which, depending
on the driver used, some functions can return null or undefined. This condition
will be externally fixed by the final commit in this series ("db/DB.js: prevent
DB layer from returning undefined"). Until that commit, the code base may have
some bugs.
This change extracts the grammar correction performed on the async branch,
anticipating them in a single commit. It cannot be folded with the previous
one, as it is not purely cosmetic.
This change is only cosmetic. Its aim is do make it easier to understand the
async changes that are going to be merged later on. It was extracted from the
original work from Ray Bellis.
To verify that nothing has changed, you can run the following command on each
file touched by this commit:
npm install uglify-es
diff --unified <(uglify-js --beautify bracketize <BEFORE.js>) <(uglify-js --beautify bracketize <AFTER.js>)
This is a complete script that does the same automatically (works from a
mercurial clone):
```bash
#!/usr/bin/env bash
set -eu
REVISION=<THIS_REVISION>
PARENT_REV=$(hg identify --rev "${REVISION}" --template '{p1rev}')
FILE_LIST=$(hg status --no-status --change ${REVISION})
UGLIFYJS="node_modules/uglify-es/bin/uglifyjs"
for FILE_NAME in ${FILE_LIST[@]}; do
echo "Checking ${FILE_NAME}"
diff --unified \
<("${UGLIFYJS}" --beautify bracketize <(hg cat --rev "${PARENT_REV}" "${FILE_NAME}")) \
<("${UGLIFYJS}" --beautify bracketize <(hg cat --rev "${REVISION}" "${FILE_NAME}"))
done
```
This is documented to be more performant.
The substitution was made on frontend code, too (i.e., the one in /static),
because Date.now() is supported since IE 9, and we are life supporting only
IE 11.
Commands:
find . -name *.js | xargs sed --in-place "s/new Date().getTime()/Date.now()/g"
find . -name *.js | xargs sed --in-place "s/(new Date()).getTime()/Date.now()/g"
Not done on jQuery.
The guard condition on count being non negative and < 100 used the wrong
boolean operator. In its form it was impossible.
This error was introduced in 2013, in 5592c4b0fe.
Fixes#3499
The HTTP API doesn't ever omit arguments, it always passes `undefined` for a
parameter that wasn't supplied in the request.
The functions that were simplified are:
- getRevisionChangeset()
- getText()
- getHTML()
- saveRevision()
The only function still supporting optional arguments is getPadSafe(), which is
only called from this module.
This function was simulating two overloads:
1. copy(destinationID, force, callback)
2. copy(destinationID, callback), in this case "force" would be assumed false
But all the call sites always used the version with arity 3.
Thus, we can remove that optionality and always assume that the funcion will be
called with three parameters. This will simplify future work.
- path.exists() is no longer part of nodejs
- fs.exists() is deprecated (as of nodejs >= 8)
- checking a file for existence before using it is open to raca condition. It is
preferable to go ahead and use the file, and eventually handle the error
- we can afford two simple synchronous fs operations here
Next version will be Etherpad 1.8. As planned in #3424, we are going to require
NodeJS >=8.9.0 and npm >= 6.4.
This commit implements that change and updates documentation and scripts.
Subsequent changes will get rid of old idioms, dating back to node < 0.7, that
still survive in the code.
Once migrated to NodeJS 8, we will be able to start working on migrating the
code base from callbacks to async/await, greatly simplifying legibility (see
#3540).
Closes#3557
Until Etherpad 1.7.5, process.on('SIGTERM') and process.on('SIGINT') were not
hooked up under Windows, because old nodejs versions did not support them.
This excluded the possibility of doing a graceful shutdown of the database
connection under that platform.
According to nodejs 6.x documentation, it is now safe to do so. This allows to
gracefully close the DB connection when hitting CTRL+C under Windows, for
example.
Source: https://nodejs.org/docs/latest-v6.x/api/process.html#process_signal_events
- SIGTERM is not supported on Windows, it can be listened on.
- SIGINT from the terminal is supported on all platforms, and can usually be
generated with <Ctrl>+C (though this may be configurable). It is not
generated when terminal raw mode is enabled.
A Windows manual install has the same directory layout of a normal Unix one
(e.g. the nice symlink node_modules/ep_etherpad-lite -> ../src).
Only when running from the pre-built Windows package the directory layout is
different (e.g. src is physically copied into node_modules/ep_etherpad-lite).
The previous version of the code wrongly assumed that all Windows installs would
be run from the pre-built pakage.
In this version the path search is the same on all platform. If it fails, and we
are on Windows, there is a fallback for the specific case of the pre-built
package.
Fixes#3550
This commit vastly shortens (and simplifies) the version table within
handler/APIHandler.js by building each version's entry incrementally based off
the previous version.
The resulting table has been validated by comparing the "before" and "after"
output of the following loop on both versions of the code (albeit with an
intermediate "sort" step to account for the different insertion order)
for (let v in version) {
let m = version[v];
for (let [k, a] of Object.entries(m)) {
console.log(v, k, a);
}
}
The patch also fixes a few typos, and removes a duplicate definition of
getChatHistory which in each applicable version was defined with two different
parameter lists, but where only the second would be used.
Since the original comparison compared for truthy and not for "===", and it's
3 AM now, I blindly negated it, in order to show how fragile it was in the first
instance.
No functional changes.
This is the final commit of this refactoring series.
Get rid of an else branch to simplify code layout. No functional changes at all.
==============
This series is an attempt to reduce the control structure depth of the code
base, maintaining at the same time its exact same behaviour, bugs included. It
is, in a sense, an initial attempt at a refactoring in the spirit of its
original definition [0].
The idea beyond this refactoring is that reducing the code depth and, sometimes,
inverting some conditions, bugs and logic errors may become easier to spot, and
the code easier to read.
When looked at ignoring whitespace changes, all of these diffs should appear
trivial.
[0] https://refactoring.com/
It's just synctactic sugar, but it is always better than executing string
concatenations in one's mind.
Do not do this with files in src/static, because we want to keep IE 11
compatibility.
The old "static/custom" directory is replaced by "static/skins/<skinName>",
where <skinName> is taken from settings.json.
When no value is found, a default of "no-skin" is assumed, so that backward
compatibility is maintained.
The most evident security concerns have been addressed.
Closes#3471.
skinName must be a single string (no directory separators in it) pointing to an
existing directory under /src/static/skins.
In case these conditions are not met, its value is rewritten to "no-skin".
Also, the value of skinName if sent to the client via clientVars for allowing
its use it in the browser.
If you edit `src/templates/export_html.html` to remove the
`<meta name="changedby" content="Etherpad">` tag[1], PDF export with
soffice has a bug: the first word of the pad is deleted and a blank page
is inserted as first page (the pad's text begins on the second page).
The `--writer` soffice option avoids that bug.
[1] you may want to delete that tag since it is inserted as a comment in
.doc or .odt soffice export.
This file uses it for robots.txt and favicon.ico.
This makes use of the new stable settings.root introduced with #3466, and will
be modified when introducing support for custom skins.
This should look to consistent locations when looking for relative paths,
without depending on current working directory.
For absolute paths, nothing changes.
This is just a function (with an ugly side effect for caching purposes) that
heuristically tries to compute the Etherpad installation path when running under
Unix and win32 (they have different file system layouts).
This path can be used by Etherpad as a base for all the relative paths, in order
to be deterministic and not depending on cwd.
This is the location that is choosen by default when Etherpad starts with no
settings.json file.
It was different than the one contained into setting.json.template.
Minimum supported Node version is 6.9.0, but Object.values() was introduced in
Node < 7. Let's use a polyfill if needed.
This will be removed when minimum supported Node version is raised to 8.9.0.
Fixes#3459
When installing dependencies, npm informed us that measured had been deprecated,
and renamed to measured-core. Let's follow the advice, and get rid of the
warning.
npm WARN deprecated measured@1.1.0: This package has been renamed to
measured-core, all versions of measured have been re-released under
measured-core, please update your package and consider updating to the newest
version. See https://github.com/yaorg/node-measured for latest updates.
This package is used to expose a single endpoint ("/stats"), whose output does
not change after this commit.
Fixes#3458
The hostname:port of URIs used in Minify are currently bogus and refer
to localhost only for historical reasons; there's no reason to retain
them and omitting them avoids generating an invalid URI when "port" is
not an integer.
Context: settings.port is passed to express's listen; if not numeric, it
is used a filename for a Unix domain socket.
This allows e.g. starting a server to be reverse-proxied on a multi-user
system, using the filesystem to handle access control and avoiding need
to allocate port numbers.
Before this change, etherpad-lite starts without error when configured
to listen on a Unix domain socket in this manner. However, `pad.js` and
`ace2_common.js` are generated incorrecting, causing an error
"Uncaught Error: The module at "ep_etherpad-lite/static/js/rjquery" does not exist."
when loading the editor:
When settings.port is a non-numeric string, e.g. `etherpad.sock`, a URI
of the form `http://localhost:etherpad.sock/static/js/rjquery.js` is
generated and parsed to find the file needed. In this case, the file
searched for is `:etherpad.sock/static/js/rjquery.js`, rather than the
expected `static/js/rjquery.js`. No such file exists, and the required
code is silently omitted from the bundle.
As a workaround, hard-code a (meaningless) hostname which can be parsed
correctly, since the current code makes no use of it anyway.
Etherpad 1.6.6 does not run on node <= 5 already.
Node 6.9 is the first LTS release in the 6 series, and comes with npm 3.10.8.
Declarations in package.json are advisory unless the user has set
`engine-strict` config flag.
Updated the docs accordingly.
When using LibreOffice to convert pads to doc, we got `Error: no export filter for /tmp/xxxx.doc` (tested with LO 5 and 6). Maybe it's a regression from LO. Anyway, converting HTML to odt, then to doc works.
Thx to lpagliari for her review!
* Add scroll when it edits a line out of viewport
By default, when there is an edition of a line, which is out of the
viewport, Etherpad scrolls the minimum necessary to make this line
visible. This makes that the line stays either on the top or the bottom
of the viewport. With this commit, we add a setting to make possible to
scroll to a position x% pixels from the viewport. Besides of that, we
add a setting to make an animation of this scroll.
If nothing is changed on settings.json the Etherpad default behavior is
kept
Shut down database connection and exit the node process
when SIGTERM is encountered. This is especially important
when nodejs is run as PID1, e.g. in a docker container.
Shutting down connections to clients (browsers) is beyond
this patche's scope.
Resolves#3265
Users still cannot choose which shortcuts they want to enable/disable,
so it does not make sense (yet) to have a cookie with that preference.
This can be reverted once we create an UI to change shortcuts, but
PLEASE PLEASE PLEASE do not read the cookie every time handleKeyEvent is
called!!!
This is an adjustment to #2891.
On some erros that display a modal with "Force reconnect" button, allow
Etherpad to automatically reload pad after a few seconds. Amount of
seconds is defined on settings.json.
Still need to create tests for this feature, and implement i18n.
This meant plumbing a callback through to `compressCSS()`, which meant that
I had to alter the innards of `getFileCompressed()`. I tried to leave that
function looking more understandable than when I found it; for example, I
flattened out the nested `if`.
I went ahead and upgraded the version of `clean-css` while I was in the
territory.
The semantics of all the substitutions are identical to what they were before.
I _did_ take the liberty of formatting the CSS to be a bit more readable
(at the cost of adding a little bit of whitespace).
The new hook does the same as exportHtmlAdditionalTags, but is declared
in another hook to avoid confusion about how to export tags when they
are stored as ['tag', 'value'] on attribute pool.
This complements #2762, as per @Gared suggestions.
Currently pressing ENTER on a line that ends with ':', '[', '(' or '{'
automaticaly indents the new line with 4 spaces. The variable added by
this commit to settings.json allow an Etherpad instance to not have this
behavior.
Address issue 2674 by checking that the session is valid and has a valid
author and padId before using it to update the userInfo for that
session. Otherwise it is possible that by the time we try to update a
session with new userInfo that session has disconnected and is no longer
available to be updated. Without this commit the etherpad-lite service
gracefully shutsdown whenever this happens.
Ray Bellis