IAuthenticationRequest.AddCallbackArgument no longer appends parameters to existing values.

Fixes Google Code Issue 203.

4 files changed, 64 insertions, 6 deletions

diff --git a/src/DotNetOpenAuth.Test/OpenId/RelyingParty/AuthenticationRequestTests.cs b/src/DotNetOpenAuth.Test/OpenId/RelyingParty/AuthenticationRequestTests.cs
index e73ec13..ce95038 100644
--- a/src/DotNetOpenAuth.Test/OpenId/RelyingParty/AuthenticationRequestTests.cs
+++ b/src/DotNetOpenAuth.Test/OpenId/RelyingParty/AuthenticationRequestTests.cs
@@ -11,6 +11,7 @@ namespace DotNetOpenAuth.Test.OpenId.RelyingParty {
 	using System.Linq;
 	using System.Text;
 	using System.Web;
+	using DotNetOpenAuth.Messaging;
 	using DotNetOpenAuth.OpenId;
 	using DotNetOpenAuth.OpenId.Extensions.SimpleRegistration;
 	using DotNetOpenAuth.OpenId.Messages;
@@ -20,14 +21,15 @@ namespace DotNetOpenAuth.Test.OpenId.RelyingParty {
 	[TestClass]
 	public class AuthenticationRequestTests : OpenIdTestBase {
 		private readonly Realm realm = new Realm("http://localhost/rp.aspx");
-		private readonly Uri returnTo = new Uri("http://localhost/rp.aspx");
 		private readonly Identifier claimedId = "http://claimedId";
 		private readonly Identifier delegatedLocalId = "http://localId";
 		private readonly Protocol protocol = Protocol.Default;
+		private Uri returnTo;
 
 		[TestInitialize]
 		public override void SetUp() {
 			base.SetUp();
+			this.returnTo = new Uri("http://localhost/rp.aspx");
 		}
 
 		/// <summary>
@@ -113,11 +115,41 @@ namespace DotNetOpenAuth.Test.OpenId.RelyingParty {
 		}
 
 		/// <summary>
+		/// Verifies that AddCallbackArguments adds query arguments to the return_to URL of the message.
+		/// </summary>
+		[TestMethod]
+		public void AddCallbackArgument() {
+			IAuthenticationRequest_Accessor authRequest = this.CreateAuthenticationRequest(this.claimedId, this.claimedId);
+			Assert.AreEqual(this.returnTo, authRequest.ReturnToUrl);
+			authRequest.AddCallbackArguments("p1", "v1");
+			var req = (SignedResponseRequest)authRequest.RedirectingResponse.OriginalMessage;
+			NameValueCollection query = HttpUtility.ParseQueryString(req.ReturnTo.Query);
+			Assert.AreEqual("v1", query["p1"]);
+		}
+
+		/// <summary>
+		/// Verifies that AddCallbackArguments replaces pre-existing parameter values 
+		/// rather than appending them.
+		/// </summary>
+		[TestMethod]
+		public void AddCallbackArgumentClearsPreviousArgument() {
+			UriBuilder returnToWithArgs = new UriBuilder(this.returnTo);
+			returnToWithArgs.AppendQueryArgs(new Dictionary<string, string> { { "p1", "v1" } });
+			this.returnTo = returnToWithArgs.Uri;
+			IAuthenticationRequest_Accessor authRequest = this.CreateAuthenticationRequest(this.claimedId, this.claimedId);
+			authRequest.AddCallbackArguments("p1", "v2");
+			var req = (SignedResponseRequest)authRequest.RedirectingResponse.OriginalMessage;
+			NameValueCollection query = HttpUtility.ParseQueryString(req.ReturnTo.Query);
+			Assert.AreEqual("v2", query["p1"]);
+		}
+
+		/// <summary>
 		/// Verifies that authentication requests are generated first for OPs that respond
 		/// to authentication requests.
 		/// </summary>
 		[TestMethod, Ignore]
 		public void UnresponsiveProvidersComeLast() {
+			// TODO: code here
 			Assert.Inconclusive("Not yet implemented.");
 		}
 
diff --git a/src/DotNetOpenAuth/Messaging/MessagingUtilities.cs b/src/DotNetOpenAuth/Messaging/MessagingUtilities.cs
index 0292107..2c17c4b 100644
--- a/src/DotNetOpenAuth/Messaging/MessagingUtilities.cs
+++ b/src/DotNetOpenAuth/Messaging/MessagingUtilities.cs
@@ -476,10 +476,12 @@ namespace DotNetOpenAuth.Messaging {
 		/// The arguments to add to the query.  
 		/// If null, <paramref name="builder"/> is not changed.
 		/// </param>
+		/// <remarks>
+		/// If the parameters to add match names of parameters that already are defined
+		/// in the query string, the existing ones are <i>not</i> replaced.
+		/// </remarks>
 		internal static void AppendQueryArgs(this UriBuilder builder, IEnumerable<KeyValuePair<string, string>> args) {
-			if (builder == null) {
-				throw new ArgumentNullException("builder");
-			}
+			ErrorUtilities.VerifyArgumentNotNull(builder, "builder");
 
 			if (args != null && args.Count() > 0) {
 				StringBuilder sb = new StringBuilder(50 + (args.Count() * 10));
@@ -494,6 +496,28 @@ namespace DotNetOpenAuth.Messaging {
 		}
 
 		/// <summary>
+		/// Adds parameters to a query string, replacing parameters that
+		/// match ones that already exist in the query string.
+		/// </summary>
+		/// <param name="builder">The UriBuilder to add arguments to.</param>
+		/// <param name="args">
+		/// The arguments to add to the query.  
+		/// If null, <paramref name="builder"/> is not changed.
+		/// </param>
+		internal static void AppendAndReplaceQueryArgs(this UriBuilder builder, IEnumerable<KeyValuePair<string, string>> args) {
+			ErrorUtilities.VerifyArgumentNotNull(builder, "builder");
+
+			if (args != null && args.Count() > 0) {
+				NameValueCollection aggregatedArgs = HttpUtility.ParseQueryString(builder.Query);
+				foreach (var pair in args) {
+					aggregatedArgs[pair.Key] = pair.Value;
+				}
+
+				builder.Query = CreateQueryString(aggregatedArgs.ToDictionary());
+			}
+		}
+
+		/// <summary>
 		/// Extracts the recipient from an HttpRequestInfo.
 		/// </summary>
 		/// <param name="request">The request to get recipient information from.</param>
diff --git a/src/DotNetOpenAuth/OpenId/Messages/SignedResponseRequest.cs b/src/DotNetOpenAuth/OpenId/Messages/SignedResponseRequest.cs
index 0e21807..9f3fbaf 100644
--- a/src/DotNetOpenAuth/OpenId/Messages/SignedResponseRequest.cs
+++ b/src/DotNetOpenAuth/OpenId/Messages/SignedResponseRequest.cs
@@ -140,7 +140,7 @@ namespace DotNetOpenAuth.OpenId.Messages {
 		internal void AddReturnToArguments(IEnumerable<KeyValuePair<string, string>> keysValues) {
 			ErrorUtilities.VerifyArgumentNotNull(keysValues, "keysValues");
 			UriBuilder returnToBuilder = new UriBuilder(this.ReturnTo);
-			returnToBuilder.AppendQueryArgs(keysValues);
+			returnToBuilder.AppendAndReplaceQueryArgs(keysValues);
 			this.ReturnTo = returnToBuilder.Uri;
 		}
 
diff --git a/src/DotNetOpenAuth/OpenId/RelyingParty/AuthenticationRequest.cs b/src/DotNetOpenAuth/OpenId/RelyingParty/AuthenticationRequest.cs
index 6724fd1..c7d0bae 100644
--- a/src/DotNetOpenAuth/OpenId/RelyingParty/AuthenticationRequest.cs
+++ b/src/DotNetOpenAuth/OpenId/RelyingParty/AuthenticationRequest.cs
@@ -418,7 +418,9 @@ namespace DotNetOpenAuth.OpenId.RelyingParty {
 			request.ReturnTo = this.ReturnToUrl;
 			request.AssociationHandle = association != null ? association.Handle : null;
 			request.AddReturnToArguments(this.returnToArgs);
-			request.AddReturnToArguments(UserSuppliedIdentifierParameterName, this.endpoint.UserSuppliedIdentifier);
+			if (this.endpoint.UserSuppliedIdentifier != null) {
+				request.AddReturnToArguments(UserSuppliedIdentifierParameterName, this.endpoint.UserSuppliedIdentifier);
+			}
 			foreach (IOpenIdMessageExtension extension in this.extensions) {
 				request.Extensions.Add(extension);
 			}