From 2f22767dc10fbabd448054289b97f37c523d29fc Mon Sep 17 00:00:00 2001 From: Tony Copping Date: Mon, 19 Aug 2024 18:41:35 -0600 Subject: [PATCH 1/4] Issue #12175 Update SslContextFactory to use Credential instead of Password Signed-off-by: Tony Copping --- .../jetty/util/ssl/SslContextFactory.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java index 83cf59cd1d8..1351a653400 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java @@ -86,7 +86,7 @@ import org.eclipse.jetty.util.resource.ResourceFactory; import org.eclipse.jetty.util.resource.Resources; import org.eclipse.jetty.util.security.CertificateUtils; import org.eclipse.jetty.util.security.CertificateValidator; -import org.eclipse.jetty.util.security.Password; +import org.eclipse.jetty.util.security.Credential; import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -157,9 +157,9 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du private Resource _trustStoreResource; private String _trustStoreProvider; private String _trustStoreType; - private Password _keyStorePassword; - private Password _keyManagerPassword; - private Password _trustStorePassword; + private Credential _keyStorePassword; + private Credential _keyManagerPassword; + private Credential _trustStorePassword; private String _sslProvider; private String _sslProtocol = "TLS"; private String _secureRandomAlgorithm; @@ -1148,7 +1148,7 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du { String type = Objects.toString(getTrustStoreType(), getKeyStoreType()); String provider = Objects.toString(getTrustStoreProvider(), getKeyStoreProvider()); - Password passwd = _trustStorePassword; + Credential passwd = _trustStorePassword; if (resource == null || resource.equals(_keyStoreResource)) { resource = _keyStoreResource; @@ -1614,23 +1614,23 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du * Returns the password object for the given realm. * * @param realm the realm - * @return the Password object + * @return the Credential object */ - protected Password getPassword(String realm) + protected Credential getPassword(String realm) { String password = System.getProperty(realm); return password == null ? null : newPassword(password); } /** - * Creates a new Password object. + * Creates a new Credential object. * * @param password the password string - * @return the new Password object + * @return the new Credential object */ - public Password newPassword(String password) + public Credential newPassword(String password) { - return new Password(password); + return Credential.getCredential(password); } public SSLServerSocket newSslServerSocket(String host, int port, int backlog) throws IOException From 66e4bd862f038e5ac50b93f217319748210d23d3 Mon Sep 17 00:00:00 2001 From: Tony Copping Date: Sat, 24 Aug 2024 09:59:28 -0600 Subject: [PATCH 2/4] Issue #12175 Updates based on feedback Signed-off-by: Tony Copping --- .../jetty/util/ssl/SslContextFactory.java | 72 ++++++++++++------- 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java index 1351a653400..bf08c19a08f 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java @@ -87,6 +87,7 @@ import org.eclipse.jetty.util.resource.Resources; import org.eclipse.jetty.util.security.CertificateUtils; import org.eclipse.jetty.util.security.CertificateValidator; import org.eclipse.jetty.util.security.Credential; +import org.eclipse.jetty.util.security.Password; import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -157,9 +158,9 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du private Resource _trustStoreResource; private String _trustStoreProvider; private String _trustStoreType; - private Credential _keyStorePassword; - private Credential _keyManagerPassword; - private Credential _trustStorePassword; + private Credential _keyStoreCredential; + private Credential _keyManagerCredential; + private Credential _trustStoreCredential; private String _sslProvider; private String _sslProtocol = "TLS"; private String _secureRandomAlgorithm; @@ -811,46 +812,42 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du public String getKeyStorePassword() { - return _keyStorePassword == null ? null : _keyStorePassword.toString(); + return _keyStoreCredential == null ? null : _keyStoreCredential.toString(); } /** - * @param password The password for the key store. If null is passed and - * a keystore is set, then - * the {@link #getPassword(String)} is used to - * obtain a password either from the {@value #PASSWORD_PROPERTY} - * system property. + * @param password The password for the key store. If null is passed then + * {@link #getCredential(String)} is used to obtain a password from + * the {@value #PASSWORD_PROPERTY} system property. */ public void setKeyStorePassword(String password) { - _keyStorePassword = password == null ? getPassword(PASSWORD_PROPERTY) : newPassword(password); + _keyStoreCredential = password == null ? getCredential(PASSWORD_PROPERTY) : newCredential(password); } public String getKeyManagerPassword() { - return _keyManagerPassword == null ? null : _keyManagerPassword.toString(); + return _keyManagerCredential == null ? null : _keyManagerCredential.toString(); } /** * @param password The password (if any) for the specific key within the key store. - * If null is passed and the {@value #KEYPASSWORD_PROPERTY} system property is set, - * then the {@link #getPassword(String)} is used to + * If null is passed then {@link #getCredential(String)} is used to * obtain a password from the {@value #KEYPASSWORD_PROPERTY} system property. */ public void setKeyManagerPassword(String password) { - _keyManagerPassword = password == null ? getPassword(KEYPASSWORD_PROPERTY) : newPassword(password); + _keyManagerCredential = password == null ? getCredential(KEYPASSWORD_PROPERTY) : newCredential(password); } /** * @param password The password for the truststore. If null is passed then - * the {@link #getPassword(String)} is used to - * obtain a password from the {@value #PASSWORD_PROPERTY} + * {@link #getCredential(String)} is used to obtain a password from the {@value #PASSWORD_PROPERTY} * system property. */ public void setTrustStorePassword(String password) { - _trustStorePassword = password == null ? getPassword(PASSWORD_PROPERTY) : newPassword(password); + _trustStoreCredential = password == null ? getCredential(PASSWORD_PROPERTY) : newCredential(password); } /** @@ -1133,7 +1130,7 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du */ protected KeyStore loadKeyStore(Resource resource) throws Exception { - String storePassword = Objects.toString(_keyStorePassword, null); + String storePassword = Objects.toString(_keyStoreCredential, null); return CertificateUtils.getKeyStore(resource, getKeyStoreType(), getKeyStoreProvider(), storePassword); } @@ -1148,12 +1145,12 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du { String type = Objects.toString(getTrustStoreType(), getKeyStoreType()); String provider = Objects.toString(getTrustStoreProvider(), getKeyStoreProvider()); - Credential passwd = _trustStorePassword; + Credential passwd = _trustStoreCredential; if (resource == null || resource.equals(_keyStoreResource)) { resource = _keyStoreResource; if (passwd == null) - passwd = _keyStorePassword; + passwd = _keyStoreCredential; } return CertificateUtils.getKeyStore(resource, type, provider, Objects.toString(passwd, null)); } @@ -1180,7 +1177,7 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du if (keyStore != null) { KeyManagerFactory keyManagerFactory = getKeyManagerFactoryInstance(); - keyManagerFactory.init(keyStore, _keyManagerPassword == null ? (_keyStorePassword == null ? null : _keyStorePassword.toString().toCharArray()) : _keyManagerPassword.toString().toCharArray()); + keyManagerFactory.init(keyStore, _keyManagerCredential == null ? (_keyStoreCredential == null ? null : _keyStoreCredential.toString().toCharArray()) : _keyManagerCredential.toString().toCharArray()); managers = keyManagerFactory.getKeyManagers(); if (managers != null) @@ -1614,21 +1611,48 @@ public abstract class SslContextFactory extends ContainerLifeCycle implements Du * Returns the password object for the given realm. * * @param realm the realm - * @return the Credential object + * @return the Password object + * @deprecated use {#link getCredential} instead. */ - protected Credential getPassword(String realm) + @Deprecated(since = "12.0.13", forRemoval = true) + protected Password getPassword(String realm) { String password = System.getProperty(realm); return password == null ? null : newPassword(password); } + /** + * Creates a new Password object. + * + * @param password the password string + * @return the new Password object + * @deprecated use {#link newCredential} instead. + */ + @Deprecated(since = "12.0.13", forRemoval = true) + public Password newPassword(String password) + { + return new Password(password); + } + + /** + * Returns the credential object for the given realm. + * + * @param realm the realm + * @return the Credential object + */ + protected Credential getCredential(String realm) + { + String password = System.getProperty(realm); + return password == null ? null : newCredential(password); + } + /** * Creates a new Credential object. * * @param password the password string * @return the new Credential object */ - public Credential newPassword(String password) + public Credential newCredential(String password) { return Credential.getCredential(password); } From edbd21bdf08143ab57f66184cedf3e4c93015d70 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Wed, 28 Aug 2024 18:09:15 +1000 Subject: [PATCH 3/4] Issue #11322 Upgrade to mongodb-driver-sync 5.x (#11681) Upgrade to mongodb driver async 5.x --------- Signed-off-by: Olivier Lamy --- jetty-integrations/jetty-nosql/pom.xml | 13 +- .../config/modules/session-store-mongo.mod | 8 +- .../src/main/java/module-info.java | 4 +- .../jetty/nosql/NoSqlSessionDataStore.java | 2 +- .../nosql/mongodb/MongoSessionDataStore.java | 202 ++++++++---------- .../mongodb/MongoSessionDataStoreFactory.java | 14 +- .../jetty/nosql/mongodb/MongoUtils.java | 26 ++- pom.xml | 9 +- tests/jetty-test-session-common/pom.xml | 2 +- .../session/test/tools/MongoTestHelper.java | 88 ++++---- .../MongodbSessionDistributionTests.java | 2 +- 11 files changed, 190 insertions(+), 180 deletions(-) diff --git a/jetty-integrations/jetty-nosql/pom.xml b/jetty-integrations/jetty-nosql/pom.xml index cef8dfdbd2d..a09cc643cea 100644 --- a/jetty-integrations/jetty-nosql/pom.xml +++ b/jetty-integrations/jetty-nosql/pom.xml @@ -23,10 +23,21 @@ org.mongodb - mongo-java-driver + bson ${mongodb.version} compile + + org.mongodb + mongodb-driver-core + ${mongodb.version} + compile + + + org.mongodb + mongodb-driver-sync + compile + org.slf4j slf4j-api diff --git a/jetty-integrations/jetty-nosql/src/main/config/modules/session-store-mongo.mod b/jetty-integrations/jetty-nosql/src/main/config/modules/session-store-mongo.mod index ea6e8f7b1d9..7d8123f936e 100644 --- a/jetty-integrations/jetty-nosql/src/main/config/modules/session-store-mongo.mod +++ b/jetty-integrations/jetty-nosql/src/main/config/modules/session-store-mongo.mod @@ -14,11 +14,15 @@ sessions sessions/mongo/${connection-type} [files] -maven://org.mongodb/mongo-java-driver/${mongodb.version}|lib/nosql/mongo-java-driver-${mongodb.version}.jar +maven://org.mongodb/mongodb-driver-sync/${mongodb.version}|lib/nosql/mongodb-driver-sync-${mongodb.version}.jar +maven://org.mongodb/mongodb-driver-core/${mongodb.version}|lib/nosql/mongodb-driver-core-${mongodb.version}.jar +maven://org.mongodb/bson/${mongodb.version}|lib/nosql/bson-${mongodb.version}.jar [lib] lib/jetty-nosql-${jetty.version}.jar -lib/nosql/mongo-java-driver-${mongodb.version}.jar +lib/nosql/mongodb-driver-sync-${mongodb.version}.jar +lib/nosql/mongodb-driver-core-${mongodb.version}.jar +lib/nosql/bson-${mongodb.version}.jar [license] The java driver for the MongoDB document-based database system is hosted on GitHub and released under the Apache 2.0 license. diff --git a/jetty-integrations/jetty-nosql/src/main/java/module-info.java b/jetty-integrations/jetty-nosql/src/main/java/module-info.java index 2ef01b87a2b..0729e1c4a5d 100644 --- a/jetty-integrations/jetty-nosql/src/main/java/module-info.java +++ b/jetty-integrations/jetty-nosql/src/main/java/module-info.java @@ -13,7 +13,9 @@ module org.eclipse.jetty.nosql { - requires transitive mongo.java.driver; + requires transitive org.mongodb.driver.core; + requires transitive org.mongodb.driver.sync.client; + requires transitive org.mongodb.bson; requires transitive org.eclipse.jetty.session; exports org.eclipse.jetty.nosql; diff --git a/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/NoSqlSessionDataStore.java b/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/NoSqlSessionDataStore.java index 211812c75ba..8e92f1b5945 100644 --- a/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/NoSqlSessionDataStore.java +++ b/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/NoSqlSessionDataStore.java @@ -62,7 +62,7 @@ public abstract class NoSqlSessionDataStore extends ObjectStreamSessionDataStore public Set getAllAttributeNames() { - return new HashSet(_attributes.keySet()); + return new HashSet<>(_attributes.keySet()); } } diff --git a/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java b/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java index 3395f9bb1f7..9d00ef0cdc2 100644 --- a/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java +++ b/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java @@ -15,22 +15,29 @@ package org.eclipse.jetty.nosql.mongodb; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.util.HashMap; -import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; -import com.mongodb.BasicDBList; import com.mongodb.BasicDBObject; import com.mongodb.BasicDBObjectBuilder; -import com.mongodb.DBCollection; -import com.mongodb.DBCursor; import com.mongodb.DBObject; import com.mongodb.MongoException; -import com.mongodb.WriteConcern; -import com.mongodb.WriteResult; +import com.mongodb.client.FindIterable; +import com.mongodb.client.MongoCollection; +import com.mongodb.client.model.Filters; +import com.mongodb.client.model.IndexModel; +import com.mongodb.client.model.IndexOptions; +import com.mongodb.client.model.Indexes; +import com.mongodb.client.model.Projections; +import com.mongodb.client.model.UpdateOptions; +import com.mongodb.client.result.UpdateResult; +import org.bson.Document; +import org.bson.conversions.Bson; +import org.bson.types.Binary; import org.eclipse.jetty.nosql.NoSqlSessionDataStore; import org.eclipse.jetty.session.SessionContext; import org.eclipse.jetty.session.SessionData; @@ -155,15 +162,15 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore /** * Access to MongoDB */ - private DBCollection _dbSessions; + private MongoCollection _dbSessions; - public void setDBCollection(DBCollection collection) + public void setDBCollection(MongoCollection collection) { _dbSessions = collection; } @ManagedAttribute(value = "DBCollection", readonly = true) - public DBCollection getDBCollection() + public MongoCollection getDBCollection() { return _dbSessions; } @@ -171,7 +178,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore @Override public SessionData doLoad(String id) throws Exception { - DBObject sessionDocument = _dbSessions.findOne(new BasicDBObject(__ID, id)); + Document sessionDocument = _dbSessions.find(Filters.eq(__ID, id)).first(); try { @@ -191,7 +198,8 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore Object version = MongoUtils.getNestedValue(sessionDocument, getContextSubfield(__VERSION)); Long lastSaved = (Long)MongoUtils.getNestedValue(sessionDocument, getContextSubfield(__LASTSAVED)); String lastNode = (String)MongoUtils.getNestedValue(sessionDocument, getContextSubfield(__LASTNODE)); - byte[] attributes = (byte[])MongoUtils.getNestedValue(sessionDocument, getContextSubfield(__ATTRIBUTES)); + Binary binary = ((Binary)MongoUtils.getNestedValue(sessionDocument, getContextSubfield(__ATTRIBUTES))); + byte[] attributes = binary == null ? null : binary.getData(); Long created = (Long)sessionDocument.get(__CREATED); Long accessed = (Long)sessionDocument.get(__ACCESSED); @@ -202,7 +210,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore NoSqlSessionData data = null; // get the session for the context - DBObject sessionSubDocumentForContext = (DBObject)MongoUtils.getNestedValue(sessionDocument, getContextField()); + Document sessionSubDocumentForContext = (Document)MongoUtils.getNestedValue(sessionDocument, getContextField()); if (LOG.isDebugEnabled()) LOG.debug("attrs {}", sessionSubDocumentForContext); @@ -239,9 +247,9 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore else { //attributes have special serialized format - try (ByteArrayInputStream bais = new ByteArrayInputStream(attributes);) + try (ByteArrayInputStream bais = new ByteArrayInputStream(attributes)) { - deserializeAttributes(data, bais); + deserializeAttributes(data, bais); } } } @@ -269,17 +277,17 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore * Check if the session exists and if it does remove the context * associated with this session */ - BasicDBObject mongoKey = new BasicDBObject(__ID, id); + Bson filterId = Filters.eq(__ID, id); - DBObject sessionDocument = _dbSessions.findOne(new BasicDBObject(__ID, id)); + Document sessionDocument = _dbSessions.find(filterId).first(); if (sessionDocument != null) { - DBObject c = (DBObject)MongoUtils.getNestedValue(sessionDocument, __CONTEXT); + Document c = (Document)MongoUtils.getNestedValue(sessionDocument, __CONTEXT); if (c == null) { //delete whole doc - _dbSessions.remove(mongoKey, WriteConcern.SAFE); + _dbSessions.deleteOne(filterId); return false; } @@ -287,14 +295,14 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore if (contexts.isEmpty()) { //delete whole doc - _dbSessions.remove(mongoKey, WriteConcern.SAFE); + _dbSessions.deleteOne(filterId); return false; } if (contexts.size() == 1 && contexts.iterator().next().equals(getCanonicalContextId())) { //delete whole doc - _dbSessions.remove(new BasicDBObject(__ID, id), WriteConcern.SAFE); + _dbSessions.deleteOne(filterId); return true; } @@ -303,7 +311,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore BasicDBObject unsets = new BasicDBObject(); unsets.put(getContextField(), 1); remove.put("$unset", unsets); - _dbSessions.update(mongoKey, remove, false, false, WriteConcern.SAFE); + _dbSessions.updateOne(filterId, remove); return true; } else @@ -315,12 +323,9 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore @Override public boolean doExists(String id) throws Exception { - DBObject fields = new BasicDBObject(); - fields.put(__EXPIRY, 1); - fields.put(__VALID, 1); - fields.put(getContextSubfield(__VERSION), 1); - - DBObject sessionDocument = _dbSessions.findOne(new BasicDBObject(__ID, id), fields); + Bson projection = Projections.fields(Projections.include(__ID, __VALID, __EXPIRY, __VERSION, getContextField()), Projections.excludeId()); + Bson filterId = Filters.eq(__ID, id); + Document sessionDocument = _dbSessions.find(filterId).projection(projection).first(); if (sessionDocument == null) return false; //doesn't exist @@ -332,48 +337,33 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore Long expiry = (Long)sessionDocument.get(__EXPIRY); //expired? - if (expiry.longValue() > 0 && expiry.longValue() < System.currentTimeMillis()) + if (expiry != null && expiry > 0 && expiry < System.currentTimeMillis()) return false; //it's expired //does it exist for this context? Object version = MongoUtils.getNestedValue(sessionDocument, getContextSubfield(__VERSION)); - if (version == null) - return false; - - return true; + return version != null; } @Override public Set doCheckExpired(Set candidates, long time) { - Set expiredSessions = new HashSet<>(); //firstly ask mongo to verify if these candidate ids have expired - all of //these candidates will be for our node - BasicDBObject query = new BasicDBObject(); - query.append(__ID, new BasicDBObject("$in", candidates)); - query.append(__EXPIRY, new BasicDBObject("$gt", 0).append("$lte", time)); - - DBCursor verifiedExpiredSessions = null; - try - { - verifiedExpiredSessions = _dbSessions.find(query, new BasicDBObject(__ID, 1)); - for (DBObject session : verifiedExpiredSessions) - { - String id = (String)session.get(__ID); - if (LOG.isDebugEnabled()) - LOG.debug("{} Mongo confirmed expired session {}", _context, id); - expiredSessions.add(id); - } - } - finally - { - if (verifiedExpiredSessions != null) - verifiedExpiredSessions.close(); - } - + Bson query = Filters.and( + Filters.in(__ID, candidates), + Filters.gt(__EXPIRY, 0), + Filters.lte(__EXPIRY, time)); - //check through sessions that were candidates, but not found as expired. + + FindIterable verifiedExpiredSessions = _dbSessions.find(query); // , new BasicDBObject(__ID, 1) + Set expiredSessions = + StreamSupport.stream(verifiedExpiredSessions.spliterator(), false) + .map(document -> document.getString(__ID)) + .collect(Collectors.toSet()); + + //check through sessions that were candidates, but not found as expired. //they may no longer be persisted, in which case they are treated as expired. for (String c:candidates) { @@ -398,37 +388,17 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore { // now ask mongo to find sessions for this context, last managed by any // node, that expired before timeLimit - Set expiredSessions = new HashSet<>(); + Bson query = Filters.and( + Filters.gt(__EXPIRY, 0), + Filters.lte(__EXPIRY, timeLimit) + ); - BasicDBObject query = new BasicDBObject(); - BasicDBObject gt = new BasicDBObject(__EXPIRY, new BasicDBObject("$gt", 0)); - BasicDBObject lt = new BasicDBObject(__EXPIRY, new BasicDBObject("$lte", timeLimit)); - BasicDBList list = new BasicDBList(); - list.add(gt); - list.add(lt); - query.append("$and", list); - - DBCursor oldExpiredSessions = null; - try - { - BasicDBObject bo = new BasicDBObject(__ID, 1); - bo.append(__EXPIRY, 1); - - oldExpiredSessions = _dbSessions.find(query, bo); - for (DBObject session : oldExpiredSessions) - { - String id = (String)session.get(__ID); - - //TODO we should verify if there is a session for my context, not any context - expiredSessions.add(id); - } - } - finally - { - if (oldExpiredSessions != null) - oldExpiredSessions.close(); - } + //TODO we should verify if there is a session for my context, not any context + FindIterable documents = _dbSessions.find(query); + Set expiredSessions = StreamSupport.stream(documents.spliterator(), false) + .map(document -> document.getString(__ID)) + .collect(Collectors.toSet()); return expiredSessions; } @@ -438,9 +408,11 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore //Delete all session documents where the expiry time (which is always the most //up-to-date expiry of all contexts sharing that session id) has already past as //at the timeLimit. - BasicDBObject query = new BasicDBObject(); - query.append(__EXPIRY, new BasicDBObject("$gt", 0).append("$lte", timeLimit)); - _dbSessions.remove(query, WriteConcern.SAFE); + Bson query = Filters.and( + Filters.gt(__EXPIRY, 0), + Filters.lte(__EXPIRY, timeLimit) + ); + _dbSessions.deleteMany(query); } /** @@ -458,8 +430,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore public void doStore(String id, SessionData data, long lastSaveTime) throws Exception { // Form query for upsert - final BasicDBObject key = new BasicDBObject(__ID, id); - + Bson key = Filters.eq(__ID, id);; // Form updates BasicDBObject update = new BasicDBObject(); boolean upsert = false; @@ -487,12 +458,13 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore sets.put(getContextSubfield(__LASTNODE), data.getLastNode()); version = ((Number)version).longValue() + 1L; ((NoSqlSessionData)data).setVersion(version); - update.put("$inc", _version1); + // what is this?? this field is used no where... + //sets.put("$inc", _version1); //if max idle time and/or expiry is smaller for this context, then choose that for the whole session doc BasicDBObject fields = new BasicDBObject(); fields.append(__MAX_IDLE, true); fields.append(__EXPIRY, true); - DBObject o = _dbSessions.findOne(new BasicDBObject("id", id), fields); + Document o = _dbSessions.find(key).first(); if (o != null) { Long tmpLong = (Long)o.get(__MAX_IDLE); @@ -516,37 +488,39 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore try (ByteArrayOutputStream baos = new ByteArrayOutputStream();) { serializeAttributes(data, baos); - sets.put(getContextSubfield(__ATTRIBUTES), baos.toByteArray()); + Binary binary = new Binary(baos.toByteArray()); + sets.put(getContextSubfield(__ATTRIBUTES), binary); } // Do the upsert if (!sets.isEmpty()) update.put("$set", sets); - WriteResult res = _dbSessions.update(key, update, upsert, false, WriteConcern.SAFE); + UpdateResult res = _dbSessions.updateOne(key, update, new UpdateOptions().upsert(upsert)); if (LOG.isDebugEnabled()) LOG.debug("Save:db.sessions.update( {}, {},{} )", key, update, res); } protected void ensureIndexes() throws MongoException { - _version1 = new BasicDBObject(getContextSubfield(__VERSION), 1); - DBObject idKey = BasicDBObjectBuilder.start().add("id", 1).get(); - _dbSessions.createIndex(idKey, - BasicDBObjectBuilder.start() - .add("name", "id_1") - .add("ns", _dbSessions.getFullName()) - .add("sparse", false) - .add("unique", true) - .get()); - - DBObject versionKey = BasicDBObjectBuilder.start().add("id", 1).add("version", 1).get(); - _dbSessions.createIndex(versionKey, BasicDBObjectBuilder.start() - .add("name", "id_1_version_1") - .add("ns", _dbSessions.getFullName()) - .add("sparse", false) - .add("unique", true) - .get()); + var indexes = + StreamSupport.stream(_dbSessions.listIndexes().spliterator(), false) + .toList(); + var indexesNames = indexes.stream().map(document -> document.getString("name")).toList(); + if (!indexesNames.contains("id_1")) + { + String createResult = _dbSessions.createIndex(Indexes.text("id"), + new IndexOptions().unique(true).name("id_1").sparse(false)); + LOG.info("create index {}, result: {}", "id_1", createResult); + } + if (!indexesNames.contains("id_1_version_1")) + { + // Command failed with error 67 (CannotCreateIndex): 'only one text index per collection allowed, found existing text index "id_1"' + String createResult = _dbSessions.createIndex( + Indexes.compoundIndex(Indexes.descending("id"), Indexes.descending("version")), + new IndexOptions().unique(false).name("id_1_version_1").sparse(false)); + LOG.info("create index {}, result: {}", "id_1_version_1", createResult); + } if (LOG.isDebugEnabled()) LOG.debug("Done ensure Mongodb indexes existing"); //TODO perhaps index on expiry time? @@ -587,4 +561,4 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore { return String.format("%s[collection=%s]", super.toString(), getDBCollection()); } -} +} \ No newline at end of file diff --git a/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreFactory.java b/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreFactory.java index 14ee7fbe9c8..f1913d9aff2 100644 --- a/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreFactory.java +++ b/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreFactory.java @@ -15,8 +15,8 @@ package org.eclipse.jetty.nosql.mongodb; import java.net.UnknownHostException; -import com.mongodb.MongoClient; -import com.mongodb.MongoClientURI; +import com.mongodb.client.MongoClient; +import com.mongodb.client.MongoClients; import org.eclipse.jetty.session.AbstractSessionDataStoreFactory; import org.eclipse.jetty.session.SessionDataStore; import org.eclipse.jetty.session.SessionManager; @@ -136,14 +136,14 @@ public class MongoSessionDataStoreFactory extends AbstractSessionDataStoreFactor MongoClient mongo; if (!StringUtil.isBlank(getConnectionString())) - mongo = new MongoClient(new MongoClientURI(getConnectionString())); + mongo = MongoClients.create(getConnectionString()); else if (!StringUtil.isBlank(getHost()) && getPort() != -1) - mongo = new MongoClient(getHost(), getPort()); + mongo = MongoClients.create("mongodb://" + getHost() + ":" + getPort()); else if (!StringUtil.isBlank(getHost())) - mongo = new MongoClient(getHost()); + mongo = MongoClients.create("mongodb://" + getHost()); else - mongo = new MongoClient(); - store.setDBCollection(mongo.getDB(getDbName()).getCollection(getCollectionName())); + mongo = MongoClients.create(); + store.setDBCollection(mongo.getDatabase(getDbName()).getCollection(getCollectionName())); return store; } } diff --git a/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java b/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java index 9d6ecd99b7b..285d0444ddd 100644 --- a/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java +++ b/jetty-integrations/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java @@ -22,7 +22,8 @@ import java.util.HashMap; import java.util.Map; import com.mongodb.BasicDBObject; -import com.mongodb.DBObject; +import org.bson.Document; +import org.bson.types.Binary; import org.eclipse.jetty.util.ClassLoadingObjectInputStream; import org.eclipse.jetty.util.URIUtil; @@ -40,6 +41,13 @@ public class MongoUtils { return valueToDecode; } + else if (valueToDecode instanceof Binary) + { + final byte[] decodeObject = ((Binary)valueToDecode).getData(); + final ByteArrayInputStream bais = new ByteArrayInputStream(decodeObject); + final ClassLoadingObjectInputStream objectInputStream = new ClassLoadingObjectInputStream(bais); + return objectInputStream.readUnshared(); + } else if (valueToDecode instanceof byte[]) { final byte[] decodeObject = (byte[])valueToDecode; @@ -47,13 +55,13 @@ public class MongoUtils final ClassLoadingObjectInputStream objectInputStream = new ClassLoadingObjectInputStream(bais); return objectInputStream.readUnshared(); } - else if (valueToDecode instanceof DBObject) + else if (valueToDecode instanceof Document) { - Map map = new HashMap(); - for (String name : ((DBObject)valueToDecode).keySet()) + Map map = new HashMap<>(); + for (String name : ((Document)valueToDecode).keySet()) { String attr = decodeName(name); - map.put(attr, decodeValue(((DBObject)valueToDecode).get(name))); + map.put(attr, decodeValue(((Document)valueToDecode).get(name))); } return map; } @@ -107,19 +115,19 @@ public class MongoUtils /** * Dig through a given dbObject for the nested value * - * @param dbObject the mongo object to search + * @param sessionDocument the mongo document to search * @param nestedKey the field key to find * @return the value of the field key */ - public static Object getNestedValue(DBObject dbObject, String nestedKey) + public static Object getNestedValue(Document sessionDocument, String nestedKey) { String[] keyChain = nestedKey.split("\\."); - DBObject temp = dbObject; + Document temp = sessionDocument; for (int i = 0; i < keyChain.length - 1; ++i) { - temp = (DBObject)temp.get(keyChain[i]); + temp = (Document)temp.get(keyChain[i]); if (temp == null) { diff --git a/pom.xml b/pom.xml index 0cce8d9092a..3124b3913b9 100644 --- a/pom.xml +++ b/pom.xml @@ -277,8 +277,8 @@ 3.9.0 3.4.0 2.2.3 - 3.2.20 - 3.12.14 + 5.0.26 + 5.1.3 4.1.109.Final 0.9.1 8.1.0 @@ -1074,6 +1074,11 @@ mariadb-java-client ${mariadb.version} + + org.mongodb + mongodb-driver-sync + ${mongodb.version} + org.mortbay.jetty.quiche jetty-quiche-native diff --git a/tests/jetty-test-session-common/pom.xml b/tests/jetty-test-session-common/pom.xml index 6d9cf6b3d36..11c0d8675ad 100644 --- a/tests/jetty-test-session-common/pom.xml +++ b/tests/jetty-test-session-common/pom.xml @@ -101,7 +101,7 @@ org.mongodb - mongo-java-driver + mongodb-driver-sync ${mongodb.version} compile diff --git a/tests/jetty-test-session-common/src/main/java/org/eclipse/jetty/session/test/tools/MongoTestHelper.java b/tests/jetty-test-session-common/src/main/java/org/eclipse/jetty/session/test/tools/MongoTestHelper.java index 525859be766..5c3d16d4a65 100644 --- a/tests/jetty-test-session-common/src/main/java/org/eclipse/jetty/session/test/tools/MongoTestHelper.java +++ b/tests/jetty-test-session-common/src/main/java/org/eclipse/jetty/session/test/tools/MongoTestHelper.java @@ -18,13 +18,20 @@ import java.io.ByteArrayOutputStream; import java.io.ObjectOutputStream; import java.net.UnknownHostException; import java.util.Map; +import java.util.stream.StreamSupport; import com.mongodb.BasicDBObject; -import com.mongodb.DBCollection; import com.mongodb.DBObject; -import com.mongodb.MongoClient; import com.mongodb.MongoException; import com.mongodb.WriteConcern; +import com.mongodb.client.MongoClient; +import com.mongodb.client.MongoClients; +import com.mongodb.client.MongoCollection; +import com.mongodb.client.model.CreateCollectionOptions; +import com.mongodb.client.model.Filters; +import com.mongodb.client.model.UpdateOptions; +import org.bson.Document; +import org.bson.types.Binary; import org.eclipse.jetty.nosql.mongodb.MongoSessionDataStore; import org.eclipse.jetty.nosql.mongodb.MongoSessionDataStoreFactory; import org.eclipse.jetty.nosql.mongodb.MongoUtils; @@ -57,7 +64,7 @@ public class MongoTestHelper static { - mongo = new MongoDBContainer(DockerImageName.parse("mongo:" + System.getProperty("mongo.docker.version", "3.2.20"))) + mongo = new MongoDBContainer(DockerImageName.parse("mongo:" + System.getProperty("mongo.docker.version", "5.0.26"))) .withLogConsumer(new Slf4jLogConsumer(MONGO_LOG)); long start = System.currentTimeMillis(); mongo.start(); @@ -65,21 +72,21 @@ public class MongoTestHelper mongoPort = mongo.getMappedPort(MONGO_PORT); LOG.info("Mongo container started for {}:{} - {}ms", mongoHost, mongoPort, System.currentTimeMillis() - start); - mongoClient = new MongoClient(mongoHost, mongoPort); + mongoClient = MongoClients.create(mongo.getConnectionString()); } public static MongoClient getMongoClient() throws UnknownHostException { if (mongoClient == null) { - mongoClient = new MongoClient(mongoHost, mongoPort); + mongoClient = MongoClients.create(mongo.getConnectionString()); } return mongoClient; } public static void dropCollection(String dbName, String collectionName) throws Exception { - getMongoClient().getDB(dbName).getCollection(collectionName).drop(); + getMongoClient().getDatabase(dbName).getCollection(collectionName).withWriteConcern(WriteConcern.JOURNALED).drop(); } public static void shutdown() throws Exception @@ -89,12 +96,14 @@ public class MongoTestHelper public static void createCollection(String dbName, String collectionName) throws UnknownHostException, MongoException { - getMongoClient().getDB(dbName).createCollection(collectionName, null); + if (StreamSupport.stream(getMongoClient().getDatabase(dbName).listCollectionNames().spliterator(), false) + .filter(collectionName::equals).findAny().isEmpty()) + getMongoClient().getDatabase(dbName).withWriteConcern(WriteConcern.JOURNALED).createCollection(collectionName, new CreateCollectionOptions()); } - public static DBCollection getCollection(String dbName, String collectionName) throws UnknownHostException, MongoException + public static MongoCollection getCollection(String dbName, String collectionName) throws UnknownHostException, MongoException { - return getMongoClient().getDB(dbName).getCollection(collectionName); + return getMongoClient().getDatabase(dbName).getCollection(collectionName); } public static MongoSessionDataStoreFactory newSessionDataStoreFactory(String dbName, String collectionName) @@ -108,15 +117,15 @@ public class MongoTestHelper } public static boolean checkSessionExists(String id, String dbName, String collectionName) - throws Exception + throws Exception { - DBCollection collection = getMongoClient().getDB(dbName).getCollection(collectionName); + MongoCollection collection = getMongoClient().getDatabase(dbName).getCollection(collectionName); DBObject fields = new BasicDBObject(); fields.put(MongoSessionDataStore.__EXPIRY, 1); fields.put(MongoSessionDataStore.__VALID, 1); - DBObject sessionDocument = collection.findOne(new BasicDBObject(MongoSessionDataStore.__ID, id), fields); + Document sessionDocument = collection.find(Filters.eq(MongoSessionDataStore.__ID, id)).first(); if (sessionDocument == null) return false; //doesn't exist @@ -125,21 +134,21 @@ public class MongoTestHelper } public static boolean checkSessionPersisted(SessionData data, String dbName, String collectionName) - throws Exception + throws Exception { - DBCollection collection = getMongoClient().getDB(dbName).getCollection(collectionName); + MongoCollection collection = getMongoClient().getDatabase(dbName).getCollection(collectionName); DBObject fields = new BasicDBObject(); - DBObject sessionDocument = collection.findOne(new BasicDBObject(MongoSessionDataStore.__ID, data.getId()), fields); + Document sessionDocument = collection.find(Filters.eq(MongoSessionDataStore.__ID, data.getId())).first(); if (sessionDocument == null) return false; //doesn't exist LOG.debug("{}", sessionDocument); - Boolean valid = (Boolean)sessionDocument.get(MongoSessionDataStore.__VALID); + boolean valid = (Boolean)sessionDocument.get(MongoSessionDataStore.__VALID); - if (valid == null || !valid) + if (!valid) return false; Long created = (Long)sessionDocument.get(MongoSessionDataStore.__CREATED); @@ -149,13 +158,13 @@ public class MongoTestHelper Long expiry = (Long)sessionDocument.get(MongoSessionDataStore.__EXPIRY); Object version = MongoUtils.getNestedValue(sessionDocument, - MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath() + "." + MongoSessionDataStore.__VERSION); + MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath() + "." + MongoSessionDataStore.__VERSION); Long lastSaved = (Long)MongoUtils.getNestedValue(sessionDocument, - MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath() + "." + MongoSessionDataStore.__LASTSAVED); + MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath() + "." + MongoSessionDataStore.__LASTSAVED); String lastNode = (String)MongoUtils.getNestedValue(sessionDocument, - MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath() + "." + MongoSessionDataStore.__LASTNODE); - byte[] attributes = (byte[])MongoUtils.getNestedValue(sessionDocument, - MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath() + "." + MongoSessionDataStore.__ATTRIBUTES); + MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath() + "." + MongoSessionDataStore.__LASTNODE); + byte[] attributes = ((Binary)MongoUtils.getNestedValue(sessionDocument, + MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath() + "." + MongoSessionDataStore.__ATTRIBUTES)).getData(); assertEquals(data.getCreated(), created.longValue()); assertEquals(data.getAccessed(), accessed.longValue()); @@ -167,9 +176,9 @@ public class MongoTestHelper assertNotNull(lastSaved); // get the session for the context - DBObject sessionSubDocumentForContext = - (DBObject)MongoUtils.getNestedValue(sessionDocument, - MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath()); + Document sessionSubDocumentForContext = + (Document)MongoUtils.getNestedValue(sessionDocument, + MongoSessionDataStore.__CONTEXT + "." + data.getVhost().replace('.', '_') + ":" + data.getContextPath()); assertNotNull(sessionSubDocumentForContext); @@ -200,12 +209,9 @@ public class MongoTestHelper long lastAccessed, long maxIdle, long expiry, Map attributes, String dbName, String collectionName) - throws Exception + throws Exception { - DBCollection collection = getMongoClient().getDB(dbName).getCollection(collectionName); - - // Form query for upsert - BasicDBObject key = new BasicDBObject(MongoSessionDataStore.__ID, id); + MongoCollection collection = getMongoClient().getDatabase(dbName).getCollection(collectionName); // Form updates BasicDBObject update = new BasicDBObject(); @@ -236,12 +242,12 @@ public class MongoTestHelper ObjectOutputStream oos = new ObjectOutputStream(baos)) { SessionData.serializeAttributes(tmp, oos); - sets.put(MongoSessionDataStore.__CONTEXT + "." + vhost.replace('.', '_') + ":" + contextPath + "." + MongoSessionDataStore.__ATTRIBUTES, baos.toByteArray()); + sets.put(MongoSessionDataStore.__CONTEXT + "." + vhost.replace('.', '_') + ":" + contextPath + "." + MongoSessionDataStore.__ATTRIBUTES, new Binary(baos.toByteArray())); } } update.put("$set", sets); - collection.update(key, update, upsert, false, WriteConcern.SAFE); + collection.updateOne(Filters.eq(MongoSessionDataStore.__ID, id), update, new UpdateOptions().upsert(true)); } public static void createSession(String id, String contextPath, String vhost, @@ -249,10 +255,10 @@ public class MongoTestHelper long lastAccessed, long maxIdle, long expiry, Map attributes, String dbName, String collectionName) - throws Exception + throws Exception { - DBCollection collection = getMongoClient().getDB(dbName).getCollection(collectionName); + MongoCollection collection = getMongoClient().getDatabase(dbName).getCollection(collectionName); // Form query for upsert BasicDBObject key = new BasicDBObject(MongoSessionDataStore.__ID, id); @@ -283,12 +289,12 @@ public class MongoTestHelper ObjectOutputStream oos = new ObjectOutputStream(baos)) { SessionData.serializeAttributes(tmp, oos); - sets.put(MongoSessionDataStore.__CONTEXT + "." + vhost.replace('.', '_') + ":" + contextPath + "." + MongoSessionDataStore.__ATTRIBUTES, baos.toByteArray()); + sets.put(MongoSessionDataStore.__CONTEXT + "." + vhost.replace('.', '_') + ":" + contextPath + "." + MongoSessionDataStore.__ATTRIBUTES, new Binary(baos.toByteArray())); } } update.put("$set", sets); - collection.update(key, update, upsert, false, WriteConcern.SAFE); + collection.updateOne(key, update, new UpdateOptions().upsert(true)); } public static void createLegacySession(String id, String contextPath, String vhost, @@ -296,10 +302,10 @@ public class MongoTestHelper long lastAccessed, long maxIdle, long expiry, Map attributes, String dbName, String collectionName) - throws Exception + throws Exception { //make old-style session to test if we can retrieve it - DBCollection collection = getMongoClient().getDB(dbName).getCollection(collectionName); + MongoCollection collection = getMongoClient().getDatabase(dbName).getCollection(collectionName); // Form query for upsert BasicDBObject key = new BasicDBObject(MongoSessionDataStore.__ID, id); @@ -329,10 +335,10 @@ public class MongoTestHelper { Object value = attributes.get(name); sets.put(MongoSessionDataStore.__CONTEXT + "." + vhost.replace('.', '_') + ":" + contextPath + "." + MongoUtils.encodeName(name), - MongoUtils.encodeName(value)); + MongoUtils.encodeName(value)); } } update.put("$set", sets); - collection.update(key, update, upsert, false, WriteConcern.SAFE); + collection.updateOne(key, update, new UpdateOptions().upsert(true)); } -} +} \ No newline at end of file diff --git a/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/session/MongodbSessionDistributionTests.java b/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/session/MongodbSessionDistributionTests.java index a4c0a17dbf7..7c54f9dd726 100644 --- a/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/session/MongodbSessionDistributionTests.java +++ b/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/session/MongodbSessionDistributionTests.java @@ -35,7 +35,7 @@ public class MongodbSessionDistributionTests extends AbstractSessionDistribution private static final int MONGO_PORT = 27017; - final String imageName = "mongo:" + System.getProperty("mongo.docker.version", "3.2.20"); + final String imageName = "mongo:" + System.getProperty("mongo.docker.version", "5.0.26"); final MongoDBContainer mongoDBContainer = new MongoDBContainer(DockerImageName.parse(imageName)) From aa07995f373a82441373d2c978f85209461e8eeb Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 28 Aug 2024 20:24:10 +1000 Subject: [PATCH 4/4] sendError(-1) is an abort (#12206) restore behaviour from jetty <= 11 where a sendError(-1) is a true abort, without an attempt to send an error response. --- .../org/eclipse/jetty/server/Request.java | 30 +++ .../server/internal/HttpChannelState.java | 18 +- .../ee10/servlet/ServletApiResponse.java | 2 +- .../jetty/ee10/servlet/ErrorPageTest.java | 220 ++++++++++++++++++ .../eclipse/jetty/ee9/nested/Response.java | 2 +- .../jetty/ee9/servlet/ErrorPageTest.java | 46 +++- 6 files changed, 305 insertions(+), 13 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index d1ad6b93ca3..84fbc3cd238 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -47,6 +47,7 @@ import org.eclipse.jetty.http.MultiPartCompliance; import org.eclipse.jetty.http.MultiPartConfig; import org.eclipse.jetty.http.Trailers; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.server.internal.CompletionStreamWrapper; import org.eclipse.jetty.server.internal.HttpChannelState; import org.eclipse.jetty.util.Attributes; @@ -716,6 +717,7 @@ public interface Request extends Attributes, Content.Source * is returned, then this method must not generate a response, nor complete the callback. * @throws Exception if there is a failure during the handling. Catchers cannot assume that the callback will be * called and thus should attempt to complete the request as if a false had been returned. + * @see AbortException */ boolean handle(Request request, Response response, Callback callback) throws Exception; @@ -725,6 +727,34 @@ public interface Request extends Attributes, Content.Source { return InvocationType.BLOCKING; } + + /** + * A marker {@link Exception} that can be passed the {@link Callback#failed(Throwable)} of the {@link Callback} + * passed in {@link #handle(Request, Response, Callback)}, to cause request handling to be aborted. For HTTP/1 + * an abort is handled with a {@link EndPoint#close()}, for later versions of HTTP, a reset message will be sent. + */ + class AbortException extends Exception + { + public AbortException() + { + super(); + } + + public AbortException(String message) + { + super(message); + } + + public AbortException(String message, Throwable cause) + { + super(message, cause); + } + + public AbortException(Throwable cause) + { + super(cause); + } + } } /** diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java index f3455d38095..d2068f6381e 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java @@ -1590,18 +1590,18 @@ public class HttpChannelState implements HttpChannel, Components httpChannelState._callbackFailure = failure; - // Consume any input. - Throwable unconsumed = stream.consumeAvailable(); - ExceptionUtil.addSuppressedIfNotAssociated(failure, unconsumed); + if (!stream.isCommitted() && !(failure instanceof Request.Handler.AbortException)) + { + // Consume any input. + Throwable unconsumed = stream.consumeAvailable(); + ExceptionUtil.addSuppressedIfNotAssociated(failure, unconsumed); - ChannelResponse response = httpChannelState._response; - if (LOG.isDebugEnabled()) - LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", stream.isCommitted(), response.isCommitted(), this); + ChannelResponse response = httpChannelState._response; + if (LOG.isDebugEnabled()) + LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", stream.isCommitted(), response.isCommitted(), this); - // There may have been an attempt to write an error response that failed. - // Do not try to write again an error response if already committed. - if (!stream.isCommitted()) errorResponse = new ErrorResponse(request); + } } if (errorResponse != null) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java index 70a145bc116..45bf9b1252c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java @@ -138,7 +138,7 @@ public class ServletApiResponse implements HttpServletResponse { switch (sc) { - case -1 -> getServletChannel().abort(new IOException(msg)); + case -1 -> getServletChannel().abort(new Request.Handler.AbortException(msg)); case HttpStatus.PROCESSING_102, HttpStatus.EARLY_HINTS_103 -> { if (!isCommitted()) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java index c1b3d8234d8..3667b60f8dd 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ErrorPageTest.java @@ -13,8 +13,12 @@ package org.eclipse.jetty.ee10.servlet; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStream; import java.io.PrintWriter; +import java.net.Socket; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -50,6 +54,7 @@ import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; import org.eclipse.jetty.util.Callback; @@ -70,6 +75,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -780,6 +786,220 @@ public class ErrorPageTest assertThat(responseBody, Matchers.containsString("ERROR_REQUEST_URI: /fail/599")); } + @Test + public void testAbortWithSendError() throws Exception + { + ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS); + contextHandler.setContextPath("/"); + + HttpServlet failServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException + { + response.sendError(-1); + } + }; + + contextHandler.addServlet(failServlet, "/abort"); + startServer(contextHandler); + + ServerConnector connector = new ServerConnector(_server); + connector.setPort(0); + _server.addConnector(connector); + connector.start(); + try (Socket socket = new Socket("localhost", connector.getLocalPort())) + { + OutputStream output = socket.getOutputStream(); + + String request = """ + GET /abort HTTP/1.1\r + Host: test\r + \r + """; + output.write(request.getBytes(StandardCharsets.UTF_8)); + output.flush(); + + BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream())); + String line = in.readLine(); + assertNull(line); + } + } + + @Test + public void testAbortWithSendErrorChunked() throws Exception + { + ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS); + contextHandler.setContextPath("/"); + + HttpServlet failServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException + { + response.getOutputStream().write("test".getBytes(StandardCharsets.UTF_8)); + response.flushBuffer(); + response.sendError(-1); + } + }; + + contextHandler.addServlet(failServlet, "/abort"); + startServer(contextHandler); + + ServerConnector connector = new ServerConnector(_server); + connector.setPort(0); + _server.addConnector(connector); + connector.start(); + try (Socket socket = new Socket("localhost", connector.getLocalPort())) + { + OutputStream output = socket.getOutputStream(); + + String request = """ + GET /abort HTTP/1.1\r + Host: test\r + \r + """; + output.write(request.getBytes(StandardCharsets.UTF_8)); + output.flush(); + + BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream())); + String line = in.readLine(); + assertThat(line, is("HTTP/1.1 200 OK")); + + boolean chunked = false; + while (!line.isEmpty()) + { + line = in.readLine(); + assertNotNull(line); + chunked |= line.equals("Transfer-Encoding: chunked"); + } + assertTrue(chunked); + + line = in.readLine(); + assertThat(line, is("4")); + line = in.readLine(); + assertThat(line, is("test")); + + line = in.readLine(); + assertNull(line); + } + } + + @Test + public void testAbortWithSendErrorContent() throws Exception + { + ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS); + contextHandler.setContextPath("/"); + + HttpServlet failServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException + { + response.setContentLength(10); + response.getOutputStream().write("test\r\n".getBytes(StandardCharsets.UTF_8)); + response.flushBuffer(); + response.sendError(-1); + } + }; + + contextHandler.addServlet(failServlet, "/abort"); + startServer(contextHandler); + + ServerConnector connector = new ServerConnector(_server); + connector.setPort(0); + _server.addConnector(connector); + connector.start(); + try (Socket socket = new Socket("localhost", connector.getLocalPort())) + { + OutputStream output = socket.getOutputStream(); + + String request = """ + GET /abort HTTP/1.1\r + Host: test\r + \r + """; + output.write(request.getBytes(StandardCharsets.UTF_8)); + output.flush(); + + BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream())); + String line = in.readLine(); + assertThat(line, is("HTTP/1.1 200 OK")); + + boolean chunked = false; + while (!line.isEmpty()) + { + line = in.readLine(); + assertNotNull(line); + chunked |= line.equals("Transfer-Encoding: chunked"); + } + assertFalse(chunked); + + line = in.readLine(); + assertThat(line, is("test")); + + line = in.readLine(); + assertNull(line); + } + } + + @Test + public void testAbortWithSendErrorComplete() throws Exception + { + ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS); + contextHandler.setContextPath("/"); + + HttpServlet failServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException + { + response.setContentLength(6); + response.getOutputStream().write("test\r\n".getBytes(StandardCharsets.UTF_8)); + response.sendError(-1); + } + }; + + contextHandler.addServlet(failServlet, "/abort"); + startServer(contextHandler); + + ServerConnector connector = new ServerConnector(_server); + connector.setPort(0); + _server.addConnector(connector); + connector.start(); + try (Socket socket = new Socket("localhost", connector.getLocalPort())) + { + OutputStream output = socket.getOutputStream(); + + String request = """ + GET /abort HTTP/1.1\r + Host: test\r + \r + """; + output.write(request.getBytes(StandardCharsets.UTF_8)); + output.flush(); + + BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream())); + String line = in.readLine(); + assertThat(line, is("HTTP/1.1 200 OK")); + + boolean chunked = false; + while (!line.isEmpty()) + { + line = in.readLine(); + assertNotNull(line); + chunked |= line.equals("Transfer-Encoding: chunked"); + } + assertFalse(chunked); + + line = in.readLine(); + assertThat(line, is("test")); + + line = in.readLine(); + assertNull(line); + } + } + @Test public void testErrorCodeNoDefaultServletNonExistentErrorLocation() throws Exception { diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java index 10e33baa484..37f94674459 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java @@ -488,7 +488,7 @@ public class Response implements HttpServletResponse switch (code) { - case -1 -> _channel.abort(new IOException(message)); + case -1 -> _channel.abort(new org.eclipse.jetty.server.Request.Handler.AbortException(message)); case HttpStatus.PROCESSING_102 -> sendProcessing(); case HttpStatus.EARLY_HINTS_103 -> sendEarlyHint(); default -> _channel.getState().sendError(code, message); diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java index 02b5682aa55..bbed1f62278 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ErrorPageTest.java @@ -13,8 +13,12 @@ package org.eclipse.jetty.ee9.servlet; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStream; import java.io.PrintWriter; +import java.net.Socket; import java.nio.charset.StandardCharsets; import java.util.EnumSet; import java.util.concurrent.ConcurrentHashMap; @@ -29,7 +33,6 @@ import jakarta.servlet.DispatcherType; import jakarta.servlet.Filter; import jakarta.servlet.FilterChain; import jakarta.servlet.FilterConfig; -import jakarta.servlet.RequestDispatcher; import jakarta.servlet.Servlet; import jakarta.servlet.ServletException; import jakarta.servlet.ServletRequest; @@ -49,10 +52,10 @@ import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,6 +65,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; //@Disabled // TODO @@ -105,6 +109,7 @@ public class ErrorPageTest _context.addServlet(ErrorAndStatusServlet.class, "/error-and-status/*"); _context.addServlet(ErrorContentTypeCharsetWriterInitializedServlet.class, "/error-mime-charset-writer/*"); _context.addServlet(ExceptionServlet.class, "/exception-servlet"); + _context.addServlet(AbortServlet.class, "/abort"); HandlerWrapper noopHandler = new HandlerWrapper() { @@ -300,6 +305,34 @@ public class ErrorPageTest assertThat(response, Matchers.containsString("ERROR_REQUEST_URI: /fail/code")); } + @Test + public void testAbortWithSendError() throws Exception + { + ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS); + contextHandler.setContextPath("/"); + + ServerConnector connector = new ServerConnector(_server); + connector.setPort(0); + _server.addConnector(connector); + connector.start(); + try (Socket socket = new Socket("localhost", connector.getLocalPort())) + { + OutputStream output = socket.getOutputStream(); + + String request = """ + GET /abort HTTP/1.1\r + Host: test\r + \r + """; + output.write(request.getBytes(StandardCharsets.UTF_8)); + output.flush(); + + BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream())); + String line = in.readLine(); + assertNull(line); + } + } + @Test public void testErrorException() throws Exception { @@ -871,4 +904,13 @@ public class ErrorPageTest super(rootCause); } } + + public static class AbortServlet extends HttpServlet + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException + { + response.sendError(-1); + } + } }