From f49eb66bb7ed231541236617843544890670e598 Mon Sep 17 00:00:00 2001 From: "Alexander \"PapaTutuWawa" Date: Sun, 12 Mar 2023 19:11:55 +0100 Subject: [PATCH] fix(xep): Fix usage of 'max' in publish options (#33) This commit fixes two issues: 1. Fix an issue where [PubSubManager.publish] would always, if given publish options with maxItems set to 'max', use 'max' in the max_items publish options, even if the server indicates it does not support that. 2. Fix an issue with the StanzaExpectation, where it would let every stanza pass. --- .../lib/src/xeps/xep_0060/xep_0060.dart | 22 +- packages/moxxmpp/test/helpers/manager.dart | 63 ++++ packages/moxxmpp/test/helpers/xmpp.dart | 88 +++++- packages/moxxmpp/test/stringxml_test.dart | 48 +++ packages/moxxmpp/test/xeps/xep_0060_test.dart | 293 +++++++++--------- 5 files changed, 348 insertions(+), 166 deletions(-) create mode 100644 packages/moxxmpp/test/helpers/manager.dart diff --git a/packages/moxxmpp/lib/src/xeps/xep_0060/xep_0060.dart b/packages/moxxmpp/lib/src/xeps/xep_0060/xep_0060.dart index 57ddcb5..f1254c6 100644 --- a/packages/moxxmpp/lib/src/xeps/xep_0060/xep_0060.dart +++ b/packages/moxxmpp/lib/src/xeps/xep_0060/xep_0060.dart @@ -1,3 +1,5 @@ +import 'package:freezed_annotation/freezed_annotation.dart'; +import 'package:meta/meta.dart'; import 'package:moxxmpp/src/events.dart'; import 'package:moxxmpp/src/jid.dart'; import 'package:moxxmpp/src/managers/base.dart'; @@ -130,7 +132,10 @@ class PubSubManager extends XmppManagerBase { return count; } - Future _preprocessPublishOptions( + // TODO(PapaTutuWawa): This should return a Result in case we cannot proceed + // with the requested configuration. + @visibleForTesting + Future preprocessPublishOptions( String jid, String node, PubSubPublishOptions options, @@ -285,7 +290,7 @@ class PubSubManager extends XmppManagerBase { }) async { PubSubPublishOptions? pubOptions; if (options != null) { - pubOptions = await _preprocessPublishOptions(jid, node, options); + pubOptions = await preprocessPublishOptions(jid, node, options); } final result = await getAttributes().sendStanza( @@ -310,14 +315,11 @@ class PubSubManager extends XmppManagerBase { ) ], ), - ...options != null - ? [ - XMLNode( - tag: 'publish-options', - children: [options.toXml()], - ), - ] - : [], + if (pubOptions != null) + XMLNode( + tag: 'publish-options', + children: [pubOptions.toXml()], + ), ], ) ], diff --git a/packages/moxxmpp/test/helpers/manager.dart b/packages/moxxmpp/test/helpers/manager.dart new file mode 100644 index 0000000..dbb41cb --- /dev/null +++ b/packages/moxxmpp/test/helpers/manager.dart @@ -0,0 +1,63 @@ +import 'dart:async'; +import 'package:moxxmpp/src/connection.dart'; +import 'package:moxxmpp/src/connectivity.dart'; +import 'package:moxxmpp/src/jid.dart'; +import 'package:moxxmpp/src/managers/attributes.dart'; +import 'package:moxxmpp/src/managers/base.dart'; +import 'package:moxxmpp/src/reconnect.dart'; +import 'package:moxxmpp/src/settings.dart'; +import 'package:moxxmpp/src/socket.dart'; +import 'package:moxxmpp/src/stringxml.dart'; + +import '../helpers/xmpp.dart'; + +/// This class allows registering managers for easier testing. +class TestingManagerHolder { + TestingManagerHolder({ + BaseSocketWrapper? socket, + }) : _socket = socket ?? StubTCPSocket([]); + + final BaseSocketWrapper _socket; + + final Map _managers = {}; + + static final JID jid = JID.fromString('testuser@example.org/abc123'); + static final ConnectionSettings settings = ConnectionSettings( + jid: jid, + password: 'abc123', + useDirectTLS: true, + allowPlainAuth: true, + ); + + Future _sendStanza(stanza, { StanzaFromType addFrom = StanzaFromType.full, bool addId = true, bool awaitable = true, bool encrypted = false, bool forceEncryption = false, }) async { + return XMLNode.fromString(''); + } + + T? _getManagerById(String id) { + return _managers[id] as T?; + } + + Future register(XmppManagerBase manager) async { + manager.register( + XmppManagerAttributes( + sendStanza: _sendStanza, + getConnection: () => XmppConnection( + TestingReconnectionPolicy(), + AlwaysConnectedConnectivityManager(), + _socket, + ), + getConnectionSettings: () => settings, + sendNonza: (_) {}, + sendEvent: (_) {}, + getSocket: () => _socket, + isFeatureSupported: (_) => false, + getNegotiatorById: getNegotiatorNullStub, + getFullJID: () => jid, + getManagerById: _getManagerById, + ), + ); + + await manager.postRegisterCallback(); + _managers[manager.id] = manager; + } +} \ No newline at end of file diff --git a/packages/moxxmpp/test/helpers/xmpp.dart b/packages/moxxmpp/test/helpers/xmpp.dart index 2fdc538..c76bab9 100644 --- a/packages/moxxmpp/test/helpers/xmpp.dart +++ b/packages/moxxmpp/test/helpers/xmpp.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:convert'; import 'package:moxxmpp/moxxmpp.dart'; import 'package:test/test.dart'; @@ -13,8 +14,8 @@ T? getManagerNullStub(String id) { } abstract class ExpectationBase { - ExpectationBase(this.expectation, this.response); + final String expectation; final String response; @@ -33,27 +34,84 @@ class StringExpectation extends ExpectationBase { /// class StanzaExpectation extends ExpectationBase { StanzaExpectation(String expectation, String response, {this.ignoreId = false, this.adjustId = false }) : super(expectation, response); + final bool ignoreId; final bool adjustId; @override bool matches(String input) { final ex = XMLNode.fromString(expectation); - final recv = XMLNode.fromString(expectation); + final recv = XMLNode.fromString(input); return compareXMLNodes(recv, ex, ignoreId: ignoreId); } } -class StubTCPSocket extends BaseSocketWrapper { // Request -> Response(s) +/// Use [settings] to build the beginning of a play that can be used with StubTCPSocket. [settings]'s allowPlainAuth must +/// be set to true. +List buildAuthenticatedPlay(ConnectionSettings settings) { + assert(settings.allowPlainAuth, 'SASL PLAIN must be allowed'); + + final plain = base64.encode(utf8.encode('\u0000${settings.jid.local}\u0000${settings.password}')); + return [ + StringExpectation( + "", + ''' + + + + PLAIN + + ''', + ), + StringExpectation( + "$plain", + '' + ), + StringExpectation( + "", + ''' + + + + + + +''', + ), + StanzaExpectation( + '', + '${settings.jid.toBare()}/MU29eEZn', + ignoreId: true, + ), + StanzaExpectation( + "chat", + '', + ), + ]; +} + +class StubTCPSocket extends BaseSocketWrapper { // Request -> Response(s) + StubTCPSocket(this._play); + + StubTCPSocket.authenticated(ConnectionSettings settings, List play) : _play = [ + ...buildAuthenticatedPlay(settings), + ...play, + ]; - StubTCPSocket({ required List play }) - : _play = play, - _dataStream = StreamController.broadcast(), - _eventStream = StreamController.broadcast(); int _state = 0; - final StreamController _dataStream; - final StreamController _eventStream; + final StreamController _dataStream = StreamController.broadcast(); + final StreamController _eventStream = StreamController.broadcast(); final List _play; String? lastId; @@ -99,9 +157,11 @@ class StubTCPSocket extends BaseSocketWrapper { // Request -> Response(s) str = str.substring(0, str.length - 16); } - if (!expectation.matches(str)) { - expect(true, false, reason: 'Expected ${expectation.expectation}, got $str'); - } + expect( + expectation.matches(str), + true, + reason: 'Expected ${expectation.expectation}, got $str', + ); // Make sure to only progress if everything passed so far _state++; @@ -109,7 +169,7 @@ class StubTCPSocket extends BaseSocketWrapper { // Request -> Response(s) var response = expectation.response; if (expectation is StanzaExpectation) { final inputNode = XMLNode.fromString(str); - lastId = inputNode.attributes['id']; + lastId = inputNode.attributes['id'] as String?; if (expectation.adjustId) { final outputNode = XMLNode.fromString(response); @@ -134,4 +194,4 @@ class StubTCPSocket extends BaseSocketWrapper { // Request -> Response(s) @override bool managesKeepalives() => false; -} +} \ No newline at end of file diff --git a/packages/moxxmpp/test/stringxml_test.dart b/packages/moxxmpp/test/stringxml_test.dart index 7d1efc4..5ca630e 100644 --- a/packages/moxxmpp/test/stringxml_test.dart +++ b/packages/moxxmpp/test/stringxml_test.dart @@ -29,4 +29,52 @@ void main() { expect(compareXMLNodes(node1.firstTag('body')!, XMLNode.fromString('Hallo')), true); expect(compareXMLNodes(node1.firstTagByXmlns('a')!, XMLNode.fromString('')), true); }); + + test('Test compareXMLNodes', () { + final node1 = XMLNode.fromString(''' + + + + + + + + + + + http://jabber.org/protocol/pubsub#publish-options + + + max + + + + + +''', + ); + final node2 = XMLNode.fromString(''' + + + + + + + + + + + http://jabber.org/protocol/pubsub#publish-options + + + 1 + + + + + +'''); + + expect(compareXMLNodes(node1, node2, ignoreId: true), false); + }); } diff --git a/packages/moxxmpp/test/xeps/xep_0060_test.dart b/packages/moxxmpp/test/xeps/xep_0060_test.dart index f4a40f6..9ca4587 100644 --- a/packages/moxxmpp/test/xeps/xep_0060_test.dart +++ b/packages/moxxmpp/test/xeps/xep_0060_test.dart @@ -2,121 +2,23 @@ import 'package:moxxmpp/moxxmpp.dart'; import 'package:test/test.dart'; import '../helpers/logging.dart'; +import '../helpers/manager.dart'; import '../helpers/xmpp.dart'; class StubbedDiscoManager extends DiscoManager { - StubbedDiscoManager() : super([]); + StubbedDiscoManager(this._itemError) : super([]); + + final bool _itemError; @override Future> discoInfoQuery(String entity, { String? node, bool shouldEncrypt = true }) async { final result = DiscoInfo.fromQuery( XMLNode.fromString( - ''' - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -http://jabber.org/network/serverinfo - - -mailto:support@tigase.net -xmpp:tigase@mix.tigase.im -xmpp:tigase@muc.tigase.org -https://tigase.net/technical-support - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + ''' + + + + ''' ), JID.fromString('pubsub.server.example.org'), @@ -124,49 +26,156 @@ class StubbedDiscoManager extends DiscoManager { return Result(result); } -} -T? getDiscoManagerStub(String id) { - return StubbedDiscoManager() as T; + @override + Future>> discoItemsQuery(String entity, {String? node, bool shouldEncrypt = true}) async { + if (_itemError) { + return Result( + UnknownDiscoError(), + ); + } + return const Result>( + [], + ); + } } void main() { initLogger(); - test('Test publishing with pubsub#max_items when the server does not support it', () async { - XMLNode? sent; + test('Test pre-processing with pubsub#max_items when the server does not support it (1/2)', () async { final manager = PubSubManager(); - manager.register( - XmppManagerAttributes( - sendStanza: (stanza, { StanzaFromType addFrom = StanzaFromType.full, bool addId = true, bool awaitable = true, bool encrypted = false, bool forceEncryption = false, }) async { - sent = stanza; + final TestingManagerHolder tm = TestingManagerHolder(); + await tm.register(StubbedDiscoManager(false)); + await tm.register(manager); - return XMLNode.fromString(''); - }, - sendNonza: (_) {}, - sendEvent: (_) {}, - getManagerById: getDiscoManagerStub, - getConnectionSettings: () => ConnectionSettings( - jid: JID.fromString('hallo@example.server'), - password: 'password', - useDirectTLS: true, - allowPlainAuth: false, - ), - isFeatureSupported: (_) => false, - getFullJID: () => JID.fromString('hallo@example.server/uwu'), - getSocket: () => StubTCPSocket(play: []), - getConnection: () => XmppConnection(TestingReconnectionPolicy(), AlwaysConnectedConnectivityManager(), StubTCPSocket(play: [])), - getNegotiatorById: getNegotiatorNullStub, - ), + final result = await manager.preprocessPublishOptions( + 'pubsub.server.example.org', + 'urn:xmpp:omemo:2:bundles', + const PubSubPublishOptions(maxItems: 'max'), ); - // final result = await manager.preprocessPublishOptions( - // 'pubsub.server.example.org', - // 'example:node', - // PubSubPublishOptions( - // maxItems: 'max', - // ), - // ); - + expect(result.maxItems, '1'); }); -} + + test('Test pre-processing with pubsub#max_items when the server does not support it (2/2)', () async { + final manager = PubSubManager(); + final TestingManagerHolder tm = TestingManagerHolder(); + await tm.register(StubbedDiscoManager(true)); + await tm.register(manager); + + final result = await manager.preprocessPublishOptions( + 'pubsub.server.example.org', + 'urn:xmpp:omemo:2:bundles', + const PubSubPublishOptions(maxItems: 'max'), + ); + + expect(result.maxItems, '1'); + }); + + test('Test publishing with pubsub#max_items when the server does not support it', () async { + final socket = StubTCPSocket.authenticated( + TestingManagerHolder.settings, + [ + StanzaExpectation( + ''' + + + +''', + ''' + + + + + + + +''', + ignoreId: true, + adjustId: true, + ), + StanzaExpectation( + ''' + + + +''', + ''' + + + +''', + ignoreId: true, + adjustId: true, + ), + StanzaExpectation( + ''' + + + + + + + + + + + http://jabber.org/protocol/pubsub#publish-options + + + 1 + + + + +''', + ''' + + + + + + +''', + ignoreId: true, + adjustId: true, + ) + ], + ); + + final connection = XmppConnection( + TestingReconnectionPolicy(), + AlwaysConnectedConnectivityManager(), + socket, + ); + + await connection.registerManagers([ + PubSubManager(), + DiscoManager([]), + PresenceManager(), + MessageManager(), + RosterManager(TestingRosterStateManager(null, [])), + PingManager(), + ]); + connection..registerFeatureNegotiators([ + SaslPlainNegotiator(), + ResourceBindingNegotiator(), + ]) + ..setConnectionSettings(TestingManagerHolder.settings); + await connection.connect( + waitUntilLogin: true, + ); + + final item = XMLNode(tag: "test-item"); + final result = await connection.getManagerById(pubsubManager)!.publish( + 'pubsub.server.example.org', + 'princely_musings', + item, + id: 'current', + options: const PubSubPublishOptions(maxItems: 'max'), + ); + + expect(result.isType(), true); + }); +} \ No newline at end of file