Address silent failure of unrecognized URIs in Attach Link to URI

Generated files for a more robust attached-link-dialog and localized strings
* AttachLink.js
* AttachLink.xul

zotero/xpcom/attachments.js
* created function cleanAttachmentURI
* in function linkFromURL, removed the regex constraints and the comment list of valid protocols
* removed outdated function declaration from beginning of script
* Improved automatic title generation mechanism

chrome/content/zotero/attachLink.js

@@ -0,0 +1,60 @@
+/*
+    ***** BEGIN LICENSE BLOCK *****
+    
+    Copyright © 2014 Center for History and New Media
+                     George Mason University, Fairfax, Virginia, USA
+                     http://zotero.org
+    
+    This file is part of Zotero.
+    
+    Zotero is free software: you can redistribute it and/or modify
+    it under the terms of the GNU Affero General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+    
+    Zotero is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU Affero General Public License for more details.
+    
+    You should have received a copy of the GNU Affero General Public License
+    along with Zotero.  If not, see <http://www.gnu.org/licenses/>.
+    
+    ***** END LICENSE BLOCK *****
+*/
+
+var Zotero_AttachLink = new function() {
+	function getAttachFileLabel() {
+		return window.opener.document
+			.getElementById('zotero-tb-attachment-add-file-link')
+			.label;
+	};
+
+	this.submit = function() {
+		var link = document.getElementById('zotero-attach-uri-input').value;
+		var message = document.getElementById('zotero-attach-uri-message');
+		var cleanURI = Zotero.Attachments.cleanAttachmentURI(link, true);
+		
+		if (!cleanURI) {
+			message.textContent = Zotero.getString('pane.items.attach.link.uri.unrecognized');
+			window.sizeToContent();
+			document.getElementById('zotero-attach-uri-input').select();
+			return false;
+		}
+		// Don't allow "file:" links, because using "Attach link to file" is the right way
+		else if (cleanURI.toLowerCase().indexOf('file:') == 0) {
+			message.textContent = Zotero.getString('pane.items.attach.link.uri.file',
+				[getAttachFileLabel()]);
+			window.sizeToContent();
+			document.getElementById('zotero-attach-uri-input').select();
+			return false;
+		}
+		else {
+			window.arguments[0].out = {
+				link:	cleanURI,
+				title:	document.getElementById('zotero-attach-uri-title').value
+			};
+			return true;
+		}
+	};
+}

chrome/content/zotero/attachLink.xul

@@ -0,0 +1,32 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<?xml-stylesheet href="chrome://zotero/skin/zotero.css" type="text/css"?>
+
+<!DOCTYPE window SYSTEM "chrome://zotero/locale/zotero.dtd">
+
+<dialog
+	xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+	buttons="accept,cancel"
+	ondialogaccept="return Zotero_AttachLink.submit();"
+	
+	id="zotero-attach-uri-dialog"
+	title="&zotero.attachLink.title;"
+	>
+	
+	<script src="include.js"/>
+	<script src="AttachLink.js"/>
+	
+	<vbox id="zotero-attach-uri-container">
+		<hbox>
+			<description id="zotero-attach-uri-message" class="zotero-message-error"></description>
+		</hbox>
+		<hbox align="center">
+			<label id="zotero-attach-uri-label-input" value="&zotero.attachLink.label.link;" control="zotero-attach-uri-input"></label>
+			<textbox id="zotero-attach-uri-input" flex="1"/>
+		</hbox>
+		<hbox align="center">
+			<label id="zotero-attach-uri-label-title" value="&zotero.attachLink.label.title;" control="zotero-attach-uri-title"></label>
+			<textbox id="zotero-attach-uri-title" flex="1" placeholder="&zotero.general.optional;"/>
+		</hbox>
+	</vbox>
+</dialog>

chrome/content/zotero/xpcom/attachments.js

