Fixed bug 123787: [Patch] Apply Patch error handling for invalid formats

3 files changed, 117 insertions, 105 deletions

diff --git a/bundles/org.eclipse.compare.core/src/org/eclipse/compare/internal/core/patch/PatchReader.java b/bundles/org.eclipse.compare.core/src/org/eclipse/compare/internal/core/patch/PatchReader.java
index 1d38288bc..05b85eec7 100644
--- a/bundles/org.eclipse.compare.core/src/org/eclipse/compare/internal/core/patch/PatchReader.java
+++ b/bundles/org.eclipse.compare.core/src/org/eclipse/compare/internal/core/patch/PatchReader.java
@@ -264,73 +264,67 @@ public class PatchReader {
 		try {
 			// read lines of hunk
 			while (true) {
-				
+
 				line= reader.readLine();
 				if (line == null)
 					return null;
-					
+
 				if (reader.lineContentLength(line) == 0) {
 					//System.out.println("Warning: found empty line in hunk; ignored");
 					//lines.add(' ' + line);
 					continue;
 				}
-				
+
 				char c= line.charAt(0);
 				switch (c) {
-				case '@':
-					if (line.startsWith("@@ ")) { //$NON-NLS-1$
-						// flush old hunk
-						if (lines.size() > 0) {
-							Hunk.createHunk(diff, oldRange, newRange, lines,encounteredPlus, encounteredMinus, encounteredSpace);
-							lines.clear();
+					case '@':
+						if (line.startsWith("@@ ")) { //$NON-NLS-1$
+							// flush old hunk
+							if (lines.size() > 0) {
+								Hunk.createHunk(diff, oldRange, newRange, lines, encounteredPlus, encounteredMinus, encounteredSpace);
+								lines.clear();
+							}
+
+							// format: @@ -oldStart,oldLength +newStart,newLength @@
+							extractPair(line, '-', oldRange);
+							extractPair(line, '+', newRange);
+							continue;
 						}
-								
-						// format: @@ -oldStart,oldLength +newStart,newLength @@
-						extractPair(line, '-', oldRange);
-						extractPair(line, '+', newRange);
+						break;
+					case ' ':
+						encounteredSpace= true;
+						lines.add(line);
 						continue;
-					}
-					break;
-				case ' ':
-					encounteredSpace = true;
-					lines.add(line);
-					continue;
-				case '+':
-					encounteredPlus = true;
-					lines.add(line);
-					continue;
-				case '-':
-					encounteredMinus = true;
-					lines.add(line);
-					continue;
-				case '\\':
-					if (line.indexOf("newline at end") > 0) { //$NON-NLS-1$
-						int lastIndex= lines.size();
-						if (lastIndex > 0) {
-							line= (String) lines.get(lastIndex-1);
-							int end= line.length()-1;
-							char lc= line.charAt(end);
-							if (lc == '\n') {
-								end--;
-								if (end > 0 && line.charAt(end) == '\r')
+					case '+':
+						encounteredPlus= true;
+						lines.add(line);
+						continue;
+					case '-':
+						encounteredMinus= true;
+						lines.add(line);
+						continue;
+					case '\\':
+						if (line.indexOf("newline at end") > 0) { //$NON-NLS-1$
+							int lastIndex= lines.size();
+							if (lastIndex > 0) {
+								line= (String)lines.get(lastIndex - 1);
+								int end= line.length() - 1;
+								char lc= line.charAt(end);
+								if (lc == '\n') {
+									end--;
+									if (end > 0 && line.charAt(end) == '\r')
+										end--;
+								} else if (lc == '\r') {
 									end--;
-							} else if (lc == '\r') {
-								end--;
+								}
+								line= line.substring(0, end + 1);
+								lines.set(lastIndex - 1, line);
 							}
-							line= line.substring(0, end+1);
-							lines.set(lastIndex-1, line);
+							continue;
 						}
-						continue;
-					}
-					break;
-				default:
-					if (DEBUG) {
-						int a1= c, a2= 0;
-						if (line.length() > 1)
-							a2= line.charAt(1);
-						System.out.println("char: " + a1 + " " + a2); //$NON-NLS-1$ //$NON-NLS-2$
-					}
-					break;
+						break;
+					default:
+						throw new IOException("Invalid patch"); //$NON-NLS-1$
 				}
 				return line;
 			}
diff --git a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/patch/InputPatchPage.java b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/patch/InputPatchPage.java
index 544fdc3f3..c68ed57a0 100644
--- a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/patch/InputPatchPage.java
+++ b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/patch/InputPatchPage.java
@@ -23,31 +23,8 @@ import java.lang.reflect.InvocationTargetException;
 import java.net.MalformedURLException;
 import java.net.URL;
 
-import org.eclipse.compare.internal.ICompareContextIds;
-import org.eclipse.compare.internal.Utilities;
-import org.eclipse.compare.internal.core.patch.FilePatch2;
-import org.eclipse.core.resources.IFile;
-import org.eclipse.core.resources.IFolder;
-import org.eclipse.core.resources.IProject;
-import org.eclipse.core.resources.IResource;
-import org.eclipse.core.resources.ResourcesPlugin;
-import org.eclipse.core.runtime.IPath;
-import org.eclipse.core.runtime.OperationCanceledException;
-import org.eclipse.core.runtime.Path;
-import org.eclipse.jface.dialogs.Dialog;
-import org.eclipse.jface.dialogs.IDialogConstants;
-import org.eclipse.jface.dialogs.IDialogSettings;
-import org.eclipse.jface.dialogs.MessageDialog;
-import org.eclipse.jface.viewers.DoubleClickEvent;
-import org.eclipse.jface.viewers.IDoubleClickListener;
-import org.eclipse.jface.viewers.ISelection;
-import org.eclipse.jface.viewers.ISelectionChangedListener;
-import org.eclipse.jface.viewers.SelectionChangedEvent;
-import org.eclipse.jface.viewers.StructuredSelection;
-import org.eclipse.jface.viewers.TreeSelection;
-import org.eclipse.jface.viewers.TreeViewer;
-import org.eclipse.jface.wizard.IWizardPage;
-import org.eclipse.jface.wizard.WizardPage;
+import com.ibm.icu.text.MessageFormat;
+
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.dnd.Clipboard;
 import org.eclipse.swt.dnd.TextTransfer;
@@ -68,12 +45,40 @@ import org.eclipse.swt.widgets.FileDialog;
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.swt.widgets.Text;
+
+import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.OperationCanceledException;
+import org.eclipse.core.runtime.Path;
+
+import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IFolder;
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.ResourcesPlugin;
+
+import org.eclipse.jface.dialogs.Dialog;
+import org.eclipse.jface.dialogs.IDialogConstants;
+import org.eclipse.jface.dialogs.IDialogSettings;
+import org.eclipse.jface.dialogs.MessageDialog;
+import org.eclipse.jface.viewers.DoubleClickEvent;
+import org.eclipse.jface.viewers.IDoubleClickListener;
+import org.eclipse.jface.viewers.ISelection;
+import org.eclipse.jface.viewers.ISelectionChangedListener;
+import org.eclipse.jface.viewers.SelectionChangedEvent;
+import org.eclipse.jface.viewers.StructuredSelection;
+import org.eclipse.jface.viewers.TreeSelection;
+import org.eclipse.jface.viewers.TreeViewer;
+import org.eclipse.jface.wizard.IWizardPage;
+import org.eclipse.jface.wizard.WizardPage;
+
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.model.WorkbenchContentProvider;
 import org.eclipse.ui.model.WorkbenchLabelProvider;
 import org.eclipse.ui.views.navigator.ResourceComparator;
 
-import com.ibm.icu.text.MessageFormat;
+import org.eclipse.compare.internal.ICompareContextIds;
+import org.eclipse.compare.internal.Utilities;
+import org.eclipse.compare.patch.IFilePatch2;
 
 public class InputPatchPage extends WizardPage {
 
@@ -193,21 +198,11 @@ public class InputPatchPage extends WizardPage {
 	 * @return PreviewPatchPage if multi-project patch, PatchTargetPage if single project patch
 	 */
 	public IWizardPage getNextPage() {
+		if (!checkPageComplete())
+			return this;
 
 		WorkspacePatcher patcher= ((PatchWizard) getWizard()).getPatcher();
 		
-		// Read in the patch
-		readInPatch();
-		
-		FilePatch2[] diffs= patcher.getDiffs();
-		if (diffs == null || diffs.length == 0) {
-			String format= PatchMessages.InputPatchPage_NoDiffsFound_format;	
-			String message= MessageFormat.format(format, new String[] { fPatchSource });
-			MessageDialog.openInformation(null,
-				PatchMessages.InputPatchPage_PatchErrorDialog_title, message); 
-			return this;
-		}
-
 		// guess prefix count
 		int guess= 0; // guessPrefix(diffs);
 		patcher.setStripPrefixSegments(guess);
@@ -236,6 +231,22 @@ public class InputPatchPage extends WizardPage {
 		return super.getNextPage();
 	}
 
+	boolean checkPageComplete() {
+		WorkspacePatcher patcher= ((PatchWizard)getWizard()).getPatcher();
+
+		// Read in the patch
+		readInPatch();
+
+		IFilePatch2[] diffs= patcher.getDiffs();
+		if (diffs == null || diffs.length == 0) {
+			String format= PatchMessages.InputPatchPage_NoDiffsFound_format;
+			String message= MessageFormat.format(format, new String[] { fPatchSource });
+			MessageDialog.openInformation(null, PatchMessages.InputPatchPage_PatchErrorDialog_title, message);
+			return false;
+		}
+		return true;
+	}
+
 	/*
 	 * Reads in the patch contents
 	 */
@@ -309,6 +320,7 @@ public class InputPatchPage extends WizardPage {
 					fPatchRead=true;
 				} catch (Exception ex) {
 					// Ignore. User will be informed of error since patcher contains no diffs
+					setPageComplete(false);
 				}
 			}
 		} finally {
@@ -874,12 +886,10 @@ public class InputPatchPage extends WizardPage {
 					}
 					// maybe it's an URL
 					try {
-						URL url = new URL((String)o);
-						if(url != null) {
-							setInputButtonState(URL);
-							fPatchURLField.setText((String)o);
-							return true;
-						}
+						new URL((String)o);
+						setInputButtonState(URL);
+						fPatchURLField.setText((String)o);
+						return true;
 					} catch (MalformedURLException e) {
 						// ignore
 					}
@@ -906,7 +916,7 @@ public class InputPatchPage extends WizardPage {
 			return false;
 		}
 
-		FilePatch2[] diffs= patcher.getDiffs();
+		IFilePatch2[] diffs= patcher.getDiffs();
 		if (diffs == null || diffs.length == 0)
 			return false;
 		return true;
diff --git a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/patch/PatchWizard.java b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/patch/PatchWizard.java
index 3f7f02d86..1daddd07e 100644
--- a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/patch/PatchWizard.java
+++ b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/patch/PatchWizard.java
@@ -13,26 +13,30 @@ package org.eclipse.compare.internal.patch;
 import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 
-import org.eclipse.compare.CompareConfiguration;
-import org.eclipse.compare.internal.CompareUIPlugin;
-import org.eclipse.compare.internal.ExceptionHandler;
-import org.eclipse.compare.internal.Utilities;
-import org.eclipse.core.resources.IFile;
-import org.eclipse.core.resources.IResource;
-import org.eclipse.core.resources.IStorage;
-import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.Assert;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.jobs.ISchedulingRule;
 import org.eclipse.core.runtime.jobs.MultiRule;
+
+import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.IStorage;
+import org.eclipse.core.resources.ResourcesPlugin;
+
 import org.eclipse.jface.dialogs.ErrorDialog;
 import org.eclipse.jface.dialogs.IDialogSettings;
 import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.jface.wizard.IWizardPage;
 import org.eclipse.jface.wizard.Wizard;
+
 import org.eclipse.ui.actions.WorkspaceModifyOperation;
 
+import org.eclipse.compare.CompareConfiguration;
+import org.eclipse.compare.internal.CompareUIPlugin;
+import org.eclipse.compare.internal.ExceptionHandler;
+import org.eclipse.compare.internal.Utilities;
+
 public class PatchWizard extends Wizard {
 
 	// dialog store id constants
@@ -128,6 +132,10 @@ public class PatchWizard extends Wizard {
 			if (!fPatchWizardPage.isPatchRead())
 				fPatchWizardPage.readInPatch();
 			fPatcher.refresh();
+			
+			// make sure that the patch is not invalid
+			if (!fPatchWizardPage.checkPageComplete())
+				return false;
 		} else {
 			//either we have a patch from the patch input page or one has
 			//been specified; double check this