395095: failed to download Bugzilla attachment

Change-Id: I94895c780e8829b2dfe80b1c3df2aa11841c8aad
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=395095
diff --git a/org.eclipse.mylyn.bugzilla.tests/src/org/eclipse/mylyn/bugzilla/tests/BugzillaAttachmentHandlerTest.java b/org.eclipse.mylyn.bugzilla.tests/src/org/eclipse/mylyn/bugzilla/tests/BugzillaAttachmentHandlerTest.java
index 3a10423..c66e548 100644
--- a/org.eclipse.mylyn.bugzilla.tests/src/org/eclipse/mylyn/bugzilla/tests/BugzillaAttachmentHandlerTest.java
+++ b/org.eclipse.mylyn.bugzilla.tests/src/org/eclipse/mylyn/bugzilla/tests/BugzillaAttachmentHandlerTest.java
@@ -506,4 +506,5 @@
 		}
 		fail("Should have failed due to invalid userid and password.");
 	}
+
 }
diff --git a/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/ui/AttachmentUtilTest.java b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/ui/AttachmentUtilTest.java
new file mode 100644
index 0000000..b46d9a6
--- /dev/null
+++ b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/ui/AttachmentUtilTest.java
@@ -0,0 +1,131 @@
+/*******************************************************************************
+ * Copyright (c) 2012 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.tasks.tests.ui;
+
+import junit.framework.TestCase;
+
+import org.eclipse.core.runtime.AssertionFailedException;
+import org.eclipse.mylyn.internal.tasks.core.TaskAttachment;
+import org.eclipse.mylyn.internal.tasks.ui.util.AttachmentUtil;
+import org.eclipse.mylyn.tasks.tests.TaskTestUtil;
+
+/**
+ * @author Steffen Pingel
+ * @author Frank Becker
+ */
+public class AttachmentUtilTest extends TestCase {
+
+	TaskAttachment attachment = TaskTestUtil.createMockTaskAttachment("id");
+
+	public void testGetAttachmentFilenameNull() {
+		try {
+			AttachmentUtil.getAttachmentFilename(null);
+			fail("Expected AssertionFailedException");
+		} catch (AssertionFailedException expected) {
+
+		}
+	}
+
+	public void testGetAttachmentFilename() {
+		attachment.setFileName("file.bmp");
+		assertEquals("file.bmp", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameNoExtension() {
+		attachment.setFileName("file");
+		assertEquals("file", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmpty() {
+		attachment.setFileName("");
+		assertEquals("attachment", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeHtml() {
+		attachment.setFileName("");
+		attachment.setContentType("html");
+		assertEquals("attachment.html", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeText() {
+		attachment.setFileName("");
+		attachment.setContentType("text");
+		assertEquals("attachment.txt", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeZip() {
+		attachment.setFileName("");
+		attachment.setContentType("zip");
+		assertEquals("attachment.zip", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeOctetStream() {
+		attachment.setFileName("");
+		attachment.setContentType("octet-stream");
+		assertEquals("attachment", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeApplicationOctetStream() {
+		attachment.setFileName("");
+		attachment.setContentType("application/octet-stream");
+		assertEquals("attachment.octet-stream", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeImagePng() {
+		attachment.setFileName("");
+		attachment.setContentType("image/png");
+		assertEquals("attachment.png", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeImageJpeg() {
+		attachment.setFileName("");
+		attachment.setContentType("image/jpeg");
+		assertEquals("attachment.jpeg", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeImageGif() {
+		attachment.setFileName("");
+		attachment.setContentType("image/gif");
+		assertEquals("attachment.gif", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeTextPlain() {
+		attachment.setFileName("");
+		attachment.setContentType("text/plain");
+		assertEquals("attachment.txt", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeTextHtml() {
+		attachment.setFileName("");
+		attachment.setContentType("text/html");
+		assertEquals("attachment.html", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameEmptyContentTypeApplicationXml() {
+		attachment.setFileName("");
+		attachment.setContentType("application/xml");
+		assertEquals("attachment.xml", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentFilenameTestHtmlContentTypeOctetStream() {
+		attachment.setFileName("Test.html");
+		attachment.setContentType("octet-stream");
+		assertEquals("Test.html", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+	public void testGetAttachmentIlegalFilename() {
+		attachment.setFileName("Ilegal:File:Name");
+		attachment.setContentType("octet-stream");
+		assertEquals("Ilegal%3AFile%3AName", AttachmentUtil.getAttachmentFilename(attachment));
+	}
+
+}
diff --git a/org.eclipse.mylyn.tasks.ui/src/org/eclipse/mylyn/internal/tasks/ui/util/AttachmentUtil.java b/org.eclipse.mylyn.tasks.ui/src/org/eclipse/mylyn/internal/tasks/ui/util/AttachmentUtil.java
index 3871082..e90fb4f 100644
--- a/org.eclipse.mylyn.tasks.ui/src/org/eclipse/mylyn/internal/tasks/ui/util/AttachmentUtil.java
+++ b/org.eclipse.mylyn.tasks.ui/src/org/eclipse/mylyn/internal/tasks/ui/util/AttachmentUtil.java
@@ -34,6 +34,7 @@
 import org.eclipse.jface.operation.IRunnableWithProgress;
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.jface.viewers.IStructuredSelection;
+import org.eclipse.mylyn.commons.core.CoreUtil;
 import org.eclipse.mylyn.commons.core.StatusHandler;
 import org.eclipse.mylyn.commons.net.Policy;
 import org.eclipse.mylyn.internal.tasks.core.ITasksCoreConstants;
@@ -76,8 +77,6 @@
 
 	private static final String CTYPE_ZIP = "zip"; //$NON-NLS-1$
 
-	private static final String CTYPE_OCTET_STREAM = "octet-stream"; //$NON-NLS-1$
-
 	private static final String CTYPE_TEXT = "text"; //$NON-NLS-1$
 
 	private static final String CTYPE_HTML = "html"; //$NON-NLS-1$
@@ -366,24 +365,33 @@
 	}
 
 	public static String getAttachmentFilename(ITaskAttachment attachment) {
+		Assert.isNotNull(attachment);
 		String name = attachment.getFileName();
-		// if no filename is set, make one up with the proper extension so
-		// we can support opening in that filetype's default editor
-		if (name == null || "".equals(name)) { //$NON-NLS-1$
-			String ctype = attachment.getContentType();
-			if (ctype.endsWith(CTYPE_HTML)) {
-				name = ATTACHMENT_DEFAULT_NAME + ".html"; //$NON-NLS-1$
-			} else if (ctype.startsWith(CTYPE_TEXT)) {
-				name = ATTACHMENT_DEFAULT_NAME + ".txt"; //$NON-NLS-1$
-			} else if (ctype.endsWith(CTYPE_OCTET_STREAM)) {
-				name = ATTACHMENT_DEFAULT_NAME;
-			} else if (ctype.endsWith(CTYPE_ZIP)) {
-				name = ATTACHMENT_DEFAULT_NAME + "." + CTYPE_ZIP; //$NON-NLS-1$
-			} else {
-				name = ATTACHMENT_DEFAULT_NAME + "." + ctype.substring(ctype.indexOf("/") + 1); //$NON-NLS-1$ //$NON-NLS-2$
-			}
-		}
+		if (name != null && name.length() > 0) {
+			return CoreUtil.asFileName(name);
+		} else {
+			String extension = ""; //$NON-NLS-1$
 
-		return name;
+			// if no filename is set, make one up with the proper extension so
+			// we can support opening in that filetype's default editor
+			String ctype = attachment.getContentType();
+			if (ctype != null) {
+				if (ctype.endsWith(CTYPE_HTML)) {
+					extension = ".html"; //$NON-NLS-1$
+				} else if (ctype.startsWith(CTYPE_TEXT)) {
+					extension = ".txt"; //$NON-NLS-1$
+				} else if (ctype.endsWith(CTYPE_ZIP)) {
+					extension = ".zip"; //$NON-NLS-1$
+				} else {
+					int i = ctype.lastIndexOf("/"); //$NON-NLS-1$
+					if (i != -1) {
+						extension = "." + ctype.substring(i + 1); //$NON-NLS-1$
+					}
+				}
+			}
+
+			return ATTACHMENT_DEFAULT_NAME + extension;
+		}
 	}
+
 }