Merge branch 'issues/230'

Author: Lloyd Watkin

src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/PubSubElementProcessorAbstract.java

@@ -108,13 +108,25 @@ protected void setErrorCondition(Type type, Condition condition) {
 
 	protected void createExtendedErrorReply(Type type, Condition condition,
 			String additionalElement) {
+		if ((null != additionalElement) && (additionalElement.indexOf(" ") > -1)) {
+			// Its probably an error message!
+			createExtendedErrorReply(type, condition, null, null, additionalElement);
+			return;
+		}
 		createExtendedErrorReply(type, condition, additionalElement,
 				JabberPubsub.NS_PUBSUB_ERROR);
 	}
 
 	protected void createExtendedErrorReply(Type type, Condition condition,
 			String additionalElement, String additionalNamespace) {
-		
+		createExtendedErrorReply(type, condition, additionalElement,
+				additionalNamespace, null);
+	}
+	
+	
+	protected void createExtendedErrorReply(Type type, Condition condition,
+			String additionalElement, String additionalNamespace, String text) {
+	
 		if (null == response) {
 			response = IQ.createResultIQ(request);
 		}
@@ -127,6 +139,14 @@ protected void createExtendedErrorReply(Type type, Condition condition,
 		error.addAttribute("type", type.toXMPP());
 		error.add(standardError);
 		error.add(extraError);
+		if (null != text) {
+			Element description = new DOMElement(
+				"text",
+				new org.dom4j.Namespace("", "urn:ietf:params:xml:ns:xmpp-stanzas")
+			);
+			description.setText(text);
+			error.add(description);
+		}
 		response.setChildElement(error);
 	}
 

src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/UserItemsGet.java

@@ -73,10 +73,10 @@ public void process(Element elm, JID actorJID, IQ reqIQ, Element rsm)
 		pubsub = response.getElement().addElement("pubsub",
 				JabberPubsub.NAMESPACE_URI);
 		try {
-			parseRsmElement();
-			addRecentItems();
-			addRsmElement();
-			outQueue.put(response);
+			if (true == parseRsmElement()) {
+				addRecentItems();
+				addRsmElement();
+			}
 		} catch (NodeStoreException e) {
 			LOGGER.error(e);
 			response.getElement().remove(pubsub);
@@ -87,9 +87,9 @@ public void process(Element elm, JID actorJID, IQ reqIQ, Element rsm)
 
 	}
 
-	private void parseRsmElement() {
+	private boolean parseRsmElement() {
 		if (null == resultSetManagement) {
-			return;
+			return true;
 		}
 
 		Element max = null;
@@ -104,11 +104,15 @@ private void parseRsmElement() {
 						.getTextTrim());
 			} catch (IllegalArgumentException e) {
 				LOGGER.error(e);
-				createExtendedErrorReply(Type.modify, Condition.bad_request,
-						"Could not parse the 'after' id: " + after);
-				return;
+				createExtendedErrorReply(
+						Type.modify,
+						Condition.bad_request,
+						"Could not parse the 'after' id: "
+								+ after.getTextTrim());
+				return false;
 			}
 		}
+		return true;
 	}
 
 	private void addRsmElement() throws NodeStoreException {

src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/special/FirehoseGet.java

@@ -102,14 +102,17 @@ private void determineAdminUserStatus() {
 
 	private void parseRsmElement() {
 		Element rsmElement = pubsub.element("set");
-		if (null == rsmElement)
+		if (null == rsmElement) {
 			return;
+		}
 		Element max;
 		Element after;
-		if (null != (max = rsmElement.element("max")))
+		if (null != (max = rsmElement.element("max"))) {
 			maxResults = Integer.parseInt(max.getTextTrim());
-		if (null != (after = rsmElement.element("after")))
+		}
+		if (null != (after = rsmElement.element("after"))) {
 			afterItemId = after.getTextTrim();
+		}
 	}
 
 	private void addRsmElement() throws NodeStoreException {

src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/UserItemsGetTest.java

@@ -412,4 +412,44 @@ public void whenRequestingParentOnlyCorrectFlagIsSetOnDatabaseRequest()
 				Mockito.anyInt(), Mockito.any(GlobalItemID.class),
 				Mockito.eq(true));
 	}
+	
+	/**
+	 * Issue #230
+	 */
+	@Test
+	public void badRsmValueReturnsValidError() throws Exception {
+		
+		String after = "/user/demo@buddycloud.com/posts/example.com|94948";
+		Element rsm = new BaseElement(new QName("set", new Namespace("",
+				"http://jabber.org/protocol/rsm")));
+		rsm.addElement("after")
+		    .setText(after);
+		
+		String stanza = "<iq type=\"get\" to=\"channels.buddycloud.org\" " +
+		    "from=\"test.user@buddycloud.org/resource\" id=\"id:1\">" +
+			"<pubsub xmlns=\"http://jabber.org/protocol/pubsub\">" +
+		    "<user-items xmlns=\"http://buddycloud.org/v1\" parent-only=\"true\" " +
+			"since=\"2000-01-01T00:00:00.000Z\"/>" +
+		    "<set xmlns=\"http://jabber.org/protocol/rsm\">" +
+                "<max>10</max>" +
+                "<after>/user/demo@buddycloud.com/posts/example.com|94948</after>" +
+            "</set>" +
+            "</pubsub>" +
+        "</iq>";
+		IQ request = this.toIq(stanza);
+		
+		userItemsGet.process(element, jid, request, rsm);
+		Assert.assertEquals(1, queue.size());
+		
+		IQ response = (IQ) queue.poll();
+		PacketError errorPacket = response.getError();
+		Assert.assertEquals(
+			"Could not parse the 'after' id: " + after,
+			errorPacket.getText()
+		);
+		Assert.assertEquals(PacketError.Condition.bad_request, errorPacket.getCondition());
+		Assert.assertEquals(PacketError.Type.modify, errorPacket.getType());
+		
+		
+	}
 }