OTT-241 URL Encoding All Values of Macro

PubMatic-OpenWrap · Jul 22, 2021 · 07d1b4c · 07d1b4c
1 parent 5911dd0
commit 07d1b4c
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 30 deletions.
diff --git a/adapters/vastbidder/macro_processor.go b/adapters/vastbidder/macro_processor.go
@@ -87,7 +87,6 @@ func (mp *MacroProcessor) processKey(key string) (string, bool) {
 				//escaping string nEscaping times
 				value = escape(value, nEscaping)
 			}
-
 			if nil != valueCallback && valueCallback.cached {
 				//cached value if its cached flag is true
 				mp.macroCache[key] = value
@@ -148,8 +147,7 @@ func (mp *MacroProcessor) ProcessString(in string) (response string) {
 		//glog.Infof("\nSearch[%d] <start,end,key>: [%d,%d,%s]", count, start, end, key)
 	}
 	response = out.String()
-	glog.V(3).Infof("[MACRO]:in:[%s]\nreplaced:[%s]\n", in, response)
-
+	glog.V(3).Infof("[MACRO]:in:[%s] replaced:[%s]", in, response)
 	return
 }
 
@@ -159,15 +157,15 @@ func (mp *MacroProcessor) ProcessURL(uri string, flags Flags) (response string)
 		return mp.ProcessString(uri)
 	}
 
-	url, _ := url.Parse(uri)
+	murl, _ := url.Parse(uri)
 
-	url.Path = mp.ProcessString(url.Path)
-	url.RawQuery = mp.processURLValues(url.Query(), flags)
-	url.Fragment = mp.ProcessString(url.Fragment)
+	murl.Path = mp.ProcessString(murl.Path)
+	murl.RawQuery = mp.processURLValues(murl.Query(), flags)
+	murl.Fragment = mp.ProcessString(murl.Fragment)
 
-	response = url.String()
+	response = murl.String()
 
-	glog.V(3).Infof("[MACRO]:in:[%s]\nreplaced:[%s]\n", uri, response)
+	glog.V(3).Infof("[MACRO]:in:[%s] replaced:[%s]", uri, response)
 	return
 }
 
@@ -199,7 +197,7 @@ func (mp *MacroProcessor) processURLValues(values url.Values, flags Flags) (resp
 			}
 			out.WriteString(k)
 			out.WriteByte('=')
-			out.WriteString(value)
+			out.WriteString(url.QueryEscape(value))
 		}
 	}
 	return out.String()

diff --git a/adapters/vastbidder/macro_processor_test.go b/adapters/vastbidder/macro_processor_test.go
@@ -423,21 +423,25 @@ func TestMacroProcessor_processURLValues(t *testing.T) {
 }
 
 func TestMacroProcessor_processURLValuesEscapingKeys(t *testing.T) {
+	testMacroImpValues := map[string]string{
+		MacroPubID: `pub id`,
+		MacroTagID: `tagid value`,
+	}
+
 	testMacroValues := map[string]string{
-		MacroPubID:                     `pub id`,
-		MacroPubID + macroEscapeSuffix: `pub+id`,
-		MacroTagID:                     `tagid value`,
-		MacroTagID + macroEscapeSuffix: `tagid+value`,
-		MacroTagID + macroEscapeSuffix + macroEscapeSuffix: `tagid%2Bvalue`,
+		MacroPubID:                     `pub+id`,
+		MacroTagID:                     `tagid+value`,
+		MacroTagID + macroEscapeSuffix: `tagid%2Bvalue`,
+		MacroTagID + macroEscapeSuffix + macroEscapeSuffix: `tagid%252Bvalue`,
 	}
 
 	sampleBidRequest := &openrtb2.BidRequest{
 		Imp: []openrtb2.Imp{
-			{TagID: testMacroValues[MacroTagID]},
+			{TagID: testMacroImpValues[MacroTagID]},
 		},
 		Site: &openrtb2.Site{
 			Publisher: &openrtb2.Publisher{
-				ID: testMacroValues[MacroPubID],
+				ID: testMacroImpValues[MacroPubID],
 			},
 		},
 	}
@@ -494,23 +498,20 @@ func TestMacroProcessor_processURLValuesEscapingKeys(t *testing.T) {
 }
 
 func TestMacroProcessor_ProcessURL(t *testing.T) {
-	testMacroValues := map[string]string{
-		MacroPubID:                     `123`,
-		MacroPubID + macroEscapeSuffix: `123`,
-		MacroSiteID:                    `567`,
-		MacroTagID:                     `tagid value`,
-		MacroTagID + macroEscapeSuffix: `tagid+value`,
-		MacroTagID + macroEscapeSuffix + macroEscapeSuffix: `tagid%2Bvalue`,
+	testMacroImpValues := map[string]string{
+		MacroPubID:  `123`,
+		MacroSiteID: `567`,
+		MacroTagID:  `tagid value`,
 	}
 
 	sampleBidRequest := &openrtb2.BidRequest{
 		Imp: []openrtb2.Imp{
-			{TagID: testMacroValues[MacroTagID]},
+			{TagID: testMacroImpValues[MacroTagID]},
 		},
 		Site: &openrtb2.Site{
-			ID: testMacroValues[MacroSiteID],
+			ID: testMacroImpValues[MacroSiteID],
 			Publisher: &openrtb2.Publisher{
-				ID: testMacroValues[MacroPubID],
+				ID: testMacroImpValues[MacroPubID],
 			},
 		},
 	}
@@ -533,15 +534,15 @@ func TestMacroProcessor_ProcessURL(t *testing.T) {
 			wantResponse: ``,
 		},
 		{
-			name: "RemovedEmptyParams",
+			name: "RemovedEmptyParams1",
 			args: args{
-				uri:   `http://xyz.domain.com/` + GetMacroKey(MacroPubID) + `/` + GetMacroKey(MacroSiteID) + `?tagID=` + GetMacroKey(MacroTagID+macroEscapeSuffix) + `&notfound=` + GetMacroKey(MacroTimeout) + `&k1=v1&k2=v2`,
+				uri:   `http://xyz.domain.com/` + GetMacroKey(MacroPubID) + `/` + GetMacroKey(MacroSiteID) + `?tagID=` + GetMacroKey(MacroTagID) + `&notfound=` + GetMacroKey(MacroTimeout) + `&k1=v1&k2=v2`,
 				flags: Flags{RemoveEmptyParam: true},
 			},
 			wantResponse: `http://xyz.domain.com/123/567?tagID=tagid+value&k1=v1&k2=v2`,
 		},
 		{
-			name: "RemovedEmptyParams",
+			name: "RemovedEmptyParams2",
 			args: args{
 				uri:   `http://xyz.domain.com/` + GetMacroKey(MacroPubID) + `/` + GetMacroKey(MacroSiteID) + `?tagID=` + GetMacroKey(MacroTagID+macroEscapeSuffix) + `&notfound=` + GetMacroKey(MacroTimeout) + `&k1=v1&k2=v2`,
 				flags: Flags{RemoveEmptyParam: false},