1
0
Fork 0
mirror of https://github.com/vanitasvitae/Smack.git synced 2024-12-25 12:08:00 +01:00

[core] Fix ErrorIQ not showing potential original IQ child

Also delete StanzaIdTest since the test was fundamentally weak and
flawed. It does not work anyway, since TestIQ has a fixed stanza ID.

Fixes SMACK-931.
This commit is contained in:
Florian Schmaus 2023-03-08 17:40:29 +01:00
parent ef0fc01505
commit bf3a27df9a
12 changed files with 164 additions and 126 deletions

View file

@ -49,7 +49,7 @@ public abstract class AbstractIqBuilder<IB extends AbstractIqBuilder<IB>> extend
} }
protected static IqData createResponse(IqView request, IQ.ResponseType responseType) { protected static IqData createResponse(IqView request, IQ.ResponseType responseType) {
if (!(request.getType() == IQ.Type.get || request.getType() == IQ.Type.set)) { if (!request.isRequestIQ()) {
throw new IllegalArgumentException("IQ request must be of type 'set' or 'get'. Original IQ: " + request); throw new IllegalArgumentException("IQ request must be of type 'set' or 'get'. Original IQ: " + request);
} }

View file

@ -1,6 +1,6 @@
/** /**
* *
* Copyright © 2014-2019 Florian Schmaus * Copyright © 2014-2023 Florian Schmaus
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -24,7 +24,7 @@ public class EmptyResultIQ extends IQ {
// TODO: Deprecate when stanza builder and parsing logic is ready. // TODO: Deprecate when stanza builder and parsing logic is ready.
public EmptyResultIQ() { public EmptyResultIQ() {
super(null, null); super((String) null, null);
setType(IQ.Type.result); setType(IQ.Type.result);
} }

View file

@ -1,6 +1,6 @@
/** /**
* *
* Copyright © 2014 Florian Schmaus * Copyright © 2014-2023 Florian Schmaus
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,24 +16,82 @@
*/ */
package org.jivesoftware.smack.packet; package org.jivesoftware.smack.packet;
import org.jivesoftware.smack.util.Objects; import java.util.Objects;
public class ErrorIQ extends SimpleIQ { import javax.xml.namespace.QName;
/**
* An XMPP error IQ.
* <p>
* According to RFC 6120 § 8.3.1 "4. An error stanza MUST contain an &lt;error/&gt; child element.", so this class can
* only be constructed if a stanza error is provided.
*/
public final class ErrorIQ extends IQ {
public static final String ELEMENT = StanzaError.ERROR; public static final String ELEMENT = StanzaError.ERROR;
/** private final IQ request;
* Constructs a new error IQ.
* <p> private ErrorIQ(Builder builder, QName childElementQName) {
* According to RFC 6120 § 8.3.1 "4. An error stanza MUST contain an &lt;error/&gt; child element.", so the xmppError argument is mandatory. super(builder, childElementQName);
* </p> Objects.requireNonNull(builder.getError(), "Must provide an stanza error when building error IQs");
* @param stanzaError the stanzaError (required). this.request = builder.request;
*/ }
public ErrorIQ(StanzaError stanzaError) {
super(ELEMENT, null); public static ErrorIQ createErrorResponse(final IQ request, final StanzaError error) {
Objects.requireNonNull(stanzaError, "stanzaError must not be null"); Builder builder = new Builder(error, request);
setType(IQ.Type.error); builder.setError(error);
setError(stanzaError); return builder.build();
}
@Override
protected IQChildElementXmlStringBuilder getIQChildElementBuilder(IQChildElementXmlStringBuilder xml) {
if (request == null) {
return null;
}
return request.getIQChildElementBuilder(xml);
}
public static Builder builder(StanzaError error) {
return new Builder(error, IqData.EMPTY.ofType(IQ.Type.error));
}
public static Builder builder(StanzaError error, IqData iqData) {
return new Builder(error, iqData);
}
public static final class Builder extends IqBuilder<Builder, ErrorIQ> {
private IQ request;
Builder(StanzaError error, IqData iqData) {
super(iqData);
if (iqData.getType() != IQ.Type.error) {
throw new IllegalArgumentException("Error IQs must be of type 'error'");
}
Objects.requireNonNull(error, "Must provide an stanza error when building error IQs");
setError(error);
}
Builder(StanzaError error, IQ request) {
this(error, AbstractIqBuilder.createErrorResponse(request));
this.request = request;
}
@Override
public Builder getThis() {
return this;
}
@Override
public ErrorIQ build() {
QName childElementQname = null;
if (request != null) {
childElementQname = request.getChildElementQName();
}
return new ErrorIQ(this, childElementQname);
} }
} }
}

View file

@ -68,16 +68,22 @@ public abstract class IQ extends Stanza implements IqView {
} }
protected IQ(AbstractIqBuilder<?> iqBuilder, String childElementName, String childElementNamespace) { protected IQ(AbstractIqBuilder<?> iqBuilder, String childElementName, String childElementNamespace) {
this(iqBuilder, childElementName != null ? new QName(childElementNamespace, childElementName) : null);
}
protected IQ(AbstractIqBuilder<?> iqBuilder, QName childElementQName) {
super(iqBuilder); super(iqBuilder);
type = iqBuilder.type; type = iqBuilder.type;
this.childElementName = childElementName; if (childElementQName != null) {
this.childElementNamespace = childElementNamespace; this.childElementQName = childElementQName;
if (childElementName == null) { this.childElementName = childElementQName.getLocalPart();
childElementQName = null; this.childElementNamespace = childElementQName.getNamespaceURI();
} else { } else {
childElementQName = new QName(childElementNamespace, childElementName); this.childElementQName = null;
this.childElementName = null;
this.childElementNamespace = null;
} }
} }
@ -100,32 +106,6 @@ public abstract class IQ extends Stanza implements IqView {
this.type = Objects.requireNonNull(type, "type must not be null"); this.type = Objects.requireNonNull(type, "type must not be null");
} }
/**
* Return true if this IQ is a request IQ, i.e. an IQ of type {@link Type#get} or {@link Type#set}.
*
* @return true if IQ type is 'get' or 'set', false otherwise.
* @since 4.1
*/
public boolean isRequestIQ() {
switch (type) {
case get:
case set:
return true;
default:
return false;
}
}
/**
* Return true if this IQ is a request, i.e. an IQ of type {@link Type#result} or {@link Type#error}.
*
* @return true if IQ type is 'result' or 'error', false otherwise.
* @since 4.4
*/
public boolean isResponseIQ() {
return !isRequestIQ();
}
public final QName getChildElementQName() { public final QName getChildElementQName() {
return childElementQName; return childElementQName;
} }
@ -194,7 +174,6 @@ public abstract class IQ extends Stanza implements IqView {
if (type == Type.error) { if (type == Type.error) {
// Add the error sub-packet, if there is one. // Add the error sub-packet, if there is one.
appendErrorIfExists(xml); appendErrorIfExists(xml);
return;
} }
if (childElementName == null) { if (childElementName == null) {
return; return;
@ -305,16 +284,7 @@ public abstract class IQ extends Stanza implements IqView {
* @return a new {@link Type#error IQ.Type.error} IQ based on the originating IQ. * @return a new {@link Type#error IQ.Type.error} IQ based on the originating IQ.
*/ */
public static ErrorIQ createErrorResponse(final IQ request, final StanzaError error) { public static ErrorIQ createErrorResponse(final IQ request, final StanzaError error) {
if (!request.isRequestIQ()) { return ErrorIQ.createErrorResponse(request, error);
throw new IllegalArgumentException(
"IQ must be of type 'set' or 'get'. Original IQ: " + request.toXML());
}
final ErrorIQ result = new ErrorIQ(error);
result.setStanzaId(request.getStanzaId());
result.setFrom(request.getTo());
result.setTo(request.getFrom());
return result;
} }
/** /**

View file

@ -1,6 +1,6 @@
/** /**
* *
* Copyright 2019 Florian Schmaus * Copyright 2019-2023 Florian Schmaus
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,6 +16,8 @@
*/ */
package org.jivesoftware.smack.packet; package org.jivesoftware.smack.packet;
import org.jivesoftware.smack.packet.IQ.Type;
public interface IqView extends StanzaView { public interface IqView extends StanzaView {
/** /**
@ -25,4 +27,24 @@ public interface IqView extends StanzaView {
*/ */
IQ.Type getType(); IQ.Type getType();
/**
* Return true if this IQ is a request IQ, i.e. an IQ of type {@link Type#get} or {@link Type#set}.
*
* @return true if IQ type is 'get' or 'set', false otherwise.
* @since 4.1
*/
default boolean isRequestIQ() {
IQ.Type type = getType();
return type == IQ.Type.get || type == IQ.Type.set;
}
/**
* Return true if this IQ is a request, i.e. an IQ of type {@link Type#result} or {@link Type#error}.
*
* @return true if IQ type is 'result' or 'error', false otherwise.
* @since 4.4
*/
default boolean isResponseIQ() {
return !isRequestIQ();
}
} }

View file

@ -1,6 +1,6 @@
/** /**
* *
* Copyright 2003-2007 Jive Software, 2019 Florian Schmaus. * Copyright 2003-2007 Jive Software, 2019-2023 Florian Schmaus.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -579,8 +579,9 @@ public class PacketParserUtils {
switch (iqData.getType()) { switch (iqData.getType()) {
case error: case error:
// If an IQ packet wasn't created above, create an empty error IQ packet. // If an IQ packet wasn't created above, create an empty error IQ packet.
iqPacket = new ErrorIQ(error); iqPacket = ErrorIQ.builder(error, iqData).build();
break; // The following return is simply to avoid setting iqData again below.
return iqPacket;
case result: case result:
iqPacket = new EmptyResultIQ(); iqPacket = new EmptyResultIQ();
break; break;

View file

@ -1,43 +0,0 @@
/**
*
* Copyright © 2014 Florian Schmaus
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jivesoftware.smack;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import org.jivesoftware.smack.packet.IQ;
import org.jivesoftware.smack.packet.TestIQ;
import org.jivesoftware.smack.util.StringUtils;
import org.junit.Test;
public class StanzaIdTest {
@Test
public void testIqId() {
IQ iq1 = new TestIQ();
String iq1Id = iq1.getStanzaId();
assertTrue(StringUtils.isNotEmpty(iq1Id));
IQ iq2 = new TestIQ();
String iq2Id = iq2.getStanzaId();
assertTrue(StringUtils.isNotEmpty(iq2Id));
assertFalse(iq1Id.equals(iq2Id));
}
}

View file

@ -0,0 +1,38 @@
/**
*
* Copyright © 2023 Florian Schmaus
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jivesoftware.smack.packet;
import static org.jivesoftware.smack.test.util.XmlAssertUtil.assertXmlSimilar;
import org.junit.jupiter.api.Test;
public class IqTest {
@Test
public void testIqErrorWithChildElement() {
IQ request = new TestIQ();
StanzaError error = StanzaError.getBuilder().setCondition(StanzaError.Condition.bad_request).build();
ErrorIQ errorIq = IQ.createErrorResponse(request, error);
String expected = "<iq xmlns='jabber:client' id='42' type='error'>"
+ "<error type='modify'><bad-request xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/></error>"
+ "<test-iq xmlns='https://igniterealtime.org/projects/smack'/>"
+ "</iq>";
assertXmlSimilar(expected, errorIq.toXML());
}
}

View file

@ -1,6 +1,6 @@
/** /**
* *
* Copyright © 2014-2019 Florian Schmaus * Copyright © 2014-2023 Florian Schmaus
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -21,11 +21,11 @@ import org.jivesoftware.smack.SmackConfiguration;
public class TestIQ extends SimpleIQ { public class TestIQ extends SimpleIQ {
public TestIQ() { public TestIQ() {
this(SmackConfiguration.SMACK_URL_STRING, "test-iq"); this("test-iq", SmackConfiguration.SMACK_URL_STRING);
} }
public TestIQ(String element, String namespace) { public TestIQ(String element, String namespace) {
super(element, namespace); super(StanzaBuilder.buildIqData("42"), element, namespace);
} }
@Override @Override

View file

@ -40,11 +40,10 @@ public class IBBPacketUtils {
*/ */
public static IQ createErrorIQ(Jid from, Jid to, StanzaError.Condition condition) { public static IQ createErrorIQ(Jid from, Jid to, StanzaError.Condition condition) {
StanzaError xmppError = StanzaError.getBuilder(condition).build(); StanzaError xmppError = StanzaError.getBuilder(condition).build();
IQ errorIQ = new ErrorIQ(xmppError); return ErrorIQ.builder(xmppError)
errorIQ.setType(IQ.Type.error); .from(from)
errorIQ.setFrom(from); .to(to)
errorIQ.setTo(to); .build();
return errorIQ;
} }
/** /**

View file

@ -38,12 +38,11 @@ import org.jivesoftware.smack.SmackException.FeatureNotSupportedException;
import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPConnection;
import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.XMPPException;
import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.XMPPException.XMPPErrorException;
import org.jivesoftware.smack.packet.ErrorIQ;
import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.IQ;
import org.jivesoftware.smack.packet.StanzaError; import org.jivesoftware.smack.packet.StanzaError;
import org.jivesoftware.smack.test.util.NetworkUtil; import org.jivesoftware.smack.test.util.NetworkUtil;
import org.jivesoftware.smack.util.ExceptionUtil; import org.jivesoftware.smack.util.ExceptionUtil;
import org.jivesoftware.smackx.bytestreams.ibb.IBBPacketUtils;
import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream; import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream;
import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream.StreamHost; import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream.StreamHost;
import org.jivesoftware.smackx.disco.ServiceDiscoveryManager; import org.jivesoftware.smackx.disco.ServiceDiscoveryManager;
@ -415,10 +414,7 @@ public class Socks5ByteStreamManagerTest {
Verification.requestTypeGET); Verification.requestTypeGET);
// build error packet to reject SOCKS5 Bytestream // build error packet to reject SOCKS5 Bytestream
StanzaError stanzaError = StanzaError.getBuilder(StanzaError.Condition.not_acceptable).build(); IQ rejectPacket = IBBPacketUtils.createErrorIQ(targetJID, initiatorJID, StanzaError.Condition.not_acceptable);
IQ rejectPacket = new ErrorIQ(stanzaError);
rejectPacket.setFrom(targetJID);
rejectPacket.setTo(initiatorJID);
// return error packet as response to the bytestream initiation // return error packet as response to the bytestream initiation
protocol.addResponse(rejectPacket, Verification.correspondingSenderReceiver, protocol.addResponse(rejectPacket, Verification.correspondingSenderReceiver,

View file

@ -30,11 +30,10 @@ import org.jivesoftware.smack.SmackException;
import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPConnection;
import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.XMPPException.XMPPErrorException;
import org.jivesoftware.smack.packet.EmptyResultIQ; import org.jivesoftware.smack.packet.EmptyResultIQ;
import org.jivesoftware.smack.packet.ErrorIQ;
import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.IQ;
import org.jivesoftware.smack.packet.StanzaError; import org.jivesoftware.smack.packet.StanzaError;
import org.jivesoftware.smack.util.CloseableUtil; import org.jivesoftware.smack.util.CloseableUtil;
import org.jivesoftware.smackx.bytestreams.ibb.IBBPacketUtils;
import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream; import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream;
import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream.StreamHost; import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream.StreamHost;
@ -184,9 +183,7 @@ public class Socks5ClientForInitiatorTest {
XMPPConnection connection = ConnectionUtils.createMockedConnection(protocol, initiatorJID); XMPPConnection connection = ConnectionUtils.createMockedConnection(protocol, initiatorJID);
// build error response as reply to the stream activation // build error response as reply to the stream activation
IQ error = new ErrorIQ(StanzaError.getBuilder(StanzaError.Condition.internal_server_error).build()); IQ error = IBBPacketUtils.createErrorIQ(proxyJID, initiatorJID, StanzaError.Condition.internal_server_error);
error.setFrom(proxyJID);
error.setTo(initiatorJID);
protocol.addResponse(error, Verification.correspondingSenderReceiver, protocol.addResponse(error, Verification.correspondingSenderReceiver,
Verification.requestTypeSET); Verification.requestTypeSET);