14

I'm working on legacy code and need to make a patch.

The problem: an ancient application sends bad HTTP POST requests. One of the parameters is not URL encoded. I know that this parameter always comes last and I know it's name. I'm now trying to fix it on the server side which is running inside tomcat.

This parameter is not accessible via standard getParameter method of HttpServletRequest, since it's malformed. Method simply returns null. But when I manually read the whole body of request through ServletInputStream all the other parameters disappear. Looks like underlying classes can't parse contents of ServletInputStream since it's drained out.

So far I've managed to make a wrapper that reads all parameters from body and overrides all parameter access methods. But if any filter in the chain before mine will try to access any parameter, everything will break since ServletInputStream will be empty.

Can I somehow evade this problem? May be there's different approach?

To summarize, If I'll read raw request body in the filter, parameters will disappear from the request. If I read single parameter, ServletInputStream will become empty and manual processing will be impossible. Moreover, it's impossible to read malformed parameter via getParameter method.

5 Answers 5

14

Solution I've found:

It's not enough to just redefine parameter accessing methods. Several things must be done.

  1. A filter is needed where request will be wrapped.
  2. A custom HttpRequestWrapper is needed with all parameter access methods overridden. Request body should be parsed in constructor and stored as a field.
  3. getInputStream and getReader methods should be redefined as well. They return values depend on the stored request body.
  4. Custom class extending ServletInputStream is required since this one is abstract.

This 4 combined will allow you to use getParameter without interference with getInputStream and getReader methods.

Mind that manual request parameter parsing may get complicated with multipart requests. But that's another topic.

To clarify, I redefined parameter accessing methods because my request was damaged as stated in the question. You may not need that.

4
  • Could you please provide more details regarding manual request parameter parsing? Isn't it enough to just override getInputStream and getReader? Commented Dec 22, 2011 at 17:01
  • Ok, I looked it up. I had to manyally parse parameters because one of them was damaged as described in a question. If they are fine in your request, there's no need to do that.
    – clorz
    Commented Dec 26, 2011 at 0:29
  • Do you have sample code? Having to overwrite getParameter() and friends worries me, because I'd rather leave that to the servlet container. In my case all parameters are readable, but I need to read and cache the input stream before my servlet calls getParameter(). And HttpServletRequestWrapper's getParameter() just cals the original request's getParameter() which only knows about the original requests's input stream (which is empty by then).
    – Frans
    Commented Oct 1, 2013 at 16:45
  • @clorz - Could you please share the sample code? I really need this.
    – Prateek
    Commented Dec 26, 2019 at 9:23
10

Rather than overriding methods, why don't you install a servlet filter which rewrites the request?

Jason Hunter has a pretty good article on filters.

2
  • I am not still sure yet, how to read http post data and update post data and set it back to request, if I read request.getReader(), then again I cant read it, how can we do this ?
    – Prateek
    Commented Dec 26, 2019 at 10:16
  • 1
    @PAA: I suggest you ask a new question with all the details rather than adding comments on this not-entirely-related question that is over 10 years old.
    – Jon Skeet
    Commented Dec 26, 2019 at 10:25
6

I did a more complete wrapper that allows you to still access the content in the case Content-Type is application/x-www-form-urlencoded and you already called one of the getParameterXXX methods:

import java.io.BufferedReader;
import java.io.ByteArrayInputStream;
import java.io.IOException;  
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.security.Principal;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.Locale;
import java.util.Map;
import java.util.StringTokenizer;

import javax.servlet.RequestDispatcher;
import javax.servlet.ServletInputStream;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;

/**
 * This class implements the Wrapper or Decorator pattern.<br/> 
 * Methods default to calling through to the wrapped request object, 
 * except the ones that read the request's content (parameters, stream or reader).
 * <p>
 * This class provides a buffered content reading that allows the methods
 * {@link #getReader()}, {@link #getInputStream()} and any of the getParameterXXX to be     called
 * safely and repeatedly with the same results.
 * <p>
 * This class is intended to wrap relatively small HttpServletRequest instances.
 * 
 * @author pgurov
 */
public class HttpServletRequestWrapper implements HttpServletRequest {

private class ServletInputStreamWrapper extends ServletInputStream {

