From b908c9eb0e010ed62d6fd1c7bd1ec4ca5bdc1853 Mon Sep 17 00:00:00 2001
From: Todd Lipcon <todd@apache.org>
Date: Tue, 5 Jul 2011 22:09:54 +0000
Subject: [PATCH] HADOOP-7440. HttpServer.getParameterValues throws NPE for
 missing parameters. Contributed by Uma Maheswara Rao G and Todd Lipcon.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1143212 13f79535-47bb-0310-9956-ffa450edef68
---
 common/CHANGES.txt                            |  3 ++
 .../org/apache/hadoop/http/HttpServer.java    |  3 ++
 .../apache/hadoop/http/TestHtmlQuoting.java   | 31 +++++++++++++++++--
 .../apache/hadoop/http/TestHttpServer.java    | 26 ++++++++++++++++
 4 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/common/CHANGES.txt b/common/CHANGES.txt
index b46dd5e9af..c84dc559c8 100644
--- a/common/CHANGES.txt
+++ b/common/CHANGES.txt
@@ -346,6 +346,9 @@ Trunk (unreleased changes)
     HADOOP-7090. Fix resource leaks in s3.INode, BloomMapFile, WritableUtils
     and CBZip2OutputStream.  (Uma Maheswara Rao G via szetszwo)
 
+    HADOOP-7440. HttpServer.getParameterValues throws NPE for missing
+    parameters. (Uma Maheswara Rao G and todd via todd)
+
 Release 0.22.0 - Unreleased
 
   INCOMPATIBLE CHANGES
diff --git a/common/src/java/org/apache/hadoop/http/HttpServer.java b/common/src/java/org/apache/hadoop/http/HttpServer.java
index 75b3bbac34..6d6864c63d 100644
--- a/common/src/java/org/apache/hadoop/http/HttpServer.java
+++ b/common/src/java/org/apache/hadoop/http/HttpServer.java
@@ -800,6 +800,9 @@ public String getParameter(String name) {
       public String[] getParameterValues(String name) {
         String unquoteName = HtmlQuoting.unquoteHtmlChars(name);
         String[] unquoteValue = rawRequest.getParameterValues(unquoteName);
+        if (unquoteValue == null) {
+          return null;
+        }
         String[] result = new String[unquoteValue.length];
         for(int i=0; i < result.length; ++i) {
           result[i] = HtmlQuoting.quoteHtmlChars(unquoteValue[i]);
diff --git a/common/src/test/core/org/apache/hadoop/http/TestHtmlQuoting.java b/common/src/test/core/org/apache/hadoop/http/TestHtmlQuoting.java
index 1c3663a4fd..9fc53a3b6f 100644
--- a/common/src/test/core/org/apache/hadoop/http/TestHtmlQuoting.java
+++ b/common/src/test/core/org/apache/hadoop/http/TestHtmlQuoting.java
@@ -17,11 +17,12 @@
  */
 package org.apache.hadoop.http;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
+
+import javax.servlet.http.HttpServletRequest;
 
 import org.junit.Test;
+import org.mockito.Mockito;
 
 public class TestHtmlQuoting {
 
@@ -62,4 +63,28 @@ private void runRoundTrip(String str) throws Exception {
     }
     runRoundTrip(buffer.toString());
   }
+  
+
+  @Test
+  public void testRequestQuoting() throws Exception {
+    HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
+    HttpServer.QuotingInputFilter.RequestQuoter quoter =
+      new HttpServer.QuotingInputFilter.RequestQuoter(mockReq);
+    
+    Mockito.doReturn("a<b").when(mockReq).getParameter("x");
+    assertEquals("Test simple param quoting",
+        "a&lt;b", quoter.getParameter("x"));
+    
+    Mockito.doReturn(null).when(mockReq).getParameter("x");
+    assertEquals("Test that missing parameters dont cause NPE",
+        null, quoter.getParameter("x"));
+
+    Mockito.doReturn(new String[]{"a<b", "b"}).when(mockReq).getParameterValues("x");
+    assertArrayEquals("Test escaping of an array",
+        new String[]{"a&lt;b", "b"}, quoter.getParameterValues("x"));
+
+    Mockito.doReturn(null).when(mockReq).getParameterValues("x");
+    assertArrayEquals("Test that missing parameters dont cause NPE for array",
+        null, quoter.getParameterValues("x"));
+  }
 }
diff --git a/common/src/test/core/org/apache/hadoop/http/TestHttpServer.java b/common/src/test/core/org/apache/hadoop/http/TestHttpServer.java
index 95c5d0aef7..0a25236fc8 100644
--- a/common/src/test/core/org/apache/hadoop/http/TestHttpServer.java
+++ b/common/src/test/core/org/apache/hadoop/http/TestHttpServer.java
@@ -45,16 +45,20 @@
 import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
 
+import junit.framework.Assert;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.CommonConfigurationKeys;
+import org.apache.hadoop.http.HttpServer.QuotingInputFilter.RequestQuoter;
 import org.apache.hadoop.security.Groups;
 import org.apache.hadoop.security.ShellBasedUnixGroupsMapping;
 import org.apache.hadoop.security.authorize.AccessControlList;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 public class TestHttpServer extends HttpServerFunctionalTest {
   private static HttpServer server;
@@ -379,4 +383,26 @@ public void testAuthorizationOfDefaultServlets() throws Exception {
     }
     myServer.stop();
   }
+  
+  @Test
+  public void testRequestQuoterWithNull() throws Exception {
+    HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+    Mockito.doReturn(null).when(request).getParameterValues("dummy");
+    RequestQuoter requestQuoter = new RequestQuoter(request);
+    String[] parameterValues = requestQuoter.getParameterValues("dummy");
+    Assert.assertEquals("It should return null "
+        + "when there are no values for the parameter", null, parameterValues);
+  }
+
+  @Test
+  public void testRequestQuoterWithNotNull() throws Exception {
+    HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+    String[] values = new String[] { "abc", "def" };
+    Mockito.doReturn(values).when(request).getParameterValues("dummy");
+    RequestQuoter requestQuoter = new RequestQuoter(request);
+    String[] parameterValues = requestQuoter.getParameterValues("dummy");
+    Assert.assertTrue("It should return Parameter Values", Arrays.equals(
+        values, parameterValues));
+  }
+
 }