Publish options raise conflict even if there is none

[lovetox]

Environment

• ejabberd version: 23.10

Bug description

Given a node configured like this

<iq xmlns="jabber:client" xml:lang="en" to="lovetox2@movim.eu/gajim.201FJHRE" from="lovetox2@movim.eu" type="result" id="6335763f-23d0-4f8b-9c1c-77513705ce6b">
  <pubsub xmlns="http://jabber.org/protocol/pubsub#owner">
    <configure node="urn:xmpp:bookmarks:1">
      <x type="form" xmlns="jabber:x:data">
        <field var="FORM_TYPE" type="hidden">
          <value>http://jabber.org/protocol/pubsub#node_config</value>
</field>
        <field var="pubsub#deliver_payloads" type="boolean" label="Deliver payloads with event notifications">
          <value>1</value>
</field>
        <field var="pubsub#notify_config" type="boolean" label="Notify subscribers when the node configuration changes">
          <value>0</value>
</field>
        <field var="pubsub#notify_delete" type="boolean" label="Notify subscribers when the node is deleted">
          <value>1</value>
</field>
        <field var="pubsub#notify_retract" type="boolean" label="Notify subscribers when items are removed from the node">
          <value>1</value>
</field>
        <field var="pubsub#purge_offline" type="boolean" label="Purge all items when the relevant publisher goes offline">
          <value>0</value>
</field>
        <field var="pubsub#persist_items" type="boolean" label="Persist items to storage">
          <value>1</value>
</field>
        <field var="pubsub#max_items" type="text-single" label="Max # of items to persist, or `max` for no specific limit other than a server imposed maximum">
          <value>max</value>
</field>
        <field var="pubsub#subscribe" type="boolean" label="Whether to allow subscriptions">
          <value>1</value>
</field>
        <field var="pubsub#access_model" type="list-single" label="Specify the access model">
          <value>whitelist</value>
          <option label="Subscription requests must be approved and only subscribers may retrieve items">
            <value>authorize</value>
</option>
          <option label="Anyone may subscribe and retrieve items">
            <value>open</value>
</option>
          <option label="Anyone with a presence subscription of both or from may subscribe and retrieve items">
            <value>presence</value>
</option>
          <option label="Anyone in the specified roster group(s) may subscribe and retrieve items">
            <value>roster</value>
</option>
          <option label="Only those on a whitelist may subscribe and retrieve items">
            <value>whitelist</value>
</option>
</field>
        <field var="pubsub#roster_groups_allowed" type="list-multi" label="Roster groups allowed to subscribe" />
        <field var="pubsub#publish_model" type="list-single" label="Specify the publisher model">
          <value>publishers</value>
          <option label="Only publishers may publish">
            <value>publishers</value>
</option>
          <option label="Subscribers may publish">
            <value>subscribers</value>
</option>
          <option label="Anyone may publish">
            <value>open</value>
</option>
</field>
        <field var="pubsub#notification_type" type="list-single" label="Specify the event message type">
          <value>headline</value>
          <option label="Messages of type normal">
            <value>normal</value>
</option>
          <option label="Messages of type headline">
            <value>headline</value>
</option>
</field>
        <field var="pubsub#max_payload_size" type="text-single" label="Max payload size in bytes">
          <value>250000</value>
</field>
        <field var="pubsub#send_last_published_item" type="list-single" label="When to send the last published item">
          <value>never</value>
          <option label="Never">
            <value>never</value>
</option>
          <option label="When a new subscription is processed">
            <value>on_sub</value>
</option>
          <option label="When a new subscription is processed and whenever a subscriber comes online">
            <value>on_sub_and_presence</value>
</option>
</field>
        <field var="pubsub#deliver_notifications" type="boolean" label="Deliver event notifications">
          <value>1</value>
</field>
        <field var="pubsub#presence_based_delivery" type="boolean" label="Only deliver notifications to available users">
          <value>1</value>
</field>
        <field var="pubsub#itemreply" type="list-single" label="Whether owners or publisher should receive replies to items">
          <value>publisher</value>
          <option label="Statically specify a replyto of the node owner(s)">
            <value>owner</value>
</option>
          <option label="Dynamically specify a replyto of the item publisher">
            <value>publisher</value>
</option>
          <option>
            <value>none</value>
</option>
</field>
</x>
</configure>
</pubsub>
</iq>

i try to publish a bookmark to this node with this and receive a conflict error (iq is the response but contains the query)