@@ -34,7 +34,6 @@ Zotero.Attachments = new function(){
 	this.linkFromFile = linkFromFile;
 	this.importSnapshotFromFile = importSnapshotFromFile;
 	this.importFromURL = importFromURL;
-	this.linkFromURL = linkFromURL;
 	this.linkFromDocument = linkFromDocument;
 	this.importFromDocument = importFromDocument;
 	this.createMissingAttachment = createMissingAttachment;
@@ -399,38 +398,68 @@ Zotero.Attachments = new function(){
 	}
 	
 	
+	this.cleanAttachmentURI = function (uri, tryHttp) {
+		uri = uri.trim();
+		if (!uri) return false;
+		
+		var ios = Components.classes["@mozilla.org/network/io-service;1"]
+			.getService(Components.interfaces.nsIIOService);
+		try {
+			return ios.newURI(uri, null, null).spec // Valid URI if succeeds
+		} catch (e) {
+			if (e instanceof Components.Exception
+				&& e.result == Components.results.NS_ERROR_MALFORMED_URI
+			) {
+				if (tryHttp && /\w\.\w/.test(uri)) {
+					// Assume it's a URL missing "http://" part
+					try {
+						return ios.newURI('http://' + uri, null, null).spec;
+					} catch (e) {}
+				}
+				
+				Zotero.debug('cleanAttachmentURI: Invalid URI: ' + uri, 2);
+				return false;
+			}
+			throw e;
+		}
+	}
+	
+	
 	/*
 	 * Create a link attachment from a URL
 	 *
-	 * @param	{String}		url
+	 * @param	{String}		url Validated URI
 	 * @param	{Integer}		sourceItemID	Parent item
 	 * @param	{String}		[mimeType]		MIME type of page
 	 * @param	{String}		[title]			Title to use for attachment
 	 */
-	function linkFromURL(url, sourceItemID, mimeType, title){
-		Zotero.debug('Linking attachment from URL');
-	    
-		/* Throw error on invalid URLs
-		   We currently accept the following protocols:
-		   PersonalBrain (brain://)
-		   DevonThink (x-devonthink-item://)
-		   Notational Velocity (nv://)
-		   MyLife Organized (mlo://)
-		   Evernote (evernote://)
-		   OneNote (onenote://)
-		   Kindle (kindle://) 
-		   Logos (logosres:) 
-		   Zotero (zotero://) */
-
-		var urlRe = /^((https?|zotero|evernote|onenote|brain|nv|mlo|kindle|x-devonthink-item|ftp):\/\/|logosres:)[^\s]*$/;
-		var matches = urlRe.exec(url);
-		if (!matches) {
-			throw ("Invalid URL '" + url + "' in Zotero.Attachments.linkFromURL()");
-		}
+	this.linkFromURL = function (url, sourceItemID, mimeType, title) {
+		Zotero.debug('Linking attachment from ' + url);
 		
 		// If no title provided, figure it out from the URL
-		if (!title){
-			title = url.substring(url.lastIndexOf('/')+1);
+		// Web addresses with paths will be whittled to the last element
+		// excluding references and queries. All others are the full string
+		if (!title) {
+			var ioService = Components.classes["@mozilla.org/network/io-service;1"]
+				.getService(Components.interfaces.nsIIOService);
+			var titleURL = ioService.newURI(url, null, null);
+
+			if (titleURL.scheme == 'http' || titleURL.scheme == 'https') {
+				titleURL = titleURL.QueryInterface(Components.interfaces.nsIURL);
+				if (titleURL.path == '/') {
+					title = titleURL.host;
+				}
+				else if (titleURL.fileName) {
+					title = titleURL.fileName;
+				}
+				else {
+					var dir = titleURL.directory.split('/');
+					title = dir[dir.length - 2];
+				}
+			}
+			else {
+				title = url;
+			}
 		}
 		
 		// Override MIME type to application/pdf if extension is .pdf --
@@ -446,7 +475,6 @@ Zotero.Attachments = new function(){
 		return itemID;
 	}
 	
-	
 	// TODO: what if called on file:// document?
 	function linkFromDocument(document, sourceItemID, parentCollectionIDs){
 		Zotero.debug('Linking attachment from document');

chrome/content/zotero/xpcom/translation/translate_item.js

@@ -217,29 +217,42 @@ Zotero.Translate.ItemSaver.prototype = {
 	},
 	
 	"_saveAttachmentFile":function(attachment, parentID, attachmentCallback) {
-		const urlRe = /(([a-z][-+\.a-z0-9]*):\/\/[^\s]*)/i; //according to RFC3986
 		Zotero.debug("Translate: Adding attachment", 4);
 			
 		if(!attachment.url && !attachment.path) {
-			Zotero.debug("Translate: Ignoring attachment: no path or URL specified", 2);
+			let e = "Translate: Ignoring attachment: no path or URL specified";
+			Zotero.debug(e, 2);
+			attachmentCallback(attachment, false, e);
 			return false;
 		}
 		
 		if(!attachment.path) {
+			let url = Zotero.Attachments.cleanAttachmentURI(attachment.url);
+			if (!url) {
+				let e = "Translate: Invalid attachment URL specified <" + attachment.url + ">";
+				Zotero.debug(e, 2);
+				attachmentCallback(attachment, false, e);
+				return false;
+			}
+			attachment.url = url;
+			url = Components.classes["@mozilla.org/network/io-service;1"]
+				.getService(Components.interfaces.nsIIOService)
+				.newURI(url, null, null); // This cannot fail, since we check above
+			
 			// see if this is actually a file URL
-			var m = urlRe.exec(attachment.url);
-			var protocol = m ? m[2].toLowerCase() : "file";
-			if(protocol == "file") {
+			if(url.scheme == "file") {
 				attachment.path = attachment.url;
 				attachment.url = false;
-			} else if(protocol != "http" && protocol != "https") {
-				Zotero.debug("Translate: Unrecognized protocol "+protocol, 2);
+			} else if(url.scheme != "http" && url.scheme != "https") {
+				let e = "Translate: " + url.scheme + " protocol is not allowed for attachments from translators.";
+				Zotero.debug(e, 2);
+				attachmentCallback(attachment, false, e);
 				return false;
 			}
 		}
 		
 		if(!attachment.path) {
-			// create from URL
+			// At this point, must be a valid HTTP/HTTPS url
 			attachment.linkMode = "linked_file";
 			try {
 				var myID = Zotero.Attachments.linkFromURL(attachment.url, parentID,
@@ -412,33 +425,43 @@ Zotero.Translate.ItemSaver.prototype = {
 			if(attachment.snapshot === false || !this._saveFiles) {
 				// if snapshot is explicitly set to false, attach as link
 				attachment.linkMode = "linked_url";
+				let url, mimeType;
 				if(attachment.document) {
-					try {
-						Zotero.Attachments.linkFromURL(attachment.document.location.href, parentID,
-								(attachment.mimeType ? attachment.mimeType : attachment.document.contentType),
-								title);
-						attachmentCallback(attachment, 100);
-					} catch(e) {
-						Zotero.debug("Translate: Error adding attachment "+attachment.url, 2);
-						attachmentCallback(attachment, false, e);
-					}
-					return true;
+					url = attachment.document.location.href;
+					mimeType = attachment.mimeType ? attachment.mimeType : attachment.document.contentType;
 				} else {
-					if(!attachment.mimeType || !title) {
-						Zotero.debug("Translate: Either mimeType or title is missing; attaching file will be slower", 3);
-					}
-					
-					try {
-						Zotero.Attachments.linkFromURL(attachment.url, parentID,
-								(attachment.mimeType ? attachment.mimeType : undefined),
-								title);
-						attachmentCallback(attachment, 100);
-					} catch(e) {
-						Zotero.debug("Translate: Error adding attachment "+attachment.url, 2);
-						attachmentCallback(attachment, false, e);
-					}
-					return true;
+					url = attachment.url
+					mimeType = attachment.mimeType ? attachment.mimeType : undefined;
 				}
+				
+				let cleanURI = Zotero.Attachments.cleanAttachmentURI(url);
+				if (!cleanURI) {
+					let e = "Translate: Invalid attachment URL specified <" + url + ">";
+					Zotero.debug(e, 2);
+					attachmentCallback(attachment, false, e);
+					return false;
+				}
+				url = Components.classes["@mozilla.org/network/io-service;1"]
+					.getService(Components.interfaces.nsIIOService)
+					.newURI(cleanURI, null, null); // This cannot fail, since we check above
+				
+				// Only HTTP/HTTPS links are allowed
+				if(url.scheme != "http" && url.scheme != "https") {
+					let e = "Translate: " + url.scheme + " protocol is not allowed for attachments from translators.";
+					Zotero.debug(e, 2);
+					attachmentCallback(attachment, false, e);
+					return false;
+				}
+				
+				try {
+					Zotero.Attachments.linkFromURL(cleanURI, parentID, mimeType, title);
+					attachmentCallback(attachment, 100);
+				} catch(e) {
+					Zotero.debug("Translate: Error adding attachment "+attachment.url, 2);
+					attachmentCallback(attachment, false, e);
+					return false;
+				}
+				return true;
 			} else {
 				// if snapshot is not explicitly set to false, retrieve snapshot
 				if(attachment.document) {

chrome/content/zotero/zoteroPane.js

@@ -3038,19 +3038,13 @@ var ZoteroPane = new function()
 			this.displayCannotEditLibraryMessage();
 			return;
 		}
-		var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
-					.getService(Components.interfaces.nsIPromptService);
 		
-		var input = {};
-		var check = {value : false};
-		
-		// TODO: Allow title to be specified?
-		var result = ps.prompt(null, Zotero.getString('pane.items.attach.link.uri.title'),
-			Zotero.getString('pane.items.attach.link.uri'), input, "", {});
-		if (!result || !input.value) return false;
-		
-		// Create a new attachment
-		Zotero.Attachments.linkFromURL(input.value, itemID);
+		var io = {};
+		window.openDialog('chrome://zotero/content/attachLink.xul', 
+			'zotero-attach-uri-dialog', 'centerscreen, modal', io);
+		if (io.out) {
+			Zotero.Attachments.linkFromURL(io.out.link, itemID, null, io.out.title);
+		}
 	}
 	
 	

chrome/content/zotero/zoteroPane.xul

@@ -174,7 +174,7 @@
 							<menuitem class="menuitem-iconic zotero-menuitem-attachments-web-link standalone-no-display" label="&zotero.items.menu.attach.link;" oncommand="var itemID = ZoteroPane_Local.getSelectedItems()[0].id; ZoteroPane_Local.addAttachmentFromPage(true, itemID)"/>
 							<menuitem class="menuitem-iconic zotero-menuitem-attachments-web-link" label="&zotero.items.menu.attach.link.uri;" oncommand="var itemID = ZoteroPane_Local.getSelectedItems()[0].id; ZoteroPane_Local.addAttachmentFromURI(true, itemID);"/>
 							<menuitem class="menuitem-iconic zotero-menuitem-attachments-file" label="&zotero.items.menu.attach.file;" oncommand="var itemID = ZoteroPane_Local.getSelectedItems()[0].id; ZoteroPane_Local.addAttachmentFromDialog(false, itemID);"/>
-							<menuitem class="menuitem-iconic zotero-menuitem-attachments-link" label="&zotero.items.menu.attach.fileLink;" oncommand="var itemID = ZoteroPane_Local.getSelectedItems()[0].id; ZoteroPane_Local.addAttachmentFromDialog(true, itemID);"/>
+							<menuitem class="menuitem-iconic zotero-menuitem-attachments-link" label="&zotero.items.menu.attach.fileLink;" oncommand="var itemID = ZoteroPane_Local.getSelectedItems()[0].id; ZoteroPane_Local.addAttachmentFromDialog(true, itemID);" id="zotero-tb-attachment-add-file-link"/>
 						</menupopup>
 					</toolbarbutton>
 					<toolbarseparator/>

chrome/locale/en-US/zotero/zotero.dtd

@@ -283,4 +283,10 @@
 
 <!ENTITY zotero.downloadManager.label			"Save to Zotero">
 <!ENTITY zotero.downloadManager.saveToLibrary.description	"Attachments cannot be saved to the currently selected library. This item will be saved to your library instead.">
-<!ENTITY zotero.downloadManager.noPDFTools.description	"To use this feature, you must first install the PDF tools in the Search pane of the Zotero preferences.">
+<!ENTITY zotero.downloadManager.noPDFTools.description	"To use this feature, you must first install the PDF tools in the Search pane of the Zotero preferences.">
+
+<!ENTITY zotero.attachLink.title                     "Attach Link to URI">
+<!ENTITY zotero.attachLink.label.link                "Link:">
+<!ENTITY zotero.attachLink.label.title               "Title:">
+
+

chrome/locale/en-US/zotero/zotero.properties

@@ -195,8 +195,8 @@ tagColorChooser.maxTags                        = Up to %S tags in each library c
 pane.items.loading				= Loading items list…
 pane.items.columnChooser.moreColumns = More Columns
 pane.items.columnChooser.secondarySort = Secondary Sort (%S)
-pane.items.attach.link.uri.title	 	= Attach Link to URI
-pane.items.attach.link.uri		= Enter a URI:
+pane.items.attach.link.uri.unrecognized	      = Zotero did not recognize the URI you entered. Please check the address and try again.
+pane.items.attach.link.uri.file               = To attach a link to a file, please use “%S”.
 pane.items.trash.title			= Move to Trash
 pane.items.trash				= Are you sure you want to move the selected item to the Trash?
 pane.items.trash.multiple		= Are you sure you want to move the selected items to the Trash?

chrome/skin/default/zotero/zotero.css

@@ -242,6 +242,10 @@ label.zotero-text-link {
 	background: rgb(89, 139, 236);
 }
 
+.zotero-message-error
+{
+	font-weight: bold;
+}
 
 #zotero-progress-box
 {
@@ -354,4 +358,16 @@ label.zotero-text-link {
 #zotero-advanced-search-dialog #zotero-search-buttons
 {
 	margin: 3px 0;
+}
+
+#zotero-attach-uri-container
+{
+	width: 30em;
+	max-width: 30em;
+}
+
+#zotero-attach-uri-message
+{
+	width: 29.5em;
+	max-width: 29.5em;
 }