Skip to content
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

Issue #12714 verify workername for Mongo usage #12715

Merged
merged 7 commits into from
Jan 22, 2025

Conversation

janbartel
Copy link
Contributor

Fixes #12714

The workerName configured on the DefaultSessionIdManager can be unacceptable to MongoDB. The DefaultSessionIdManager prepends the workerName onto the generated random id to form a session id. If this workerName contains punctuation or other delimiter type characters, this messes with the MongoDB index over the session ids.

This PR allows MongoSessionDataStore to check the workerName at startup and fail the startup if it isn't admissible.

gregw
gregw previously approved these changes Jan 15, 2025
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.. but I do have a niggle you might want to fix

@@ -159,6 +157,8 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore
*/
private DBObject _version1;

private final Pattern _workerNamePattern = Pattern.compile("[0-9a-zA-Z]*");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that it will make any difference, but:

Suggested change
private final Pattern _workerNamePattern = Pattern.compile("[0-9a-zA-Z]*");
private final static Pattern __workerNamePattern = Pattern.compile("[0-9a-zA-Z]*");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -159,6 +157,8 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore
*/
private DBObject _version1;

private final Pattern _workerNamePattern = Pattern.compile("[0-9a-zA-Z]*");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Underscore is a valid character, too.
Adding it would allow the name to support snake case.

return;

if (!_workerNamePattern.matcher(_context.getWorkerName()).matches())
throw new IllegalStateException("Invalid worker name: " + _context.getWorkerName());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm!
Only improvement might be stating what's valid, so if the issue is encountered it'd be fixable without diving into the code and looking up the Pattern

@janbartel janbartel merged commit dc685b6 into jetty-12.0.x Jan 22, 2025
10 checks passed
@janbartel janbartel deleted the jetty-12.0.x-12714-mongo-workername branch January 22, 2025 08:28
@qtxo
Copy link

qtxo commented Jan 28, 2025

Hmm maybe I am missing something but I see the index id_1 is a text index that is only used in $text: { $search: "..."} queries but the code in MongoSessionDataStore does not use it (also the .sparse(false) option is ignored for this index type).

https://www.mongodb.com/docs/manual/core/indexes/index-types/index-text/

This means the index can be replaced with a regular unique index and handle any worker name. Or even more simpler to use the _id field.

@olamy
Copy link
Member

olamy commented Jan 28, 2025

@qtxo looks right. But not related to this change. Can you please create a separate issue or pull request?
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

MongoSessionDataStore can't upsert sessions if workerName contains token deliminators
5 participants