    private byte[] data;
    private int idx = 0;
    ServletInputStreamWrapper(byte[] data) {
        if(data == null)
            data = new byte[0];
        this.data = data;
    }
    @Override
    public int read() throws IOException {
        if(idx == data.length)
            return -1;
        return data[idx++];
    }

}

private HttpServletRequest req;
private byte[] contentData;
private HashMap<String, String[]> parameters;

public HttpServletRequestWrapper() {
    //a trick for Groovy
    throw new IllegalArgumentException("Please use HttpServletRequestWrapper(HttpServletRequest request) constructor!");
}

private HttpServletRequestWrapper(HttpServletRequest request, byte[] contentData, HashMap<String, String[]> parameters) {
    req = request;
    this.contentData = contentData;
    this.parameters = parameters;
}

public HttpServletRequestWrapper(HttpServletRequest request) {
    if(request == null)
        throw new IllegalArgumentException("The HttpServletRequest is null!");
    req = request;
}

/**
 * Returns the wrapped HttpServletRequest.
 * Using the getParameterXXX(), getInputStream() or getReader() methods may interfere
 * with this class operation.
 * 
 * @return 
 *      The wrapped HttpServletRequest.
 */
public HttpServletRequest getRequest() {
    try {
        parseRequest();
    } catch (IOException e) {
        throw new IllegalStateException("Cannot parse the request!", e);
    }
    return new HttpServletRequestWrapper(req, contentData, parameters);
}

/**
 * This method is safe to use multiple times.
 * Changing the returned array will not interfere with this class operation.
 * 
 * @return 
 *      The cloned content data.
 */
public byte[] getContentData() {
    return contentData.clone();
}

/**
 * This method is safe to use multiple times.
 * Changing the returned map or the array of any of the map's values will not
 * interfere with this class operation.
 * 
 * @return
 *      The clonned parameters map.
 */
public HashMap<String, String[]> getParameters() {
    HashMap<String, String[]> map = new HashMap<String, String[]>(parameters.size() * 2);
    for(String key : parameters.keySet()) {
        map.put(key, parameters.get(key).clone());
    }
    return map;
}

private void parseRequest() throws IOException {
    if(contentData != null)
        return; //already parsed

    byte[] data = new byte[req.getContentLength()];
    int len = 0, totalLen = 0;
    InputStream is = req.getInputStream();
    while(totalLen < data.length) {
        totalLen += (len = is.read(data, totalLen, data.length - totalLen));
        if(len < 1)
            throw new IOException("Cannot read more than " + totalLen + (totalLen == 1 ? " byte!" : " bytes!"));
    }
    contentData = data;
    String enc = req.getCharacterEncoding();
    if(enc == null)
        enc = "UTF-8";
    String s = new String(data, enc), name, value;
    StringTokenizer st = new StringTokenizer(s, "&");
    int i;
    HashMap<String, LinkedList<String>> mapA = new HashMap<String, LinkedList<String>>(data.length * 2);
    LinkedList<String> list;
    boolean decode = req.getContentType() != null && req.getContentType().equals("application/x-www-form-urlencoded");
    while(st.hasMoreTokens()) {
        s = st.nextToken();
        i = s.indexOf("=");
        if(i > 0 && s.length() > i + 1) {
            name = s.substring(0, i);
            value = s.substring(i+1);
            if(decode) {
                try {
                    name = URLDecoder.decode(name, "UTF-8");
                } catch(Exception e) {}
                try {
                    value = URLDecoder.decode(value, "UTF-8");
                } catch(Exception e) {}
            }
            list = mapA.get(name);
            if(list == null) {
                list = new LinkedList<String>();
                mapA.put(name, list);
            }
            list.add(value);
        }
    }
    HashMap<String, String[]> map = new HashMap<String, String[]>(mapA.size() * 2);
    for(String key : mapA.keySet()) {
        list = mapA.get(key);
        map.put(key, list.toArray(new String[list.size()]));
    }
    parameters = map;
}

/**
 * This method is safe to call multiple times.
 * Calling it will not interfere with getParameterXXX() or getReader().
 * Every time a new ServletInputStream is returned that reads data from the begining.
 * 
 * @return
 *      A new ServletInputStream.
 */
public ServletInputStream getInputStream() throws IOException {
    parseRequest();

    return new ServletInputStreamWrapper(contentData);
}

/**
 * This method is safe to call multiple times.
 * Calling it will not interfere with getParameterXXX() or getInputStream().
 * Every time a new BufferedReader is returned that reads data from the begining.
 * 
 * @return
 *      A new BufferedReader with the wrapped request's character encoding (or UTF-8 if null).
 */
public BufferedReader getReader() throws IOException {
    parseRequest();

    String enc = req.getCharacterEncoding();
    if(enc == null)
        enc = "UTF-8";
    return new BufferedReader(new InputStreamReader(new ByteArrayInputStream(contentData), enc));
}

/**
 * This method is safe to execute multiple times.
 * 
 * @see javax.servlet.ServletRequest#getParameter(java.lang.String)
 */
public String getParameter(String name) {
    try {
        parseRequest();
    } catch (IOException e) {
        throw new IllegalStateException("Cannot parse the request!", e);
    }
    String[] values = parameters.get(name);
    if(values == null || values.length == 0)
        return null;
    return values[0];
}

/**
 * This method is safe.
 * 
 * @see {@link #getParameters()}
 * @see javax.servlet.ServletRequest#getParameterMap()
 */
@SuppressWarnings("unchecked")
public Map getParameterMap() {
    try {
        parseRequest();
    } catch (IOException e) {
        throw new IllegalStateException("Cannot parse the request!", e);
    }
    return getParameters();
}

/**
 * This method is safe to execute multiple times.
 * 
 * @see javax.servlet.ServletRequest#getParameterNames()
 */
@SuppressWarnings("unchecked")
public Enumeration getParameterNames() {
    try {
        parseRequest();
    } catch (IOException e) {
        throw new IllegalStateException("Cannot parse the request!", e);
    }
    return new Enumeration<String>() {
        private String[] arr = getParameters().keySet().toArray(new String[0]); 
        private int idx = 0;

        public boolean hasMoreElements() {
            return idx < arr.length;
        }

        public String nextElement() {
            return arr[idx++];
        }

    };
}

/**
 * This method is safe to execute multiple times.
 * Changing the returned array will not interfere with this class operation.
 * 
 * @see javax.servlet.ServletRequest#getParameterValues(java.lang.String)
 */
public String[] getParameterValues(String name) {
    try {
        parseRequest();
    } catch (IOException e) {
        throw new IllegalStateException("Cannot parse the request!", e);
    }
    String[] arr = parameters.get(name);
    if(arr == null)
        return null;
    return arr.clone();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getAuthType()
 */
public String getAuthType() {
    return req.getAuthType();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getContextPath()
 */
public String getContextPath() {
    return req.getContextPath();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getCookies()
 */
public Cookie[] getCookies() {
    return req.getCookies();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getDateHeader(java.lang.String)
 */
public long getDateHeader(String name) {
    return req.getDateHeader(name);
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getHeader(java.lang.String)
 */
public String getHeader(String name) {
    return req.getHeader(name);
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getHeaderNames()
 */
@SuppressWarnings("unchecked")
public Enumeration getHeaderNames() {
    return req.getHeaderNames();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getHeaders(java.lang.String)
 */
@SuppressWarnings("unchecked")
public Enumeration getHeaders(String name) {
    return req.getHeaders(name);
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getIntHeader(java.lang.String)
 */
public int getIntHeader(String name) {
    return req.getIntHeader(name);
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getMethod()
 */
public String getMethod() {
    return req.getMethod();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getPathInfo()
 */
public String getPathInfo() {
    return req.getPathInfo();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getPathTranslated()
 */
public String getPathTranslated() {
    return req.getPathTranslated();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getQueryString()
 */
public String getQueryString() {
    return req.getQueryString();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getRemoteUser()
 */
public String getRemoteUser() {
    return req.getRemoteUser();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getRequestURI()
 */
public String getRequestURI() {
    return req.getRequestURI();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getRequestURL()
 */
public StringBuffer getRequestURL() {
    return req.getRequestURL();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getRequestedSessionId()
 */
public String getRequestedSessionId() {
    return req.getRequestedSessionId();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getServletPath()
 */
public String getServletPath() {
    return req.getServletPath();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getSession()
 */
public HttpSession getSession() {
    return req.getSession();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getSession(boolean)
 */
public HttpSession getSession(boolean create) {
    return req.getSession(create);
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#getUserPrincipal()
 */
public Principal getUserPrincipal() {
    return req.getUserPrincipal();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#isRequestedSessionIdFromCookie()
 */
public boolean isRequestedSessionIdFromCookie() {
    return req.isRequestedSessionIdFromCookie();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#isRequestedSessionIdFromURL()
 */
public boolean isRequestedSessionIdFromURL() {
    return req.isRequestedSessionIdFromURL();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#isRequestedSessionIdFromUrl()
 */
@SuppressWarnings("deprecation")
public boolean isRequestedSessionIdFromUrl() {
    return req.isRequestedSessionIdFromUrl();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#isRequestedSessionIdValid()
 */
public boolean isRequestedSessionIdValid() {
    return req.isRequestedSessionIdValid();
}

/* (non-Javadoc)
 * @see javax.servlet.http.HttpServletRequest#isUserInRole(java.lang.String)
 */
public boolean isUserInRole(String role) {
    return req.isUserInRole(role);
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getAttribute(java.lang.String)
 */
public Object getAttribute(String name) {
    return req.getAttribute(name);
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getAttributeNames()
 */
@SuppressWarnings("unchecked")
public Enumeration getAttributeNames() {
    return req.getAttributeNames();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getCharacterEncoding()
 */
public String getCharacterEncoding() {
    return req.getCharacterEncoding();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getContentLength()
 */
public int getContentLength() {
    return req.getContentLength();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getContentType()
 */
public String getContentType() {
    return req.getContentType();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getLocalAddr()
 */
public String getLocalAddr() {
    return req.getLocalAddr();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getLocalName()
 */
public String getLocalName() {
    return req.getLocalName();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getLocalPort()
 */
public int getLocalPort() {
    return req.getLocalPort();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getLocale()
 */
public Locale getLocale() {
    return req.getLocale();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getLocales()
 */
@SuppressWarnings("unchecked")
public Enumeration getLocales() {
    return req.getLocales();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getProtocol()
 */
public String getProtocol() {
    return req.getProtocol();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getRealPath(java.lang.String)
 */
@SuppressWarnings("deprecation")
public String getRealPath(String path) {
    return req.getRealPath(path);
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getRemoteAddr()
 */
public String getRemoteAddr() {
    return req.getRemoteAddr();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getRemoteHost()
 */
public String getRemoteHost() {
    return req.getRemoteHost();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getRemotePort()
 */
public int getRemotePort() {
    return req.getRemotePort();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getRequestDispatcher(java.lang.String)
 */
public RequestDispatcher getRequestDispatcher(String path) {
    return req.getRequestDispatcher(path);
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getScheme()
 */
public String getScheme() {
    return req.getScheme();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getServerName()
 */
public String getServerName() {
    return req.getServerName();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#getServerPort()
 */
public int getServerPort() {
    return req.getServerPort();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#isSecure()
 */
public boolean isSecure() {
    return req.isSecure();
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#removeAttribute(java.lang.String)
 */
public void removeAttribute(String name) {
    req.removeAttribute(name);
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#setAttribute(java.lang.String, java.lang.Object)
 */
public void setAttribute(String name, Object value) {
    req.setAttribute(name, value);
}

/* (non-Javadoc)
 * @see javax.servlet.ServletRequest#setCharacterEncoding(java.lang.String)
 */
public void setCharacterEncoding(String env)
        throws UnsupportedEncodingException {
    req.setCharacterEncoding(env);
}

}
3
  • Why do you need to override all the methods which are delegated to req objects when the base class would already do that?
    – Divick
    Commented May 12, 2014 at 17:59
  • I realized that you implement the HttpServletRequest instead of extending HttpServletRequestWrapper hence you need to implement all the required methods. But why not extend from HttpServletRequestWrapper which anyways delegates the call to the original request object?
    – Divick
    Commented May 13, 2014 at 17:05
  • The getParameterValues, getParameter methods rely on the original stream so a wrapper alone will not do it. But I cede to the use of the wrapper in order to avoid implementing all the methods of the stream. In fact undertow will reject this without extra standalone configuration.
    – F.O.O
    Commented Nov 20, 2017 at 8:10
3

I wanted to post this as a comment, but I do not have enough rep. Your solution is insufficient in that ServletInputStreamWrapper will return negative integers. For instance, mock a request with input encoding UTF-16, either big or little endian. The input may start with the Byte Order Mark indicating endianess, and when testing my statement please construct the mock request content to do so. http://en.wikipedia.org/wiki/Byte_order_mark#UTF-16 Either of these BOMs contains a 0xFF byte. Since java has no unsigned byte, this 0xFF is returned as a -1. To work around this, just change the read function like so

    public int read() throws IOException {
        if (index == data.length) {
            return -1;
        }
        return data[index++] & 0xff;
    }

I somewhat like your solution because it works well with Spring. At first I tried to eliminate some of the delegation code you wrote by extending from HttpServletRequestWrapper. However, Spring does something interesting: when it encounters a request of type ServletRequestWrapper it unwraps it, calling getRequest(). Problem being that my getRequest() method, as copied from your code, returns a new class that extends from HttpServletRequestWrapper... rinse and repeat infinitely. So it's sad to say, chalk up a win for not using interfaces!

1
  • I also found another problem, where req.getContentType().equals("application/x-www-form-urlencoded") should be req.getContentType().startsWith("application/x-www-form-urlencoded") As it turns out, Firefox can send "application/x-www-form-urlencoded; charset=UTF-8" whereas Chrome and IE only send "application/x-www-form-urlencoded" Commented Dec 18, 2013 at 19:01
1

You could write your own Servlet Filter and hopefully ensure that it appears first in the chain. Then wrap the ServletRequest object in something that will handle the re-writing where needed. Have a look at the Programming Customized Requests and Responses section of http://java.sun.com/products/servlet/Filters.html

------ Update ------

I must be missing something. You say you can read the request body and read the parameters yourself. Couldn't you then ensure your filter is first, wrap the ServletRequest object, read, process and store the parameters, pass your request object up the chain and offer the parameters you stored instead of the original ones?

1
  • I can read all parameters except broken one. I can only read broken parameter via ServletInputStream. When I use input stream, all other parameters disappear. If I parse all parameters from input stream, sometimes servlets later in the chain break. It's complicated =)
    – clorz
    Commented Mar 25, 2009 at 14:25

Not the answer you're looking for? Browse other questions tagged or ask your own question.