1. Introduce ws_client struct
2. Handle all communications from websocket.c for WS clients
3. Always use a dedicated Redis connection for WS clients
4. Add rbuf & wbuf evbuffers for incoming & outgoing WS data
5. Use event_base_once to control R/W events
6. WS test: make sure to read complete HTTP response
For WS clients, reuse a persistent cmd struct attached to the
http_client object: take the cmd built from the WS frame, and copy it to
the persistent cmd.
1. Only HTTP-based pub-sub clients were re-using a cmd object, but
WS clients were not. This led to the commands sent by a WS client
to be processed out of order, just queued to Redis but with no
guarantee that they would be de-queued from the event loop in the
same order. This change attaches a permanent cmd object (with its
associated Redis context) to WS clients just like pub-sub clients
do.
2. WS responses are also no longer sent out of order, but added to a
write buffer that is scheduled for writing as long as there is still
some data left to send. This replaces the use of http_response which
contained extra fields (headers, HTTP response) that were duplicated
without ever being sent out.
Both ws_handshake_reply and ws_reply build http_response objects without
using the status code or headers, this code can be refactored to use a
single method.
The implementation was waiting for the client, which leaves some hanging
even after they called close(). This mirrors the behavior for HTTP
connections in client.c where close() is called right before
http_client_free.
1. Origin and Sec-WebSocket-Origin are now optional: only return a matching
Sec-WebSocket-Origin if one of the two headers was provided.
2. Change order of headers: return Sec-WebSocket-Accept immediately
after Upgrade and Connection since some clients expect it there.
If the client closes the connection before we can even start to respond,
the event_add will fail and we need to clean up the http_response object
since this would have happened only after we've sent the whole response.
ws_handshake_reply was sending its response using a single write(2) call
without error-checking, which could well send only part of it and leave
the client hanging. This change creates an HTTP response object and
schedules it for writing using the event loop.
Tested with Valgrind, no memory is leaked.
The keep_alive flag is needed on http_response for websocket
connections. Without it, the server closes the connection as soon as a
reply to the first frame is sent.
Mostly adding trace logs to websocket.c, but also some in http.c and
worker.c for events relating to event loop and client requests and
responses. This is useful for debugging websocket issues.
* Add WEBDIS_TRACE log level for internal operations
* Warn when verbosity config is invalid
* Add slog_enabled to bypass buffer allocations if the level is filtered
* Only process `Connection: close` header if full request was read
(#194). This likely fixes the same issue also reported in #145.
* Fix small memory leak when the `type` query string parameter is
used; the value was not being freed leading to growing memory usage
of a few bytes per request. Upgrading is recommended if you use this
feature.
* Fix invalid call to `ioctl`, which did not seem to affect Linux
systems but could have had an impact on macOS (found in #197).
The `request` parameter is unsigned long, not int. This was invalid on
macOS and caused issues when sockets were considered non-blocking. Also
adds an error log if the call fails.
Thanks @likuilin for opening an issue that led to this discovery.
Passing `?type=foo/bar` in the query string makes Webdis return the
response with a `Content-Type: foo/bar` header (this is useful to serve
files from Webdis, e.g. web page or their dependencies such as CSS,
images, etc). I discovered with Valgrind that the *value* of this query
string parameter was leaked and never freed, which would likely not
cause a huge issue but would still gradually grow the memory usage.
There were 2 different functions taking care of this parameter, the
first calling strdup(3) on it and the second *transferring* pointer
ownership into it (meaning overwriting the just-strdup'd value).
This is now fixed, Webdis no longer leaks this small string, and an
allocation was avoided.
* Fixed compilation warnings
* Fixed code quality issues found by CodeQL
* Upgraded base image from alpine:3.12.6 to alpine:3.12.7
See CWE-125 and CVE-2021-30139). This is *not* a security issue if
you just use the webdis image to run the service, but could be if
you're building a new Docker image using webdis as a base image.
* CodeQL: overrunning write in jansson/dump.c
* CodeQL: overrunning write in http.c
* CodeQL: redundant condition in websocket.c
* CodeQL: redundant condition in jansson/utf.c
* CodeQL: File created without restricting permissions in server.c
* CodeQL: Futile conditional in pool.c
* CodeQL: Too many arguments in jansson/load.c
* CodeQL: Commented-out code in http.c
* Jansson: disable truncation warning locally in error reporting function
* Fixed compilation warnings
* Fsync frequency for log file is now configurable
* Added support for REPLY_STATUS in nested JSON objects (helps with
RediSearch)
When strings are added as elements of an array but typed as
REDIS_REPLY_STATUS instead of REDIS_REPLY_STRING, Webdis encodes them as
nulls. REDIS_REPLY_STATUS should only be encoded as [true, str] or
[false, str] when this is a top-level status response, not an array
element. In these cases we only need the string.
Fixes#188
Webdis used to call fsync after every single log message, which had a
significant negative impact on performance. This change introduces 3
config options for fsync: no explicit fsync (the new default), a periodic
fsync called every N milliseconds, or the old behavior.
The new config key is also documented and validates its inputs.
1. plaintext was not free'd after encoding credentials
2. ACL commands were duplicated when there was no need to
In both cases the value came from conf_string_or_envvar which always
uses strdup.
We were ignoring the response sent by Redis for AUTH commands. This
commit adds a callback which logs the response; I've tested it with
valid and invalid credentials and the log message is correct in both
cases. There's a lock on the server object to only log this once; I
tried adding it on the pool object but there's one pool per thread so
we still ended up with multiple messages.
A single symbol was added to the log depending on the level, one of ".-*#"
This had an issue: there were only 4 symbols but there are 5 levels; in
addition a `%b` was used which logged a number instead of a letter.
This commit changes the logic to add a single uppercase letter instead,
based on the level (e.g. WEBDIS_ERROR is E, _INFO is I, etc.)
* Change redis_auth in struct conf to handle old and new auth
* Update cfg.c to understand an array of two strings for redis_auth
* Update pool.c to send both username and password
* Use `localtime_r` instead of `localtime`
* Use correct argument type in callback to `msgpack_packer_new`
* Address FIXME in conf.c
* Remove redundant check in websocket.c
Formatting changes:
* Added `{}` after each single-line `if` (see "goto fail" bug).
* Added spaces around operators
Object key changes: reviewed the docs and selected more appropriate
names for various object keys.
This provides a way to start webdis with dynamic port allocation and
discover HTTP port number by simply grepping logs i.e. without need to
use netstat or sockstat that are not available on some architectures.
Dynamic port allocation is a feature that can be used to run webdis
ad-hoc for testing purpose.
Jessie Murray
Nicolas Favre-Felix
Jessie Murray
majklik
Andrii Senkovych