One small refactor but one huge step in the right direction
Mostly because adding another check would make the line checking for a
valid check exceed the column limit.
This channel binding method is now enabled when a hash is manually set in the
config, or it attempts to discover the hash automatically if the value is the
special string "auto".
A related change to mod_c2s prevents complicated certificate lookups in the
client connection hot path - this work now happens only when this channel
binding method is used. I'm not aware of anything else that uses ssl_cfg (vs
ssl_ctx).
Rationale for disabling by default:
- Minor performance impact in automatic cert detection
- This method is weak against a leaked/stolen private key (other methods such
as 'tls-exporter' would not be compromised in such a case)
Rationale for keeping the implementation:
- For some deployments, this may be the only method available (e.g. due to
TLS offloading in another process/server).
Some of the new roles don't quite fit nicely into 4 characters
(excluding ellipsis). Given the ability to dynamically add additional
roles from the config and possibly from modules, it seems better to just
make it a relative size since we can't know how long they will be.
Suggested by jstein in the chat
This option label is used by XMPP clients to explain what the option does.
a) The user should know where the data is archived.
b) The user needs a statement that can be enabled/disabled by the variable. A question would have the wrong logic here.
Because it changes the type of the 'opt_origins' variable from util.set
to the internal _items table so next time an http app is added an error
"attempt to call a nil value (method 'empty')" is triggered. The value
is not used anywhere else.
Noticed when reviewing uses of the '_items' set property.
Not reported by any users, implying this setting is rarely used.
Where gethostname or tohostname returns an invalid name, e.g. containing
underscores or something, to_ascii would reject this and return nil,
which triggers an error in the dns lookup.
Reported by prova2 in the chat, for whom tohostname returned a long name
containing underscores.
Could be used to implement custom connection methods (c.f. mod_onions)
without needing to duplicate the rest of route_to_new_session().
Adds a feature to enable detection since it can be difficult to detect
support for an event otherwise.
It should not be there afterwards. Noticed that it seems to fire some
time after resumption claiming that the queue size is nil, implying
that it may hold a reference to an expired session somehow.
Error stanzas should have an <error> element, but if you pass a
stanza without one to util.error.from_stanza() it triggers an attempt to
index a nil value, which this patch avoids.
In the conditional, it should be safe to assume error_tag is non-nil
since condition can't have those values then.
There shouldn't be one here but if there is, for some reason, it's
better to close it than have it around to wake up and possibly try to
destroy the session.
The cross_domain_* settings were added here prior to http_cors_override
being added back in 17d87fb2312a, so for a time there was no
replacement, but now there is.
This event was added in a7c183bb4e64 and is required to make mod_smacks know
that a session was intentionally closed and shouldn't be hibernated (see
fcea4d9e7502).
Because this was missing from mod_websocket's session.close(), mod_smacks
would always attempt to hibernate websocket sessions even if they closed
cleanly.
That mod_websocket has its own copy of session.close() is something to fix
another day (probably not in the stable branch). So for now this commit makes
the minimal change to get things working again.
Thanks to Damian and the Jitsi team for reporting.
Kim Alvefur