<iq from="lovetox2@movim.eu" id="31e6e74a-780e-4fcf-8d41-9e9b80a55882" to="lovetox2@movim.eu/gajim.201FJHRE" type="error" xml:lang="en" xmlns="jabber:client">
    <pubsub xmlns="http://jabber.org/protocol/pubsub">
        <publish node="urn:xmpp:bookmarks:1">
            <item id="gajim@conference.gajim.org">
                <conference name="Gajim" xmlns="urn:xmpp:bookmarks:1">
                    <nick>lovetox2</nick>
                    <extensions xmlns="urn:xmpp:bookmarks:1">
                        <notifications notify="quoted" xmlns="xmpp:movim.eu/notifications:0"/>
                    </extensions>
                </conference>
            </item>
        </publish>
        <publish-options>
            <x type="submit" xmlns="jabber:x:data">
                <field type="hidden" var="FORM_TYPE">
                    <value>http://jabber.org/protocol/pubsub#publish-options</value>
                </field>
                <field var="pubsub#notify_delete">
                    <value>true</value>
                </field>
                <field var="pubsub#notify_retract">
                    <value>true</value>
                </field>
                <field var="pubsub#persist_items">
                    <value>true</value>
                </field>
                <field var="pubsub#max_items">
                    <value>max</value>
                </field>
                <field var="pubsub#access_model">
                    <value>whitelist</value>
                </field>
                <field var="pubsub#send_last_published_item">
                    <value>never</value>
                </field>
            </x>
        </publish-options>
    </pubsub>
    <error type="cancel">
        <precondition-not-met xmlns="http://jabber.org/protocol/pubsub#errors"/>
        <conflict xmlns="urn:ietf:params:xml:ns:xmpp-stanzas"/>
    </error>
</iq>

the only difference i see is that i publish with true instead of 1

[licaon-kter]

Fixed by https://github.com/processone/ejabberd/commit/f48275bc112b123bb632cbd8fa52e6330ed6ac66 yes?

/LE: from last weeks support channel

(fun(S)->J=jid:decode(S),U=jid:tolower(J),mod_pubsub:set_configure(U, <<"storage:bookmarks">>, J, [{access_model, whitelist}, {persist_items, true}], <<>>) end)(<<"user@server">>).

...should fix it too.

I've asked the devs to either add it to docs or re-release 23.10 (as admins might update to stable and hit this issue as it's a release feature :))

[lovetox]

This commit looks dangerous, publish options should never be ignored, this is simply against the spec and dangerous. We publish private data to pubsub and clients need to be sure that publish options have effect and are not silently ignored.

This commit looks like its some internal publish, like the private bookmarks module calls a publish on the pubsub module. So ejabberd acts like a client. A client would never want publish options ignored, so why should ejabberd internally ignore them.

[edhelas]

I've put a comment with a XML dump showing that the fix might not work on the original ticket there https://github.com/processone/ejabberd/issues/3044#issuecomment-1775832114

[prefiks]

Strangely with node configured that way, publish with those publish options works here. Do you guys know if mod_pubsub on that server have any special options set (force_node_config maybe?). Not sure if that could be a problem here but it could help me reproduce this.

Commit that licaon-kter pointed to, just removes permission checks that we perform during transparent conversions to other storage methods (mod_private or storage:bookmarks node), clients data is still checked.

[prefiks]

Now that i think about it it could be error from trying to set storage:bookmarks node propagating here, could you also check options for storage:bookmarks node on your account?

[lovetox]

Indeed it has a conflicting config

<iq xmlns="jabber:client" xml:lang="en" to="lovetox2@movim.eu/gajim.201FJHRE" from="lovetox2@movim.eu" type="result" id="e9de5abb-c120-4498-8d8e-02e375e1f67b">
  <pubsub xmlns="http://jabber.org/protocol/pubsub#owner">
    <configure node="storage:bookmarks">
      <x type="form" xmlns="jabber:x:data">
        <field var="FORM_TYPE" type="hidden">
          <value>http://jabber.org/protocol/pubsub#node_config</value>
</field>
        <field var="pubsub#deliver_payloads" type="boolean" label="Deliver payloads with event notifications">
          <value>1</value>
</field>
        <field var="pubsub#notify_config" type="boolean" label="Notify subscribers when the node configuration changes">
          <value>0</value>
</field>
        <field var="pubsub#notify_delete" type="boolean" label="Notify subscribers when the node is deleted">
          <value>0</value>
</field>
        <field var="pubsub#notify_retract" type="boolean" label="Notify subscribers when items are removed from the node">
          <value>0</value>
