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, <[email protected]> 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
