Smack 4.4.0-alpha2: reply IQ cannot contains both StanzaError and ExtensionElement

Smack Smack Dev
1

aTalk is implementing XEP-0070: Verifying HTTP Requests via XMPP:

On received of an HTTP IQ request iqRequest i,e.

<iq type='get'
    from='files.shakespeare.lit'
    to='juliet@capulet.com/balcony'
    id='ha000'>
  <confirm xmlns='http://jabber.org/protocol/http-auth'
           id='a7374jnjlalasdf82'
           method='GET'
           url='https://files.shakespeare.lit:9345/missive.html'/>
</iq>

On deny, aTalk is required to sent the following stanza i.e.

<iq type='error'
    from='juliet@capulet.com/balcony'
    to='files.shakespeare.lit'
    id='ha000'>
  <confirm xmlns='http://jabber.org/protocol/http-auth'
           id='a7374jnjlalasdf82'
           method='GET'
           url='https://files.shakespeare.lit:9345/missive.html'/>
  <error code='401' type='auth'>
    <not-authorized xmlns='urn:ietf:params:xml:xmpp-stanzas'/>
  </error>
</iq>

Using the following approach, it was found that the return errorIQ will strip off the Confirm ExtensionElement, leaving only the StanzaError.

StanzaError errorBuilder = StanzaError.getBuilder()
        .setType(StanzaError.Type.AUTH)
        .setCondition(StanzaError.Condition.not_authorized).build();
ErrorIQ errorIQ = IQ.createErrorResponse(iqRequest, errorBuilder);

When aTalk is trying to build using IQ class method, it seems that reply IQ can only contain either a StanzaError or an ExtensionElement.

    public final XmlStringBuilder getChildElementXML(XmlEnvironment enclosingXmlEnvironment) {
        XmlStringBuilder xml = new XmlStringBuilder();
        if (type == Type.error) {
            // Add the error sub-packet, if there is one.
            appendErrorIfExists(xml, enclosingXmlEnvironment);
        }
        else if (childElementName != null) {
            // Add the query section if there is one.
            IQChildElementXmlStringBuilder iqChildElement = getIQChildElementBuilder(new IQChildElementXmlStringBuilder(this));
            if (iqChildElement != null) {
                xml.append(iqChildElement);

                List<ExtensionElement> extensionsXml = getExtensions();
                if (iqChildElement.isEmptyElement) {
                    if (extensionsXml.isEmpty()) {
                         xml.closeEmptyElement();
                         return xml;
                    } else {
                        xml.rightAngleBracket();
                    }
                }
                xml.append(extensionsXml);
                xml.closeElement(iqChildElement.element);
            }
        }
        return xml;
    }

If my understanding is correct, it means aTalk needs to take special case to generate the required HTTP request denied IQ. e.g. to override the below method to include the ErrorStanza.

protected IQChildElementXmlStringBuilder getIQChildElementBuilder(IQChildElementXmlStringBuilder xml)

Or there is an easy way to generate using the existing IQ class.

2

Finally aTalk has to resort to override the method appendErrorIfExists() to get thing working. Code attached below for those whom are interested.

The XEP-0070: Verifying HTTP Requests via XMPP full source implementation for aTalk will be in https://github.com/cmeng-git/atalk-android repository when aTalk v1.8.8 is released.
Smack team is always welcome to include the source in the future smack library release.

    public void initializeAsError(StanzaError stanzaError)
    {
        this.stanzaError = stanzaError;

        Jid from = this.getFrom();
        setFrom(this.getTo());
        setTo(from);
        setType(Type.error);
    }

    @Override
    protected IQChildElementXmlStringBuilder getIQChildElementBuilder(IQChildElementXmlStringBuilder xml)
    {
        if (confirmExtension != null) {
            xml.setEmptyElement();

            xml.attribute(ConfirmExtension.ATTR_ID, confirmExtension.getId());
            xml.attribute(ConfirmExtension.ATTR_METHOD, confirmExtension.getMethod());
            xml.attribute(ConfirmExtension.ATTR_URL, confirmExtension.getUrl());
        }
        else {
            return null;
        }
        return xml;
    }

    /**
     * Append an XMPPError is this stanza has one set.
     *
     * @param xml the XmlStringBuilder to append the error to.
     */
    @Override
    protected void appendErrorIfExists(XmlStringBuilder xml, XmlEnvironment enclosingXmlEnvironment)
    {
        IQChildElementXmlStringBuilder iqChildElement
                = getIQChildElementBuilder(new IQChildElementXmlStringBuilder(confirmExtension));

        if (iqChildElement != null) {
            xml.append(iqChildElement);
            xml.closeEmptyElement();
        }

        if (stanzaError != null) {
            xml.append(stanzaError.toXML(enclosingXmlEnvironment));
        }
    }

and the results:

2019-06-22 13:59:42.598 29538-30241/org.atalk.android D/SMACK: RECV (0): 
    <iq to='swordfish@atalk.sytes.net/atalk' from='auth.agayon.be' type='get' id='45'>
      <confirm xmlns='http://jabber.org/protocol/http-auth' method='POST' id='jjriUv' url='agayon.be'/>
    </iq>


2019-06-22 13:59:46.209 29538-30240/org.atalk.android D/SMACK: SENT (0): 
    <iq to='auth.agayon.be' id='45' type='error'>
      <confirm xmlns='http://jabber.org/protocol/http-auth' id='jjriUv' method='POST' url='agayon.be'/>
      <error type='auth'>
        <not-authorized xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>
      </error>
    </iq>
3

You are most certainly not required to send an IQ error back that includes the sender XML.

That is right. Although the RFC allows the sender XML to be included in error stanzas, I never saw a need to add it because it is a completely optional feature. I consider it also not really that much helpful, since you usually have both the request and response in your XMPP trace when debugging and I personally assume the additional send XML causes more confusion than good.

4

You are right, I also have the same thought, why there is a need to include sender xml in the reply initially, just follow as the spec said so. However I just realized that the online spec in fact stated it is optional. Thanks for your clarification.

https://xmpp.org/extensions/xep-0070.html#top

If the user wishes to deny the request, the &lt;iq/&gt; response stanza MUST be of type "error", MAY contain the original &lt;confirm/&gt; child element (although this is not necessary since the XMPP 'id' attribute can be used for tracking purposes), and MUST specify an error, which SHOULD be &lt;not-authorized/&gt;:
5

Although it’s a server generated IQ, XEP-0077 suggests to include payload with error (“SHOULD”) .
Example 16 + 23.

This at least shows that use cases for error IQs with payload do exist.
Also note, that the requester does not know about the payload, which is sent in the error response.

1 Like
6

Thanks for pointing this out.

Meanwhile I have created SMACK-873 to track this.

7

This topic was automatically closed 100 days after the last reply. New replies are no longer allowed.