</field>
        <field var="pubsub#purge_offline" type="boolean" label="Purge all items when the relevant publisher goes offline">
          <value>0</value>
</field>
        <field var="pubsub#persist_items" type="boolean" label="Persist items to storage">
          <value>1</value>
</field>
        <field var="pubsub#max_items" type="text-single" label="Max # of items to persist, or `max` for no specific limit other than a server imposed maximum">
          <value>1</value>
</field>
        <field var="pubsub#subscribe" type="boolean" label="Whether to allow subscriptions">
          <value>1</value>
</field>
        <field var="pubsub#access_model" type="list-single" label="Specify the access model">
          <value>whitelist</value>
          <option label="Subscription requests must be approved and only subscribers may retrieve items">
            <value>authorize</value>
</option>
          <option label="Anyone may subscribe and retrieve items">
            <value>open</value>
</option>
          <option label="Anyone with a presence subscription of both or from may subscribe and retrieve items">
            <value>presence</value>
</option>
          <option label="Anyone in the specified roster group(s) may subscribe and retrieve items">
            <value>roster</value>
</option>
          <option label="Only those on a whitelist may subscribe and retrieve items">
            <value>whitelist</value>
</option>
</field>
        <field var="pubsub#roster_groups_allowed" type="list-multi" label="Roster groups allowed to subscribe" />
        <field var="pubsub#publish_model" type="list-single" label="Specify the publisher model">
          <value>publishers</value>
          <option label="Only publishers may publish">
            <value>publishers</value>
</option>
          <option label="Subscribers may publish">
            <value>subscribers</value>
</option>
          <option label="Anyone may publish">
            <value>open</value>
</option>
</field>
        <field var="pubsub#notification_type" type="list-single" label="Specify the event message type">
          <value>headline</value>
          <option label="Messages of type normal">
            <value>normal</value>
</option>
          <option label="Messages of type headline">
            <value>headline</value>
</option>
</field>
        <field var="pubsub#max_payload_size" type="text-single" label="Max payload size in bytes">
          <value>250000</value>
</field>
        <field var="pubsub#send_last_published_item" type="list-single" label="When to send the last published item">
          <value>on_sub_and_presence</value>
          <option label="Never">
            <value>never</value>
</option>
          <option label="When a new subscription is processed">
            <value>on_sub</value>
</option>
          <option label="When a new subscription is processed and whenever a subscriber comes online">
            <value>on_sub_and_presence</value>
</option>
</field>
        <field var="pubsub#deliver_notifications" type="boolean" label="Deliver event notifications">
          <value>1</value>
</field>
        <field var="pubsub#presence_based_delivery" type="boolean" label="Only deliver notifications to available users">
          <value>1</value>
</field>
        <field var="pubsub#itemreply" type="list-single" label="Whether owners or publisher should receive replies to items">
          <value>publisher</value>
          <option label="Statically specify a replyto of the node owner(s)">
            <value>owner</value>
</option>
          <option label="Dynamically specify a replyto of the item publisher">
            <value>publisher</value>
</option>
          <option>
            <value>none</value>
</option>
</field>
</x>
</configure>
</pubsub>
</iq>

[lovetox]

i made it so its the same as bookmarks:1, so its not in conflict anymore, but i still cant publish

[prefiks]

Thanks, yes looks correct, we check for persist_item=true and access_model=whitelist, which seems to be set as expected, so probably something different. It's also not difference between 1 or true, they are internally converted to same value, and are just shown that way when sent as xdata payload.

[lovetox]

would debug logs from the server help?

[prefiks]

I will prepare some code tomorrow that will gather information that show what causes this conflict.

[lovetox]

my tests happend on movim.eu which uses 23.10

But i just set up a brand new docker container with 23.10, and i dont run into this problem.

@edhelas it seems to be something specific to movim.eu config

[prefiks]

So this was caused by max_items value stored in db having value infinity, recent versions perform normalization on write and use max for that case, but if node was created before that normalization step was added it value on disk may use infinity, part that match values from publish-option expect max here, and that why this was failing. Node config iq also perform normalization of option, and that why it also showed max in returned config.

I will add normalization to db read operation, to complement our write normalization, that should hopefully fix this issue.

[prefiks]

And fixup on read was added in 52e7c166fca5790e79cc5d9c6deae244e21806a8 and 3bf4cf5c3f5a5f4dec12f7068efd50c40a6a5474

[lovetox]

Thanks. A fast Bugfix release would be appreciated as we get many bug request from users.

[Neustradamus]

It will be nice to release a 23.11 or 23.10.1 with new commits ^^