You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2011/10/31 14:59:13 UTC
Re: svn commit: r1190589 - in /wicket/trunk/wicket-core/src:
main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java
Hi Pedro,
How exactly you did this change ?
The diff for ServletWebRequest below looks wrong and scary. I see that
the actual source of that class is OK but for some reason the diff is
wrong.
Even ViewVC shows the same problem:
http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java?r1=1186162&r2=1190589&diff_format=h
I remember once Peter Ertl also confused me with such commit notification.
Can you remember the steps you did to commit this change ?
I would like to avoid them in future :-)
On Sat, Oct 29, 2011 at 12:31 AM, <pe...@apache.org> wrote:
> Author: pedro
> Date: Fri Oct 28 21:31:23 2011
> New Revision: 1190589
>
> URL: http://svn.apache.org/viewvc?rev=1190589&view=rev
> Log:
> Making sure that client URL is context relative even while responding errors
> Issue: WICKET-4168
>
> Modified:
> wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java
> wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java
>
> Modified: wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java
> URL: http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java?rev=1190589&r1=1190588&r2=1190589&view=diff
> ==============================================================================
> --- wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java (original)
> +++ wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java Fri Oct 28 21:31:23 2011
> @@ -16,445 +16,120 @@
> */
> package org.apache.wicket.protocol.http.servlet;
>
> -import java.nio.charset.Charset;
> -import java.util.ArrayList;
> -import java.util.Arrays;
> -import java.util.Collections;
> -import java.util.Enumeration;
> -import java.util.HashMap;
> -import java.util.List;
> -import java.util.Locale;
> -import java.util.Map;
> -import java.util.Set;
> -
> -import javax.servlet.ServletRequest;
> -import javax.servlet.http.Cookie;
> import javax.servlet.http.HttpServletRequest;
>
> -import org.apache.wicket.protocol.http.RequestUtils;
> -import org.apache.wicket.request.IRequestParameters;
> -import org.apache.wicket.request.IWritableRequestParameters;
> +import org.apache.wicket.MarkupContainer;
> +import org.apache.wicket.Page;
> +import org.apache.wicket.markup.IMarkupResourceStreamProvider;
> +import org.apache.wicket.markup.html.WebPage;
> +import org.apache.wicket.protocol.http.WebApplication;
> +import org.apache.wicket.protocol.http.mock.MockHttpServletRequest;
> import org.apache.wicket.request.Url;
> import org.apache.wicket.request.http.WebRequest;
> -import org.apache.wicket.util.lang.Args;
> -import org.apache.wicket.util.lang.Bytes;
> -import org.apache.wicket.util.lang.Checks;
> -import org.apache.wicket.util.string.PrependingStringBuffer;
> -import org.apache.wicket.util.string.StringValue;
> -import org.apache.wicket.util.string.Strings;
> -import org.apache.wicket.util.string.UrlUtils;
> -import org.apache.wicket.util.time.Time;
> -import org.apache.wicket.util.upload.FileItemFactory;
> -import org.apache.wicket.util.upload.FileUploadException;
> -import org.slf4j.Logger;
> -import org.slf4j.LoggerFactory;
> +import org.apache.wicket.util.resource.IResourceStream;
> +import org.apache.wicket.util.resource.StringResourceStream;
> +import org.apache.wicket.util.tester.WicketTester;
> +import org.junit.Assert;
> +import org.junit.Test;
>
> /**
> - * {@link WebRequest} subclass that wraps a {@link HttpServletRequest} object.
> - *
> - * @author Matej Knopp
> - * @author Juergen Donnerstag
> - * @author Igor Vaynberg
> + * Tests for {@link ServletWebRequest}
> */
> -public class ServletWebRequest extends WebRequest
> +public class ServletWebRequestTest extends Assert
> {
> - private final HttpServletRequest httpServletRequest;
> -
> - private final Url url;
> -
> - private final String filterPrefix;
> -
> - private final ErrorAttributes errorAttributes;
> -
> - /**
> - * Construct.
> - *
> - * @param httpServletRequest
> - * @param filterPrefix
> - * contentPath + filterPath, used to extract the actual {@link Url}
> - */
> - public ServletWebRequest(HttpServletRequest httpServletRequest, String filterPrefix)
> - {
> - this(httpServletRequest, filterPrefix, null);
> - }
>
> /**
> - * Construct.
> - *
> - * @param httpServletRequest
> - * @param filterPrefix
> - * contentPath + filterPath, used to extract the actual {@link Url}
> - * @param url
> + * Tests that {@link ServletWebRequest#getClientUrl()} returns the current url + the query
> + * string when this is not error dispatched request. When the request is error dispatched it
> + * returns just the request uri to the error page without the query string
> */
> - public ServletWebRequest(HttpServletRequest httpServletRequest, String filterPrefix, Url url)
> + @Test
> + public void wicket3599()
> {
> - Args.notNull(httpServletRequest, "httpServletRequest");
> - Args.notNull(filterPrefix, "filterPrefix");
> + MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null, null);
> + httpRequest.setURL("/" + httpRequest.getContextPath() + "/request/Uri");
> + httpRequest.setParameter("some", "parameter");
>
> - this.httpServletRequest = httpServletRequest;
> - this.filterPrefix = filterPrefix;
> + ServletWebRequest webRequest = new ServletWebRequest(httpRequest, "/");
> + Url clientUrl = webRequest.getClientUrl();
> + assertEquals("request/Uri?some=parameter", clientUrl.toString());
>
> - errorAttributes = ErrorAttributes.of(httpServletRequest);
> + // simulates a request that has errors metadata
> + httpRequest.setAttribute("javax.servlet.error.request_uri",
> + "/" + httpRequest.getContextPath() + "/any/source/of/error");
> + ServletWebRequest errorWebRequest = new ServletWebRequest(httpRequest, "/");
> + Url errorClientUrl = errorWebRequest.getClientUrl();
>
> - if (url != null)
> - {
> - this.url = url;
> - }
> - else
> - {
> - this.url = getUrl(httpServletRequest, filterPrefix);
> - }
> + assertEquals("any/source/of/error", errorClientUrl.toString());
> }
>
> /**
> - * Returns base url without context or filter mapping.
> - * <p>
> - * Example: if current url is
> - *
> - * <pre>
> - * http://localhost:8080/context/filter/mapping/wicket/bookmarkable/com.foo.Page?1&id=2
> - * </pre>
> - *
> - * the base url is <em>wicket/bookmarkable/com.foo.Page</em>
> - * </p>
> - *
> - * @see org.apache.wicket.request.Request#getClientUrl()
> + * <a href="https://issues.apache.org/jira/browse/WICKET-4168">WICKET-4168</a>
> */
> - @Override
> - public Url getClientUrl()
> + @Test
> + public void testClientURLIsContextRelativeInErrorResponses()
> {
> - if (errorAttributes != null && !Strings.isEmpty(errorAttributes.getRequestUri()))
> - {
> - return setParameters(Url.parse(errorAttributes.getRequestUri(), getCharset()));
> - }
> - else if (!isAjax())
> - {
> - return getUrl(httpServletRequest, filterPrefix);
> - }
> - else
> - {
> - String base = null;
> + MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null, null);
> + httpRequest.setURL(httpRequest.getContextPath() + "/request/Uri");
>
> - base = getHeader(HEADER_AJAX_BASE_URL);
> + String problematiURI = httpRequest.getContextPath() + "/any/source/of/error";
>
> - if (base == null)
> - {
> - base = getRequestParameters().getParameterValue(PARAM_AJAX_BASE_URL).toString(null);
> - }
> + httpRequest.setAttribute("javax.servlet.error.request_uri", problematiURI);
>
> - Checks.notNull(base, "Current ajax request is missing the base url header or parameter");
> + ServletWebRequest errorWebRequest = new ServletWebRequest(httpRequest, "");
>
> - return setParameters(Url.parse(base, getCharset()));
> - }
> - }
> + Url errorClientUrl = errorWebRequest.getClientUrl();
>
> - private Url setParameters(Url url)
> - {
> - url.setPort(httpServletRequest.getServerPort());
> - url.setHost(httpServletRequest.getServerName());
> - url.setProtocol(httpServletRequest.getScheme());
> - return url;
> - }
> + assertEquals("any/source/of/error", errorClientUrl.toString());
>
> - private Url getUrl(HttpServletRequest request, String filterPrefix)
> - {
> - if (filterPrefix.length() > 0 && !filterPrefix.endsWith("/"))
> - {
> - filterPrefix += "/";
> - }
> - StringBuilder url = new StringBuilder();
> - String uri = request.getRequestURI();
> - uri = Strings.stripJSessionId(uri);
> - final int start = request.getContextPath().length() + filterPrefix.length() + 1;
> - url.append(uri.substring(start));
> -
> - if (errorAttributes == null)
> - {
> - String query = request.getQueryString();
> - if (!Strings.isEmpty(query))
> - {
> - url.append('?');
> - url.append(query);
> - }
> - }
> -
> - return setParameters(Url.parse(url.toString(), getCharset()));
> }
>
> /**
> - * Returns the prefix of Wicket filter (without the leading /)
> - *
> - * @return Wicket filter prefix
> + * https://issues.apache.org/jira/browse/WICKET-4123
> */
> - public String getFilterPrefix()
> - {
> - return filterPrefix;
> - }
> -
> - @Override
> - public List<Cookie> getCookies()
> - {
> - Cookie[] cookies = httpServletRequest.getCookies();
> - List<Cookie> result = (cookies == null) ? Collections.<Cookie> emptyList()
> - : Arrays.asList(cookies);
> - return Collections.unmodifiableList(result);
> - }
> -
> -
> - @Override
> - public Locale getLocale()
> - {
> - return httpServletRequest.getLocale();
> - }
> -
> - @Override
> - public Time getDateHeader(String name)
> - {
> - try
> - {
> - long value = httpServletRequest.getDateHeader(name);
> -
> - if (value == -1)
> - {
> - return null;
> - }
> -
> - return Time.millis(value);
> - }
> - catch (IllegalArgumentException e)
> - {
> - // per spec thrown if the header contains a value that cannot be converted to a date
> - return null;
> - }
> - }
> -
> - @Override
> - public String getHeader(String name)
> + @Test
> + public void useCustomServletWebRequest()
> {
> - return httpServletRequest.getHeader(name);
> - }
> -
> - @SuppressWarnings("unchecked")
> - @Override
> - public List<String> getHeaders(String name)
> - {
> - List<String> result = new ArrayList<String>();
> - Enumeration<String> e = httpServletRequest.getHeaders(name);
> - while (e.hasMoreElements())
> - {
> - result.add(e.nextElement());
> - }
> - return Collections.unmodifiableList(result);
> - }
> -
> - private Map<String, List<StringValue>> postParameters = null;
> -
> - private static boolean isMultiPart(ServletRequest request)
> - {
> - String contentType = request.getContentType();
> - return contentType != null && contentType.toLowerCase().contains("multipart");
> - }
> -
> - protected Map<String, List<StringValue>> generatePostParameters()
> - {
> - Map<String, List<StringValue>> postParameters = new HashMap<String, List<StringValue>>();
> -
> - IRequestParameters queryParams = getQueryParameters();
> -
> - @SuppressWarnings("unchecked")
> - Map<String, String[]> params = getContainerRequest().getParameterMap();
> - for (Map.Entry<String, String[]> param : params.entrySet())
> + WebApplication application = new WebApplication()
> {
> - final String name = param.getKey();
> - final String[] values = param.getValue();
> -
> - // build a mutable list of query params that have the same name as the post param
> - List<StringValue> queryValues = queryParams.getParameterValues(name);
> - if (queryValues == null)
> - {
> - queryValues = Collections.emptyList();
> - }
> - else
> - {
> - queryValues = new ArrayList<StringValue>(queryValues);
> - }
> -
> - // the list that will contain accepted post param values
> - List<StringValue> postValues = new ArrayList<StringValue>();
> -
> - for (String value : values)
> + @Override
> + public Class<? extends Page> getHomePage()
> {
> - StringValue val = StringValue.valueOf(value);
> - if (queryValues.contains(val))
> - {
> - // if a query param with this value exists remove it and continue
> - queryValues.remove(val);
> - }
> - else
> - {
> - // there is no query param with this value, assume post
> - postValues.add(val);
> - }
> + return CustomRequestPage.class;
> }
>
> - if (!postValues.isEmpty())
> + @Override
> + protected WebRequest newWebRequest(HttpServletRequest servletRequest, String filterPath)
> {
> - postParameters.put(name, postValues);
> + return new CustomServletWebRequest(servletRequest, filterPath);
> }
> - }
> - return postParameters;
> - }
> + };
>
> - private Map<String, List<StringValue>> getPostRequestParameters()
> - {
> - if (postParameters == null)
> - {
> - postParameters = generatePostParameters();
> - }
> - return postParameters;
> + WicketTester tester = new WicketTester(application);
> + tester.startPage(new CustomRequestPage());
> }
>
> - private final IRequestParameters postRequestParameters = new IWritableRequestParameters()
> + private static class CustomRequestPage extends WebPage implements IMarkupResourceStreamProvider
> {
> - public void reset()
> - {
> - getPostRequestParameters().clear();
> - }
> + private static final long serialVersionUID = 1L;
>
> - public void setParameterValues(String key, List<StringValue> values)
> + private CustomRequestPage()
> {
> - getPostRequestParameters().put(key, values);
> + assertTrue(getRequest() instanceof CustomServletWebRequest);
> }
>
> - public Set<String> getParameterNames()
> + public IResourceStream getMarkupResourceStream(MarkupContainer container,
> + Class<?> containerClass)
> {
> - return Collections.unmodifiableSet(getPostRequestParameters().keySet());
> + return new StringResourceStream("<html></html>");
> }
> -
> - public StringValue getParameterValue(String name)
> - {
> - List<StringValue> values = getPostRequestParameters().get(name);
> - if (values == null || values.isEmpty())
> - {
> - return StringValue.valueOf((String)null);
> - }
> - else
> - {
> - return values.iterator().next();
> - }
> - }
> -
> - public List<StringValue> getParameterValues(String name)
> - {
> - List<StringValue> values = getPostRequestParameters().get(name);
> - if (values != null)
> - {
> - values = Collections.unmodifiableList(values);
> - }
> - return values;
> - }
> - };
> -
> - @Override
> - public IRequestParameters getPostParameters()
> - {
> - return postRequestParameters;
> - }
> -
> - @Override
> - public Url getUrl()
> - {
> - return new Url(url);
> }
>
> - @Override
> - public ServletWebRequest cloneWithUrl(Url url)
> + private static class CustomServletWebRequest extends ServletWebRequest
> {
> - return new ServletWebRequest(httpServletRequest, filterPrefix, url)
> + public CustomServletWebRequest(HttpServletRequest httpServletRequest, String filterPrefix)
> {
> - @Override
> - public IRequestParameters getPostParameters()
> - {
> - // don't parse post parameters again
> - return ServletWebRequest.this.getPostParameters();
> - }
> - };
> - }
> -
> - /**
> - * Creates multipart web request from this request.
> - *
> - * @param maxSize
> - * @param upload
> - * upload identifier for {@link UploadInfo}
> - * @return multipart request
> - * @throws FileUploadException
> - */
> - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload)
> - throws FileUploadException
> - {
> - return new MultipartServletWebRequestImpl(getContainerRequest(), filterPrefix, maxSize,
> - upload);
> - }
> -
> - /**
> - * Creates multipart web request from this request.
> - *
> - * @param maxSize
> - * @param upload
> - * upload identifier for {@link UploadInfo}
> - * @param factory
> - * @return multipart request
> - * @throws FileUploadException
> - */
> - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload,
> - FileItemFactory factory) throws FileUploadException
> - {
> - return new MultipartServletWebRequestImpl(getContainerRequest(), filterPrefix, maxSize,
> - upload, factory);
> - }
> -
> - private static final Logger logger = LoggerFactory.getLogger(ServletWebRequest.class);
> -
> - @Override
> - public String getPrefixToContextPath()
> - {
> - PrependingStringBuffer buffer = new PrependingStringBuffer();
> - Url filterPrefixUrl = Url.parse(filterPrefix, getCharset());
> - for (int i = 0; i < filterPrefixUrl.getSegments().size() - 1; ++i)
> - {
> - buffer.prepend("../");
> + super(httpServletRequest, filterPrefix);
> }
> - return buffer.toString();
> - }
> -
> - @Override
> - public Charset getCharset()
> - {
> - return RequestUtils.getCharset(httpServletRequest);
> - }
> -
> - @Override
> - public HttpServletRequest getContainerRequest()
> - {
> - return httpServletRequest;
> - }
> -
> - @Override
> - public String getContextPath()
> - {
> - return UrlUtils.normalizePath(httpServletRequest.getContextPath());
> - }
> -
> - @Override
> - public String getFilterPath()
> - {
> - return UrlUtils.normalizePath(filterPrefix);
> - }
> -
> - @Override
> - public boolean shouldPreserveClientUrl()
> - {
> - return errorAttributes != null && !Strings.isEmpty(errorAttributes.getRequestUri());
> }
> }
>
> Modified: wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java
> URL: http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java?rev=1190589&r1=1190588&r2=1190589&view=diff
> ==============================================================================
> --- wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java (original)
> +++ wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java Fri Oct 28 21:31:23 2011
> @@ -54,12 +54,34 @@ public class ServletWebRequestTest exten
> Url clientUrl = webRequest.getClientUrl();
> assertEquals("request/Uri?some=parameter", clientUrl.toString());
>
> - // error dispatched
> - httpRequest.setAttribute("javax.servlet.error.request_uri", "/some/error/url");
> + // simulates a request that has errors metadata
> + httpRequest.setAttribute("javax.servlet.error.request_uri",
> + "/" + httpRequest.getContextPath() + "/any/source/of/error");
> ServletWebRequest errorWebRequest = new ServletWebRequest(httpRequest, "/");
> Url errorClientUrl = errorWebRequest.getClientUrl();
>
> - assertEquals("/some/error/url", errorClientUrl.toString());
> + assertEquals("any/source/of/error", errorClientUrl.toString());
> + }
> +
> + /**
> + * <a href="https://issues.apache.org/jira/browse/WICKET-4168">WICKET-4168</a>
> + */
> + @Test
> + public void testClientURLIsContextRelativeInErrorResponses()
> + {
> + MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null, null);
> + httpRequest.setURL(httpRequest.getContextPath() + "/request/Uri");
> +
> + String problematiURI = httpRequest.getContextPath() + "/any/source/of/error";
> +
> + httpRequest.setAttribute("javax.servlet.error.request_uri", problematiURI);
> +
> + ServletWebRequest errorWebRequest = new ServletWebRequest(httpRequest, "");
> +
> + Url errorClientUrl = errorWebRequest.getClientUrl();
> +
> + assertEquals("any/source/of/error", errorClientUrl.toString());
> +
> }
>
> /**
>
>
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com
Re: svn commit: r1190589 - in /wicket/trunk/wicket-core/src:
main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java
Posted by Martin Grigorov <mg...@apache.org>.
Now I see there is a following commit which reverts the changes in the diff.
So it seems it was bad commit, not just bad diff for commit...
All is fine.
On Mon, Oct 31, 2011 at 3:59 PM, Martin Grigorov <mg...@apache.org> wrote:
> Hi Pedro,
>
> How exactly you did this change ?
> The diff for ServletWebRequest below looks wrong and scary. I see that
> the actual source of that class is OK but for some reason the diff is
> wrong.
> Even ViewVC shows the same problem:
> http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java?r1=1186162&r2=1190589&diff_format=h
>
> I remember once Peter Ertl also confused me with such commit notification.
> Can you remember the steps you did to commit this change ?
> I would like to avoid them in future :-)
>
> On Sat, Oct 29, 2011 at 12:31 AM, <pe...@apache.org> wrote:
>> Author: pedro
>> Date: Fri Oct 28 21:31:23 2011
>> New Revision: 1190589
>>
>> URL: http://svn.apache.org/viewvc?rev=1190589&view=rev
>> Log:
>> Making sure that client URL is context relative even while responding errors
>> Issue: WICKET-4168
>>
>> Modified:
>> wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java
>> wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java
>>
>> Modified: wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java
>> URL: http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java?rev=1190589&r1=1190588&r2=1190589&view=diff
>> ==============================================================================
>> --- wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java (original)
>> +++ wicket/trunk/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java Fri Oct 28 21:31:23 2011
>> @@ -16,445 +16,120 @@
>> */
>> package org.apache.wicket.protocol.http.servlet;
>>
>> -import java.nio.charset.Charset;
>> -import java.util.ArrayList;
>> -import java.util.Arrays;
>> -import java.util.Collections;
>> -import java.util.Enumeration;
>> -import java.util.HashMap;
>> -import java.util.List;
>> -import java.util.Locale;
>> -import java.util.Map;
>> -import java.util.Set;
>> -
>> -import javax.servlet.ServletRequest;
>> -import javax.servlet.http.Cookie;
>> import javax.servlet.http.HttpServletRequest;
>>
>> -import org.apache.wicket.protocol.http.RequestUtils;
>> -import org.apache.wicket.request.IRequestParameters;
>> -import org.apache.wicket.request.IWritableRequestParameters;
>> +import org.apache.wicket.MarkupContainer;
>> +import org.apache.wicket.Page;
>> +import org.apache.wicket.markup.IMarkupResourceStreamProvider;
>> +import org.apache.wicket.markup.html.WebPage;
>> +import org.apache.wicket.protocol.http.WebApplication;
>> +import org.apache.wicket.protocol.http.mock.MockHttpServletRequest;
>> import org.apache.wicket.request.Url;
>> import org.apache.wicket.request.http.WebRequest;
>> -import org.apache.wicket.util.lang.Args;
>> -import org.apache.wicket.util.lang.Bytes;
>> -import org.apache.wicket.util.lang.Checks;
>> -import org.apache.wicket.util.string.PrependingStringBuffer;
>> -import org.apache.wicket.util.string.StringValue;
>> -import org.apache.wicket.util.string.Strings;
>> -import org.apache.wicket.util.string.UrlUtils;
>> -import org.apache.wicket.util.time.Time;
>> -import org.apache.wicket.util.upload.FileItemFactory;
>> -import org.apache.wicket.util.upload.FileUploadException;
>> -import org.slf4j.Logger;
>> -import org.slf4j.LoggerFactory;
>> +import org.apache.wicket.util.resource.IResourceStream;
>> +import org.apache.wicket.util.resource.StringResourceStream;
>> +import org.apache.wicket.util.tester.WicketTester;
>> +import org.junit.Assert;
>> +import org.junit.Test;
>>
>> /**
>> - * {@link WebRequest} subclass that wraps a {@link HttpServletRequest} object.
>> - *
>> - * @author Matej Knopp
>> - * @author Juergen Donnerstag
>> - * @author Igor Vaynberg
>> + * Tests for {@link ServletWebRequest}
>> */
>> -public class ServletWebRequest extends WebRequest
>> +public class ServletWebRequestTest extends Assert
>> {
>> - private final HttpServletRequest httpServletRequest;
>> -
>> - private final Url url;
>> -
>> - private final String filterPrefix;
>> -
>> - private final ErrorAttributes errorAttributes;
>> -
>> - /**
>> - * Construct.
>> - *
>> - * @param httpServletRequest
>> - * @param filterPrefix
>> - * contentPath + filterPath, used to extract the actual {@link Url}
>> - */
>> - public ServletWebRequest(HttpServletRequest httpServletRequest, String filterPrefix)
>> - {
>> - this(httpServletRequest, filterPrefix, null);
>> - }
>>
>> /**
>> - * Construct.
>> - *
>> - * @param httpServletRequest
>> - * @param filterPrefix
>> - * contentPath + filterPath, used to extract the actual {@link Url}
>> - * @param url
>> + * Tests that {@link ServletWebRequest#getClientUrl()} returns the current url + the query
>> + * string when this is not error dispatched request. When the request is error dispatched it
>> + * returns just the request uri to the error page without the query string
>> */
>> - public ServletWebRequest(HttpServletRequest httpServletRequest, String filterPrefix, Url url)
>> + @Test
>> + public void wicket3599()
>> {
>> - Args.notNull(httpServletRequest, "httpServletRequest");
>> - Args.notNull(filterPrefix, "filterPrefix");
>> + MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null, null);
>> + httpRequest.setURL("/" + httpRequest.getContextPath() + "/request/Uri");
>> + httpRequest.setParameter("some", "parameter");
>>
>> - this.httpServletRequest = httpServletRequest;
>> - this.filterPrefix = filterPrefix;
>> + ServletWebRequest webRequest = new ServletWebRequest(httpRequest, "/");
>> + Url clientUrl = webRequest.getClientUrl();
>> + assertEquals("request/Uri?some=parameter", clientUrl.toString());
>>
>> - errorAttributes = ErrorAttributes.of(httpServletRequest);
>> + // simulates a request that has errors metadata
>> + httpRequest.setAttribute("javax.servlet.error.request_uri",
>> + "/" + httpRequest.getContextPath() + "/any/source/of/error");
>> + ServletWebRequest errorWebRequest = new ServletWebRequest(httpRequest, "/");
>> + Url errorClientUrl = errorWebRequest.getClientUrl();
>>
>> - if (url != null)
>> - {
>> - this.url = url;
>> - }
>> - else
>> - {
>> - this.url = getUrl(httpServletRequest, filterPrefix);
>> - }
>> + assertEquals("any/source/of/error", errorClientUrl.toString());
>> }
>>
>> /**
>> - * Returns base url without context or filter mapping.
>> - * <p>
>> - * Example: if current url is
>> - *
>> - * <pre>
>> - * http://localhost:8080/context/filter/mapping/wicket/bookmarkable/com.foo.Page?1&id=2
>> - * </pre>
>> - *
>> - * the base url is <em>wicket/bookmarkable/com.foo.Page</em>
>> - * </p>
>> - *
>> - * @see org.apache.wicket.request.Request#getClientUrl()
>> + * <a href="https://issues.apache.org/jira/browse/WICKET-4168">WICKET-4168</a>
>> */
>> - @Override
>> - public Url getClientUrl()
>> + @Test
>> + public void testClientURLIsContextRelativeInErrorResponses()
>> {
>> - if (errorAttributes != null && !Strings.isEmpty(errorAttributes.getRequestUri()))
>> - {
>> - return setParameters(Url.parse(errorAttributes.getRequestUri(), getCharset()));
>> - }
>> - else if (!isAjax())
>> - {
>> - return getUrl(httpServletRequest, filterPrefix);
>> - }
>> - else
>> - {
>> - String base = null;
>> + MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null, null);
>> + httpRequest.setURL(httpRequest.getContextPath() + "/request/Uri");
>>
>> - base = getHeader(HEADER_AJAX_BASE_URL);
>> + String problematiURI = httpRequest.getContextPath() + "/any/source/of/error";
>>
>> - if (base == null)
>> - {
>> - base = getRequestParameters().getParameterValue(PARAM_AJAX_BASE_URL).toString(null);
>> - }
>> + httpRequest.setAttribute("javax.servlet.error.request_uri", problematiURI);
>>
>> - Checks.notNull(base, "Current ajax request is missing the base url header or parameter");
>> + ServletWebRequest errorWebRequest = new ServletWebRequest(httpRequest, "");
>>
>> - return setParameters(Url.parse(base, getCharset()));
>> - }
>> - }
>> + Url errorClientUrl = errorWebRequest.getClientUrl();
>>
>> - private Url setParameters(Url url)
>> - {
>> - url.setPort(httpServletRequest.getServerPort());
>> - url.setHost(httpServletRequest.getServerName());
>> - url.setProtocol(httpServletRequest.getScheme());
>> - return url;
>> - }
>> + assertEquals("any/source/of/error", errorClientUrl.toString());
>>
>> - private Url getUrl(HttpServletRequest request, String filterPrefix)
>> - {
>> - if (filterPrefix.length() > 0 && !filterPrefix.endsWith("/"))
>> - {
>> - filterPrefix += "/";
>> - }
>> - StringBuilder url = new StringBuilder();
>> - String uri = request.getRequestURI();
>> - uri = Strings.stripJSessionId(uri);
>> - final int start = request.getContextPath().length() + filterPrefix.length() + 1;
>> - url.append(uri.substring(start));
>> -
>> - if (errorAttributes == null)
>> - {
>> - String query = request.getQueryString();
>> - if (!Strings.isEmpty(query))
>> - {
>> - url.append('?');
>> - url.append(query);
>> - }
>> - }
>> -
>> - return setParameters(Url.parse(url.toString(), getCharset()));
>> }
>>
>> /**
>> - * Returns the prefix of Wicket filter (without the leading /)
>> - *
>> - * @return Wicket filter prefix
>> + * https://issues.apache.org/jira/browse/WICKET-4123
>> */
>> - public String getFilterPrefix()
>> - {
>> - return filterPrefix;
>> - }
>> -
>> - @Override
>> - public List<Cookie> getCookies()
>> - {
>> - Cookie[] cookies = httpServletRequest.getCookies();
>> - List<Cookie> result = (cookies == null) ? Collections.<Cookie> emptyList()
>> - : Arrays.asList(cookies);
>> - return Collections.unmodifiableList(result);
>> - }
>> -
>> -
>> - @Override
>> - public Locale getLocale()
>> - {
>> - return httpServletRequest.getLocale();
>> - }
>> -
>> - @Override
>> - public Time getDateHeader(String name)
>> - {
>> - try
>> - {
>> - long value = httpServletRequest.getDateHeader(name);
>> -
>> - if (value == -1)
>> - {
>> - return null;
>> - }
>> -
>> - return Time.millis(value);
>> - }
>> - catch (IllegalArgumentException e)
>> - {
>> - // per spec thrown if the header contains a value that cannot be converted to a date
>> - return null;
>> - }
>> - }
>> -
>> - @Override
>> - public String getHeader(String name)
>> + @Test
>> + public void useCustomServletWebRequest()
>> {
>> - return httpServletRequest.getHeader(name);
>> - }
>> -
>> - @SuppressWarnings("unchecked")
>> - @Override
>> - public List<String> getHeaders(String name)
>> - {
>> - List<String> result = new ArrayList<String>();
>> - Enumeration<String> e = httpServletRequest.getHeaders(name);
>> - while (e.hasMoreElements())
>> - {
>> - result.add(e.nextElement());
>> - }
>> - return Collections.unmodifiableList(result);
>> - }
>> -
>> - private Map<String, List<StringValue>> postParameters = null;
>> -
>> - private static boolean isMultiPart(ServletRequest request)
>> - {
>> - String contentType = request.getContentType();
>> - return contentType != null && contentType.toLowerCase().contains("multipart");
>> - }
>> -
>> - protected Map<String, List<StringValue>> generatePostParameters()
>> - {
>> - Map<String, List<StringValue>> postParameters = new HashMap<String, List<StringValue>>();
>> -
>> - IRequestParameters queryParams = getQueryParameters();
>> -
>> - @SuppressWarnings("unchecked")
>> - Map<String, String[]> params = getContainerRequest().getParameterMap();
>> - for (Map.Entry<String, String[]> param : params.entrySet())
>> + WebApplication application = new WebApplication()
>> {
>> - final String name = param.getKey();
>> - final String[] values = param.getValue();
>> -
>> - // build a mutable list of query params that have the same name as the post param
>> - List<StringValue> queryValues = queryParams.getParameterValues(name);
>> - if (queryValues == null)
>> - {
>> - queryValues = Collections.emptyList();
>> - }
>> - else
>> - {
>> - queryValues = new ArrayList<StringValue>(queryValues);
>> - }
>> -
>> - // the list that will contain accepted post param values
>> - List<StringValue> postValues = new ArrayList<StringValue>();
>> -
>> - for (String value : values)
>> + @Override
>> + public Class<? extends Page> getHomePage()
>> {
>> - StringValue val = StringValue.valueOf(value);
>> - if (queryValues.contains(val))
>> - {
>> - // if a query param with this value exists remove it and continue
>> - queryValues.remove(val);
>> - }
>> - else
>> - {
>> - // there is no query param with this value, assume post
>> - postValues.add(val);
>> - }
>> + return CustomRequestPage.class;
>> }
>>
>> - if (!postValues.isEmpty())
>> + @Override
>> + protected WebRequest newWebRequest(HttpServletRequest servletRequest, String filterPath)
>> {
>> - postParameters.put(name, postValues);
>> + return new CustomServletWebRequest(servletRequest, filterPath);
>> }
>> - }
>> - return postParameters;
>> - }
>> + };
>>
>> - private Map<String, List<StringValue>> getPostRequestParameters()
>> - {
>> - if (postParameters == null)
>> - {
>> - postParameters = generatePostParameters();
>> - }
>> - return postParameters;
>> + WicketTester tester = new WicketTester(application);
>> + tester.startPage(new CustomRequestPage());
>> }
>>
>> - private final IRequestParameters postRequestParameters = new IWritableRequestParameters()
>> + private static class CustomRequestPage extends WebPage implements IMarkupResourceStreamProvider
>> {
>> - public void reset()
>> - {
>> - getPostRequestParameters().clear();
>> - }
>> + private static final long serialVersionUID = 1L;
>>
>> - public void setParameterValues(String key, List<StringValue> values)
>> + private CustomRequestPage()
>> {
>> - getPostRequestParameters().put(key, values);
>> + assertTrue(getRequest() instanceof CustomServletWebRequest);
>> }
>>
>> - public Set<String> getParameterNames()
>> + public IResourceStream getMarkupResourceStream(MarkupContainer container,
>> + Class<?> containerClass)
>> {
>> - return Collections.unmodifiableSet(getPostRequestParameters().keySet());
>> + return new StringResourceStream("<html></html>");
>> }
>> -
>> - public StringValue getParameterValue(String name)
>> - {
>> - List<StringValue> values = getPostRequestParameters().get(name);
>> - if (values == null || values.isEmpty())
>> - {
>> - return StringValue.valueOf((String)null);
>> - }
>> - else
>> - {
>> - return values.iterator().next();
>> - }
>> - }
>> -
>> - public List<StringValue> getParameterValues(String name)
>> - {
>> - List<StringValue> values = getPostRequestParameters().get(name);
>> - if (values != null)
>> - {
>> - values = Collections.unmodifiableList(values);
>> - }
>> - return values;
>> - }
>> - };
>> -
>> - @Override
>> - public IRequestParameters getPostParameters()
>> - {
>> - return postRequestParameters;
>> - }
>> -
>> - @Override
>> - public Url getUrl()
>> - {
>> - return new Url(url);
>> }
>>
>> - @Override
>> - public ServletWebRequest cloneWithUrl(Url url)
>> + private static class CustomServletWebRequest extends ServletWebRequest
>> {
>> - return new ServletWebRequest(httpServletRequest, filterPrefix, url)
>> + public CustomServletWebRequest(HttpServletRequest httpServletRequest, String filterPrefix)
>> {
>> - @Override
>> - public IRequestParameters getPostParameters()
>> - {
>> - // don't parse post parameters again
>> - return ServletWebRequest.this.getPostParameters();
>> - }
>> - };
>> - }
>> -
>> - /**
>> - * Creates multipart web request from this request.
>> - *
>> - * @param maxSize
>> - * @param upload
>> - * upload identifier for {@link UploadInfo}
>> - * @return multipart request
>> - * @throws FileUploadException
>> - */
>> - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload)
>> - throws FileUploadException
>> - {
>> - return new MultipartServletWebRequestImpl(getContainerRequest(), filterPrefix, maxSize,
>> - upload);
>> - }
>> -
>> - /**
>> - * Creates multipart web request from this request.
>> - *
>> - * @param maxSize
>> - * @param upload
>> - * upload identifier for {@link UploadInfo}
>> - * @param factory
>> - * @return multipart request
>> - * @throws FileUploadException
>> - */
>> - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload,
>> - FileItemFactory factory) throws FileUploadException
>> - {
>> - return new MultipartServletWebRequestImpl(getContainerRequest(), filterPrefix, maxSize,
>> - upload, factory);
>> - }
>> -
>> - private static final Logger logger = LoggerFactory.getLogger(ServletWebRequest.class);
>> -
>> - @Override
>> - public String getPrefixToContextPath()
>> - {
>> - PrependingStringBuffer buffer = new PrependingStringBuffer();
>> - Url filterPrefixUrl = Url.parse(filterPrefix, getCharset());
>> - for (int i = 0; i < filterPrefixUrl.getSegments().size() - 1; ++i)
>> - {
>> - buffer.prepend("../");
>> + super(httpServletRequest, filterPrefix);
>> }
>> - return buffer.toString();
>> - }
>> -
>> - @Override
>> - public Charset getCharset()
>> - {
>> - return RequestUtils.getCharset(httpServletRequest);
>> - }
>> -
>> - @Override
>> - public HttpServletRequest getContainerRequest()
>> - {
>> - return httpServletRequest;
>> - }
>> -
>> - @Override
>> - public String getContextPath()
>> - {
>> - return UrlUtils.normalizePath(httpServletRequest.getContextPath());
>> - }
>> -
>> - @Override
>> - public String getFilterPath()
>> - {
>> - return UrlUtils.normalizePath(filterPrefix);
>> - }
>> -
>> - @Override
>> - public boolean shouldPreserveClientUrl()
>> - {
>> - return errorAttributes != null && !Strings.isEmpty(errorAttributes.getRequestUri());
>> }
>> }
>>
>> Modified: wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java
>> URL: http://svn.apache.org/viewvc/wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java?rev=1190589&r1=1190588&r2=1190589&view=diff
>> ==============================================================================
>> --- wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java (original)
>> +++ wicket/trunk/wicket-core/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java Fri Oct 28 21:31:23 2011
>> @@ -54,12 +54,34 @@ public class ServletWebRequestTest exten
>> Url clientUrl = webRequest.getClientUrl();
>> assertEquals("request/Uri?some=parameter", clientUrl.toString());
>>
>> - // error dispatched
>> - httpRequest.setAttribute("javax.servlet.error.request_uri", "/some/error/url");
>> + // simulates a request that has errors metadata
>> + httpRequest.setAttribute("javax.servlet.error.request_uri",
>> + "/" + httpRequest.getContextPath() + "/any/source/of/error");
>> ServletWebRequest errorWebRequest = new ServletWebRequest(httpRequest, "/");
>> Url errorClientUrl = errorWebRequest.getClientUrl();
>>
>> - assertEquals("/some/error/url", errorClientUrl.toString());
>> + assertEquals("any/source/of/error", errorClientUrl.toString());
>> + }
>> +
>> + /**
>> + * <a href="https://issues.apache.org/jira/browse/WICKET-4168">WICKET-4168</a>
>> + */
>> + @Test
>> + public void testClientURLIsContextRelativeInErrorResponses()
>> + {
>> + MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null, null);
>> + httpRequest.setURL(httpRequest.getContextPath() + "/request/Uri");
>> +
>> + String problematiURI = httpRequest.getContextPath() + "/any/source/of/error";
>> +
>> + httpRequest.setAttribute("javax.servlet.error.request_uri", problematiURI);
>> +
>> + ServletWebRequest errorWebRequest = new ServletWebRequest(httpRequest, "");
>> +
>> + Url errorClientUrl = errorWebRequest.getClientUrl();
>> +
>> + assertEquals("any/source/of/error", errorClientUrl.toString());
>> +
>> }
>>
>> /**
>>
>>
>>
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com
>
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com