Hi Folks, I could be mistaken, but this seems like a very major change that did not have a thorough and proper discussion at the mailing list? I would rather at least have an explanation of what was committed and to discuss the merits and cons of the implementation.
Thanks, Taher On Tue, Jun 26, 2018 at 5:00 AM, <sh...@apache.org> wrote: > Author: shijh > Date: Tue Jun 26 02:00:33 2018 > New Revision: 1834389 > > URL: http://svn.apache.org/viewvc?rev=1834389&view=rev > Log: > Implemented: Add method attribute to request-map to controll a uri can be > called GET or POST only > OFBIZ-10438 > > The request-map element has a new method attribute to control a uri be called > by GET or POST or all. > > Thanks: Mathieu Lirzin for the contribution. > > Added: > > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MultiValuedMapContext.java > (with props) > > ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java > (with props) > Modified: > ofbiz/ofbiz-framework/trunk/framework/webapp/config/WebappUiLabels.xml > ofbiz/ofbiz-framework/trunk/framework/webapp/dtd/site-conf.xsd > > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java > > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlServlet.java > > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java > > Added: > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MultiValuedMapContext.java > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MultiValuedMapContext.java?rev=1834389&view=auto > ============================================================================== > --- > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MultiValuedMapContext.java > (added) > +++ > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MultiValuedMapContext.java > Tue Jun 26 02:00:33 2018 > @@ -0,0 +1,249 @@ > +/******************************************************************************* > + * Licensed to the Apache Software Foundation (ASF) under one > + * or more contributor license agreements. See the NOTICE file > + * distributed with this work for additional information > + * regarding copyright ownership. The ASF licenses this file > + * to you under the Apache License, Version 2.0 (the > + * "License"); you may not use this file except in compliance > + * with the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, > + * software distributed under the License is distributed on an > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > + * KIND, either express or implied. See the License for the > + * specific language governing permissions and limitations > + * under the License. > + > *******************************************************************************/ > +package org.apache.ofbiz.base.util.collections; > + > +import java.util.Collection; > +import java.util.Deque; > +import java.util.HashSet; > +import java.util.Iterator; > +import java.util.LinkedList; > +import java.util.List; > +import java.util.Locale; > +import java.util.Map; > +import java.util.Objects; > +import java.util.Set; > +import java.util.stream.Collectors; > +import java.util.stream.Stream; > +import java.util.stream.StreamSupport; > + > +import javax.ws.rs.core.MultivaluedHashMap; > +import javax.ws.rs.core.MultivaluedMap; > + > +import org.apache.ofbiz.base.util.UtilGenerics; > + > +/** > + * Map Stack > + * > + */ > +public class MultiValuedMapContext<K, V> implements MultivaluedMap<K, V>, > LocalizedMap<V> { > + > + public static final String module = > MultiValuedMapContext.class.getName(); > + > + public static final <K, V> MultiValuedMapContext<K, V> getMapContext() { > + return new MultiValuedMapContext<>(); > + } > + > + public static <K, V> MultiValuedMapContext<K, V> createMapContext() { > + MultiValuedMapContext<K, V> newValue = > MultiValuedMapContext.getMapContext(); > + // initialize with a single entry > + newValue.push(); > + return newValue; > + } > + > + public static <K, V> MultiValuedMapContext<K, V> > createMapContext(MultivaluedMap<K, V> baseMap) { > + if (baseMap instanceof MultiValuedMapContext) { > + return createMapContext((MultiValuedMapContext<K, V>) baseMap); > + } else { > + MultiValuedMapContext<K, V> newValue = > MultiValuedMapContext.getMapContext(); > + newValue.maps.addFirst(baseMap); > + return newValue; > + } > + } > + > + /** Does a shallow copy of the internal stack of the passed MapContext; > enables simultaneous stacks that share common parent Maps */ > + public static <K, V> MultiValuedMapContext<K, V> > createMapContext(MultiValuedMapContext<K, V> source) { > + MultiValuedMapContext<K, V> newValue = > MultiValuedMapContext.getMapContext(); > + newValue.maps.addAll(source.maps); > + return newValue; > + } > + > + protected MultiValuedMapContext() { > + super(); > + } > + > + protected Deque<MultivaluedMap<K, V>> maps = new LinkedList<>(); > + > + public void reset() { > + maps = new LinkedList<>(); > + } > + > + /** Puts a new Map on the top of the stack */ > + public void push() { > + MultivaluedMap<K, V> newMap = new MultivaluedHashMap<>(); > + this.maps.addFirst(newMap); > + } > + > + /** Puts an existing Map on the top of the stack (top meaning will > override lower layers on the stack) */ > + public void push(MultivaluedMap<K, V> existingMap) { > + if (existingMap == null) { > + throw new IllegalArgumentException("Error: cannot push null > existing Map onto a MapContext"); > + } > + this.maps.addFirst(existingMap); > + } > + > + /** Puts an existing Map on the BOTTOM of the stack (bottom meaning will > be overriden by lower layers on the stack, ie everything else already there) > */ > + public void addToBottom(MultivaluedMap<K, V> existingMap) { > + if (existingMap == null) { > + throw new IllegalArgumentException("Error: cannot add null > existing Map to bottom of a MapContext"); > + } > + this.maps.add(existingMap); > + } > + > + /** Remove and returns the Map from the top of the stack; if there is > only one Map on the stack it returns null and does not remove it */ > + public MultivaluedMap<K, V> pop() { > + // always leave at least one Map in the List, ie never pop off the > last Map > + return (maps.size() < 1) ? null : maps.removeFirst(); > + } > + > + /** > + * Creates a MapContext object that has the same Map objects on its > stack; > + * meant to be used to enable a > + * situation where a parent and child context are operating > simultaneously > + * using two different MapContext objects, but sharing the Maps in common > + */ > + public MultiValuedMapContext<K, V> standAloneStack() { > + MultiValuedMapContext<K, V> standAlone = > MultiValuedMapContext.createMapContext(this); > + return standAlone; > + } > + > + /** > + * Creates a MapContext object that has the same Map objects on its > stack, > + * but with a new Map pushed on the top; meant to be used to enable a > + * situation where a parent and child context are operating > simultaneously > + * using two different MapContext objects, but sharing the Maps in common > + */ > + public MultiValuedMapContext<K, V> standAloneChildStack() { > + MultiValuedMapContext<K, V> standAloneChild = > MultiValuedMapContext.createMapContext(this); > + standAloneChild.push(); > + return standAloneChild; > + } > + > + @Override > + public int size() { > + return keySet().size(); > + } > + > + @Override > + public boolean isEmpty() { > + return maps.stream().allMatch(MultivaluedMap::isEmpty); > + } > + > + @Override > + public boolean containsKey(Object key) { > + return maps.stream().anyMatch(map -> map.containsKey(key)); > + } > + > + @Override > + public boolean containsValue(Object value) { > + return maps.stream().anyMatch(map -> map.containsValue(value)); > + } > + > + @Override > + public List<V> get(Object key) { > + return maps.stream() > + .filter(m -> m.containsKey(key)) > + .flatMap(m -> m.get(key).stream()) > + .collect(Collectors.toList()); > + } > + > + @SuppressWarnings("unchecked") > + @Override > + public V get(String name, Locale locale) { > + for (MultivaluedMap<K, V> curMap: maps) { > + // only return if the curMap contains the key, rather than > checking for null; this allows a null at a lower level to override a value at > a higher level > + if (curMap.containsKey(name)) { > + if (curMap instanceof LocalizedMap<?>) { > + LocalizedMap<V> lmap = UtilGenerics.cast(curMap); > + return lmap.get(name, locale); > + } > + return curMap.get((K) name).stream().findFirst().get(); > + } > + } > + return null; > + } > + > + @Override > + public List<V> remove(Object key) { > + return maps.getFirst().remove(key); > + } > + > + @Override > + public void clear() { > + maps.getFirst().clear(); > + } > + > + // Convert an iterator to a stream. > + private static <U> Stream<U> stream(Iterator<U> it) { > + Iterable<U> dmaps = () -> it; > + return StreamSupport.stream(dmaps.spliterator(), false); > + } > + > + @Override > + public Set<K> keySet() { > + // Collect in reverse order to let the first maps masks the next > ones. > + return stream(maps.descendingIterator()) > + .flatMap(m -> m.keySet().stream()) > + .collect(Collectors.toCollection(HashSet::new)); > + } > + > + @Override > + public List<V> put(K key, List<V> value) { > + return maps.getFirst().put(key, value); > + } > + > + @Override > + public void putAll(Map<? extends K, ? extends List<V>> m) { > + maps.getFirst().putAll(m); > + } > + > + @Override > + public Set<Entry<K, List<V>>> entrySet() { > + // Collect in reverse order to let the first maps masks the next > ones. > + return stream(maps.descendingIterator()) > + .flatMap(m -> m.entrySet().stream()) > + .collect(Collectors.toCollection(HashSet::new)); > + } > + > + @Override > + public void putSingle(K key, V value) { > + maps.getFirst().putSingle(key, value); > + } > + > + @Override > + public void add(K key, V value) { > + maps.getFirst().add(key, value); > + } > + > + @Override > + public V getFirst(K key) { > + return maps.stream() > + .map(m -> m.get(key)) > + .filter(Objects::nonNull) > + .flatMap(List::stream) > + .findFirst() > + .orElse(null); > + } > + > + @Override > + public Collection<List<V>> values() { > + return maps.stream() > + .flatMap(m -> m.values().stream()) > + .collect(Collectors.toList()); > + } > +} > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MultiValuedMapContext.java > ------------------------------------------------------------------------------ > svn:eol-style = native > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MultiValuedMapContext.java > ------------------------------------------------------------------------------ > svn:keywords = Date Rev Author URL Id > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MultiValuedMapContext.java > ------------------------------------------------------------------------------ > svn:mime-type = text/plain > > Modified: > ofbiz/ofbiz-framework/trunk/framework/webapp/config/WebappUiLabels.xml > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/config/WebappUiLabels.xml?rev=1834389&r1=1834388&r2=1834389&view=diff > ============================================================================== > --- ofbiz/ofbiz-framework/trunk/framework/webapp/config/WebappUiLabels.xml > (original) > +++ ofbiz/ofbiz-framework/trunk/framework/webapp/config/WebappUiLabels.xml > Tue Jun 26 02:00:33 2018 > @@ -339,4 +339,8 @@ > <value xml:lang="zh">æ²¡æœ‰å®Œæˆ äº‹ä»¶</value> > <value xml:lang="zh-TW">æ²’æœ‰å®Œæˆ äº‹ä»¶</value> > </property> > + <property key="RequestMethodNotMatchConfig"> > + <value xml:lang="en">[{0}] is restricted to be a [{1}] request, > cannot be called by [{2}] method.</value> > + <value xml:lang="zh">[{0}]必须使用[{1}]æ–¹æ³•è¯·æ±‚ï¼Œä¸ > 能用[{2}]方法请求。</value> > + </property> > </resource> > > Modified: ofbiz/ofbiz-framework/trunk/framework/webapp/dtd/site-conf.xsd > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/dtd/site-conf.xsd?rev=1834389&r1=1834388&r2=1834389&view=diff > ============================================================================== > --- ofbiz/ofbiz-framework/trunk/framework/webapp/dtd/site-conf.xsd (original) > +++ ofbiz/ofbiz-framework/trunk/framework/webapp/dtd/site-conf.xsd Tue Jun 26 > 02:00:33 2018 > @@ -216,6 +216,24 @@ under the License. > </xs:documentation> > </xs:annotation> > </xs:attribute> > + <xs:attribute name="method" use="optional" default="all"> > + <xs:annotation> > + <xs:documentation> > + The HTTP of this request. This will be the HTTP method > used to access the request. > + </xs:documentation> > + </xs:annotation> > + <xs:simpleType> > + <xs:restriction base="xs:token"> > + <xs:enumeration value="get"/> > + <xs:enumeration value="post"/> > + <xs:enumeration value="put"/> > + <xs:enumeration value="delete"/> > + <xs:enumeration value="patch"/> > + <xs:enumeration value="options"/> > + <xs:enumeration value="all"/> > + </xs:restriction> > + </xs:simpleType> > + </xs:attribute> > <xs:attribute type="xs:boolean" name="edit" default="true"> > <xs:annotation> > <xs:documentation> > > Modified: > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java?rev=1834389&r1=1834388&r2=1834389&view=diff > ============================================================================== > --- > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java > (original) > +++ > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java > Tue Jun 26 02:00:33 2018 > @@ -23,15 +23,20 @@ import java.io.IOException; > import java.net.MalformedURLException; > import java.net.URL; > import java.util.ArrayList; > +import java.util.Collection; > import java.util.HashMap; > import java.util.HashSet; > import java.util.LinkedHashMap; > import java.util.LinkedList; > import java.util.List; > import java.util.Map; > +import java.util.Objects; > import java.util.Set; > +import java.util.stream.Collectors; > > import javax.servlet.ServletContext; > +import javax.ws.rs.core.MultivaluedHashMap; > +import javax.ws.rs.core.MultivaluedMap; > > import org.apache.ofbiz.base.component.ComponentConfig.WebappInfo; > import org.apache.ofbiz.base.location.FlexibleLocation; > @@ -48,6 +53,7 @@ import org.apache.ofbiz.base.util.UtilVa > import org.apache.ofbiz.base.util.UtilXml; > import org.apache.ofbiz.base.util.cache.UtilCache; > import org.apache.ofbiz.base.util.collections.MapContext; > +import org.apache.ofbiz.base.util.collections.MultiValuedMapContext; > import org.w3c.dom.Document; > import org.w3c.dom.Element; > > @@ -191,7 +197,7 @@ public class ConfigXMLReader { > private Map<String, Event> beforeLogoutEventList = new > LinkedHashMap<String, Event>(); > private Map<String, String> eventHandlerMap = new HashMap<String, > String>(); > private Map<String, String> viewHandlerMap = new HashMap<String, > String>(); > - private Map<String, RequestMap> requestMapMap = new HashMap<String, > RequestMap>(); > + private MultivaluedMap<String, RequestMap> requestMapMap = new > MultivaluedHashMap<>(); > private Map<String, ViewMap> viewMapMap = new HashMap<String, > ViewMap>(); > > public ControllerConfig(URL url) throws WebAppConfigurationException > { > @@ -328,11 +334,57 @@ public class ConfigXMLReader { > return null; > } > > + // XXX: Temporary wrapper to handle conversion from Map to MultiMap. > public Map<String, RequestMap> getRequestMapMap() throws > WebAppConfigurationException { > - MapContext<String, RequestMap> result = > MapContext.getMapContext(); > + return new Map<String, RequestMap> () { > + private MultivaluedMap<String, RequestMap> deleg = > getRequestMapMultiMap(); > + @Override public int size() { return deleg.size(); } > + @Override public boolean isEmpty() { return deleg.isEmpty(); > } > + @Override public boolean containsKey(Object key) { > + return deleg.containsKey(key); > + } > + @Override public boolean containsValue(Object value) { > + return deleg.containsValue(value); > + } > + @Override public RequestMap get(Object key) { > + return deleg.getFirst((String) key); > + } > + @Override public RequestMap put(String key, RequestMap > value) { > + RequestMap res = get(key); > + deleg.putSingle(key, value); > + return res; > + } > + @Override public RequestMap remove(Object key) { > + RequestMap res = get(key); > + deleg.remove(key); > + return res; > + } > + @Override public void putAll(Map<? extends String, ? extends > RequestMap> m) { > + m.forEach(deleg::add); > + } > + @Override public void clear() { deleg.clear(); } > + @Override public Set<String> keySet() { return > deleg.keySet(); } > + @Override public Collection<RequestMap> values() { > + return deleg.values() > + .stream() > + .map(m -> > m.stream().findFirst().orElse(null)) > + .filter(Objects::nonNull) > + .collect(Collectors.toList()); > + } > + @Override public Set<Entry<String, RequestMap>> entrySet() { > + return deleg.keySet() > + .stream() > + .collect(Collectors.toMap(k -> k, k -> > get(k))) > + .entrySet(); > + } > + }; > + } > + > + public MultivaluedMap<String, RequestMap> getRequestMapMultiMap() > throws WebAppConfigurationException { > + MultiValuedMapContext<String, RequestMap> result = > MultiValuedMapContext.getMapContext(); > for (URL includeLocation : includes) { > ControllerConfig controllerConfig = > getControllerConfig(includeLocation); > - result.push(controllerConfig.getRequestMapMap()); > + result.push(controllerConfig.getRequestMapMultiMap()); > } > result.push(requestMapMap); > return result; > @@ -487,7 +539,7 @@ public class ConfigXMLReader { > private void loadRequestMap(Element root) { > for (Element requestMapElement : UtilXml.childElementList(root, > "request-map")) { > RequestMap requestMap = new RequestMap(requestMapElement); > - this.requestMapMap.put(requestMap.uri, requestMap); > + this.requestMapMap.putSingle(requestMap.uri, requestMap); > } > } > > @@ -534,6 +586,7 @@ public class ConfigXMLReader { > > public static class RequestMap { > public String uri; > + public String method; > public boolean edit = true; > public boolean trackVisit = true; > public boolean trackServerHit = true; > @@ -550,6 +603,7 @@ public class ConfigXMLReader { > public RequestMap(Element requestMapElement) { > // Get the URI info > this.uri = requestMapElement.getAttribute("uri"); > + this.method = requestMapElement.getAttribute("method"); > this.edit = > !"false".equals(requestMapElement.getAttribute("edit")); > this.trackServerHit = > !"false".equals(requestMapElement.getAttribute("track-serverhit")); > this.trackVisit = > !"false".equals(requestMapElement.getAttribute("track-visit")); > > Modified: > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlServlet.java > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlServlet.java?rev=1834389&r1=1834388&r2=1834389&view=diff > ============================================================================== > --- > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlServlet.java > (original) > +++ > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlServlet.java > Tue Jun 26 02:00:33 2018 > @@ -19,6 +19,7 @@ > package org.apache.ofbiz.webapp.control; > > import java.io.IOException; > +import java.net.URL; > import java.util.Enumeration; > > import javax.servlet.RequestDispatcher; > @@ -29,11 +30,14 @@ import javax.servlet.http.HttpServlet; > import javax.servlet.http.HttpServletRequest; > import javax.servlet.http.HttpServletResponse; > import javax.servlet.http.HttpSession; > +import javax.ws.rs.core.MultivaluedMap; > > import org.apache.ofbiz.base.util.Debug; > import org.apache.ofbiz.base.util.UtilCodec; > import org.apache.ofbiz.base.util.UtilGenerics; > import org.apache.ofbiz.base.util.UtilHttp; > +import org.apache.ofbiz.base.util.UtilMisc; > +import org.apache.ofbiz.base.util.UtilProperties; > import org.apache.ofbiz.base.util.UtilTimer; > import org.apache.ofbiz.base.util.UtilValidate; > import org.apache.ofbiz.base.util.template.FreeMarkerWorker; > @@ -45,6 +49,7 @@ import org.apache.ofbiz.entity.transacti > import org.apache.ofbiz.entity.transaction.TransactionUtil; > import org.apache.ofbiz.security.Security; > import org.apache.ofbiz.service.LocalDispatcher; > +import org.apache.ofbiz.webapp.control.ConfigXMLReader.RequestMap; > import org.apache.ofbiz.webapp.stats.ServerHitBin; > import org.apache.ofbiz.webapp.stats.VisitHandler; > import org.apache.ofbiz.widget.renderer.VisualTheme; > @@ -58,6 +63,9 @@ import freemarker.ext.servlet.ServletCon > public class ControlServlet extends HttpServlet { > > public static final String module = ControlServlet.class.getName(); > + private static final String METHOD_GET = "GET"; > + private static final String METHOD_POST = "POST"; > + private static final String REQUEST_METHOD_ALL = "all"; > > public ControlServlet() { > super(); > @@ -80,18 +88,20 @@ public class ControlServlet extends Http > } > > /** > - * @see > javax.servlet.http.HttpServlet#doPost(javax.servlet.http.HttpServletRequest, > javax.servlet.http.HttpServletResponse) > + * Handle every HTTP methods. > + * @see javax.servlet.http.HttpServlet#service > */ > @Override > - public void doPost(HttpServletRequest request, HttpServletResponse > response) throws ServletException, IOException { > - doGet(request, response); > - } > - > - /** > - * @see > javax.servlet.http.HttpServlet#doGet(javax.servlet.http.HttpServletRequest, > javax.servlet.http.HttpServletResponse) > - */ > - @Override > - public void doGet(HttpServletRequest request, HttpServletResponse > response) throws ServletException, IOException { > + protected void service(HttpServletRequest request, HttpServletResponse > response) > + throws ServletException, IOException { > + String method = request.getMethod(); > + if (!matchRequestMethod(request, response, method)) { > + return; > + } > + if (!method.equals(METHOD_GET) && !method.equals(METHOD_POST)) { > + super.service(request, response); > + return; > + } > long requestStartTime = System.currentTimeMillis(); > RequestHandler requestHandler = this.getRequestHandler(); > HttpSession session = request.getSession(); > @@ -378,4 +388,29 @@ public class ControlServlet extends Http > } > if (Debug.verboseOn()) Debug.logVerbose("--- End ServletContext > Attributes ---", module); > } > + > + private boolean matchRequestMethod(HttpServletRequest request, > HttpServletResponse response, String method) { > + URL controllerConfigUrl = > ConfigXMLReader.getControllerConfigURL(request.getServletContext()); > + try { > + ConfigXMLReader.ControllerConfig controllerConfig = > ConfigXMLReader.getControllerConfig(controllerConfigUrl); > + MultivaluedMap<String, RequestMap> requestMapMap = > controllerConfig.getRequestMapMultiMap(); > + if (UtilValidate.isEmpty(requestMapMap)) { > + return true; > + } > + String uri = RequestHandler.getRequestUri(request.getPathInfo()); > + RequestMap requestMap = requestMapMap.getFirst(uri); > + if (UtilValidate.isEmpty(requestMap) || > UtilValidate.isEmpty(requestMap.method) || > REQUEST_METHOD_ALL.equalsIgnoreCase(requestMap.method) || > method.equalsIgnoreCase(requestMap.method)) { > + return true; > + } > + String errMsg = UtilProperties.getMessage("WebappUiLabels", > "RequestMethodNotMatchConfig", UtilMisc.toList(uri, requestMap.method, > method), UtilHttp.getLocale(request)); > + response.setContentType("text/plain"); > + response.setCharacterEncoding(request.getCharacterEncoding()); > + response.setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED); > + response.getWriter().print(errMsg); > + Debug.logError(errMsg, module); > + } catch (WebAppConfigurationException | IOException e) { > + Debug.logError("Error while loading " + controllerConfigUrl, > module); > + } > + return false; > + } > } > > Modified: > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java?rev=1834389&r1=1834388&r2=1834389&view=diff > ============================================================================== > --- > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java > (original) > +++ > ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java > Tue Jun 26 02:00:33 2018 > @@ -24,16 +24,21 @@ import java.io.IOException; > import java.io.Serializable; > import java.net.URL; > import java.security.cert.X509Certificate; > +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.Optional; > +import java.util.function.Predicate; > +import java.util.stream.Stream; > > import javax.servlet.ServletContext; > import javax.servlet.http.HttpServletRequest; > import javax.servlet.http.HttpServletResponse; > import javax.servlet.http.HttpSession; > +import javax.ws.rs.core.MultivaluedMap; > > import org.apache.ofbiz.base.util.Debug; > import org.apache.ofbiz.base.util.SSLUtil; > @@ -51,6 +56,7 @@ import org.apache.ofbiz.entity.GenericVa > import org.apache.ofbiz.entity.util.EntityQuery; > import org.apache.ofbiz.entity.util.EntityUtilProperties; > import org.apache.ofbiz.webapp.OfbizUrlBuilder; > +import org.apache.ofbiz.webapp.control.ConfigXMLReader.RequestMap; > import org.apache.ofbiz.webapp.event.EventFactory; > import org.apache.ofbiz.webapp.event.EventHandler; > import org.apache.ofbiz.webapp.event.EventHandlerException; > @@ -110,6 +116,29 @@ public class RequestHandler { > return null; > } > > + /** > + * Find a request map in {@code reqMaps} matching {@code req}. > + * Otherwise fall back to the one matching {@code defaultReq}. > + * > + * @param reqMaps The dictionary associating URI to request maps > + * @param req The HTTP request to match > + * @param defaultReq the default request which serves as a fallback. > + * @return an request map {@code Optional} > + */ > + static Optional<RequestMap> resolveURI(MultivaluedMap<String, > RequestMap> reqMaps, > + HttpServletRequest req, String defaultReq) { > + String path = getRequestUri(req.getPathInfo()); > + List<RequestMap> rmaps = reqMaps.get(path); > + if (rmaps == null && defaultReq != null) { > + rmaps = reqMaps.get(defaultReq); > + } > + List<RequestMap> frmaps = (rmaps != null) ? rmaps : > Collections.emptyList(); > + return Stream.of(req.getMethod(), "all", "") > + .map(verb -> (Predicate<String>) verb::equalsIgnoreCase) > + .flatMap(p -> frmaps.stream().filter(m -> p.test(m.method))) > + .findFirst(); > + } > + > public void doRequest(HttpServletRequest request, HttpServletResponse > response, String requestUri) throws RequestHandlerException, > RequestHandlerExceptionAllowExternalRequests { > HttpSession session = request.getSession(); > Delegator delegator = (Delegator) request.getAttribute("delegator"); > @@ -127,10 +156,10 @@ public class RequestHandler { > > // get the controllerConfig once for this method so we don't have to > get it over and over inside the method > ConfigXMLReader.ControllerConfig controllerConfig = > this.getControllerConfig(); > - Map<String, ConfigXMLReader.RequestMap> requestMapMap = null; > + MultivaluedMap<String, ConfigXMLReader.RequestMap> requestMapMap = > null; > String statusCodeString = null; > try { > - requestMapMap = controllerConfig.getRequestMapMap(); > + requestMapMap = controllerConfig.getRequestMapMultiMap(); > statusCodeString = controllerConfig.getStatusCode(); > } catch (WebAppConfigurationException e) { > Debug.logError(e, "Exception thrown while parsing controller.xml > file: ", module); > @@ -143,6 +172,15 @@ public class RequestHandler { > // workaround if we are in the root webapp > String cname = UtilHttp.getApplicationName(request); > > + // Set the fallback default request. > + String defaultRequest = null; > + try { > + defaultRequest = controllerConfig.getDefaultRequest(); > + } catch (WebAppConfigurationException e) { > + Debug.logError(e, "Exception thrown while parsing controller.xml > file: ", module); > + throw new RequestHandlerException(e); > + } > + > // Grab data from request object to process > String defaultRequestUri = > RequestHandler.getRequestUri(request.getPathInfo()); > if (request.getAttribute("targetRequestUri") == null) { > @@ -156,34 +194,16 @@ public class RequestHandler { > String overrideViewUri = > RequestHandler.getOverrideViewUri(request.getPathInfo()); > > String requestMissingErrorMessage = "Unknown request [" + > defaultRequestUri + "]; this request does not exist or cannot be called > directly."; > - ConfigXMLReader.RequestMap requestMap = null; > - if (defaultRequestUri != null) { > - requestMap = requestMapMap.get(defaultRequestUri); > - } > - // check for default request > - if (requestMap == null) { > - String defaultRequest; > - try { > - defaultRequest = controllerConfig.getDefaultRequest(); > - } catch (WebAppConfigurationException e) { > - Debug.logError(e, "Exception thrown while parsing > controller.xml file: ", module); > - throw new RequestHandlerException(e); > - } > - if (defaultRequest != null) { // required! to avoid a null > pointer exception and generate a requesthandler exception if default request > not found. > - requestMap = requestMapMap.get(defaultRequest); > - } > - } > + RequestMap requestMap = > + resolveURI(requestMapMap, request, > defaultRequest).orElse(null); > > // check for override view > if (overrideViewUri != null) { > - ConfigXMLReader.ViewMap viewMap; > try { > - viewMap = > getControllerConfig().getViewMapMap().get(overrideViewUri); > - if (viewMap == null) { > - String defaultRequest = > controllerConfig.getDefaultRequest(); > - if (defaultRequest != null) { // required! to avoid a > null pointer exception and generate a requesthandler exception if default > request not found. > - requestMap = requestMapMap.get(defaultRequest); > - } > + ConfigXMLReader.ViewMap viewMap = > + > controllerConfig.getViewMapMap().get(overrideViewUri); > + if (viewMap == null && defaultRequest != null) { > + requestMap = requestMapMap.getFirst(defaultRequest); > } > } catch (WebAppConfigurationException e) { > Debug.logError(e, "Exception thrown while parsing > controller.xml file: ", module); > @@ -196,7 +216,7 @@ public class RequestHandler { > if (requestMap == null) { > if (throwRequestHandlerExceptionOnMissingLocalRequest) throw new > RequestHandlerException(requestMissingErrorMessage); > else throw new RequestHandlerExceptionAllowExternalRequests(); > - } > + } > > String eventReturn = null; > if (requestMap.metrics != null && requestMap.metrics.getThreshold() > != 0.0 && requestMap.metrics.getTotalEvents() > 3 && > requestMap.metrics.getThreshold() < requestMap.metrics.getServiceRate()) { > @@ -204,13 +224,11 @@ public class RequestHandler { > } > ConfigXMLReader.RequestMap originalRequestMap = requestMap; // Save > this so we can update the correct performance metrics. > > - > boolean interruptRequest = false; > - > // Check for chained request. > if (chain != null) { > String chainRequestUri = RequestHandler.getRequestUri(chain); > - requestMap = requestMapMap.get(chainRequestUri); > + requestMap = requestMapMap.getFirst(chainRequestUri); > if (requestMap == null) { > throw new RequestHandlerException("Unknown chained request > [" + chainRequestUri + "]; this request does not exist"); > } > @@ -234,18 +252,11 @@ public class RequestHandler { > // Check to make sure we are allowed to access this request > directly. (Also checks if this request is defined.) > // If the request cannot be called, or is not defined, check and > see if there is a default-request we can process > if (!requestMap.securityDirectRequest) { > - String defaultRequest; > - try { > - defaultRequest = controllerConfig.getDefaultRequest(); > - } catch (WebAppConfigurationException e) { > - Debug.logError(e, "Exception thrown while parsing > controller.xml file: ", module); > - throw new RequestHandlerException(e); > - } > - if (defaultRequest == null || > !requestMapMap.get(defaultRequest).securityDirectRequest) { > + if (defaultRequest == null || > !requestMapMap.getFirst(defaultRequest).securityDirectRequest) { > // use the same message as if it was missing for > security reasons, ie so can't tell if it is missing or direct request is not > allowed > throw new > RequestHandlerException(requestMissingErrorMessage); > } else { > - requestMap = requestMapMap.get(defaultRequest); > + requestMap = requestMapMap.getFirst(defaultRequest); > } > } > // Check if we SHOULD be secure and are not. > @@ -409,7 +420,8 @@ public class RequestHandler { > // Invoke the security handler > // catch exceptions and throw RequestHandlerException if failed. > if (Debug.verboseOn()) Debug.logVerbose("[RequestHandler]: > AuthRequired. Running security check. " + showSessionId(request), module); > - ConfigXMLReader.Event checkLoginEvent = > requestMapMap.get("checkLogin").event; > + ConfigXMLReader.Event checkLoginEvent = > + requestMapMap.getFirst("checkLogin").event; > String checkLoginReturnString = null; > > try { > @@ -422,9 +434,9 @@ public class RequestHandler { > eventReturn = checkLoginReturnString; > // if the request is an ajax request we don't want to return > the default login check > if > (!"XMLHttpRequest".equals(request.getHeader("X-Requested-With"))) { > - requestMap = requestMapMap.get("checkLogin"); > + requestMap = requestMapMap.getFirst("checkLogin"); > } else { > - requestMap = requestMapMap.get("ajaxCheckLogin"); > + requestMap = requestMapMap.getFirst("ajaxCheckLogin"); > } > } > } > > Added: > ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java?rev=1834389&view=auto > ============================================================================== > --- > ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java > (added) > +++ > ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java > Tue Jun 26 02:00:33 2018 > @@ -0,0 +1,156 @@ > +/******************************************************************************* > + * Licensed to the Apache Software Foundation (ASF) under one > + * or more contributor license agreements. See the NOTICE file > + * distributed with this work for additional information > + * regarding copyright ownership. The ASF licenses this file > + * to you under the Apache License, Version 2.0 (the > + * "License"); you may not use this file except in compliance > + * with the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, > + * software distributed under the License is distributed on an > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > + * KIND, either express or implied. See the License for the > + * specific language governing permissions and limitations > + * under the License. > + > *******************************************************************************/ > +package org.apache.ofbiz.webapp.control; > + > +import static org.mockito.Mockito.mock; > +import static org.mockito.Mockito.when; > +import static org.junit.Assert.assertFalse; > +import static org.junit.Assert.assertSame; > +import static org.junit.Assert.assertTrue; > + > +import javax.servlet.http.HttpServletRequest; > +import javax.ws.rs.core.MultivaluedHashMap; > +import javax.ws.rs.core.MultivaluedMap; > + > +import org.apache.ofbiz.webapp.control.ConfigXMLReader.RequestMap; > +import org.junit.Before; > +import org.junit.Test; > +import org.w3c.dom.Element; > + > +public class RequestHandlerTests { > + public static class ResolveURITests { > + private MultivaluedMap<String,RequestMap> reqMaps; > + private HttpServletRequest req; > + private Element dummyElement; > + > + @Before > + public void setUp() { > + reqMaps = new MultivaluedHashMap<>(); > + req = mock(HttpServletRequest.class); > + dummyElement = mock(Element.class); > + when(dummyElement.getAttribute("method")).thenReturn("all"); > + when(req.getMethod()).thenReturn("get"); > + } > + > + @Test > + public void resolveURIBasic() throws RequestHandlerException { > + RequestMap foo = new RequestMap(dummyElement); > + RequestMap bar = new RequestMap(dummyElement); > + reqMaps.putSingle("foo", foo); > + reqMaps.putSingle("bar", bar); > + when(req.getPathInfo()).thenReturn("/foo"); > + assertSame(foo, RequestHandler.resolveURI(reqMaps, req, > null).get()); > + } > + > + @Test > + public void resolveURIBasicPut() throws RequestHandlerException { > + when(dummyElement.getAttribute("method")).thenReturn("put"); > + when(req.getPathInfo()).thenReturn("/foo"); > + when(req.getMethod()).thenReturn("put"); > + > + RequestMap foo = new RequestMap(dummyElement); > + > + assertFalse(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + reqMaps.putSingle("foo", foo); > + assertTrue(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + } > + > + @Test > + public void resolveURIUpperCase() throws RequestHandlerException { > + when(dummyElement.getAttribute("method")).thenReturn("get"); > + RequestMap foo = new RequestMap(dummyElement); > + when(dummyElement.getAttribute("method")).thenReturn("put"); > + RequestMap bar = new RequestMap(dummyElement); > + reqMaps.putSingle("foo", foo); > + reqMaps.putSingle("bar", bar); > + > + when(req.getPathInfo()).thenReturn("/foo"); > + when(req.getMethod()).thenReturn("GET"); > + assertSame(foo, RequestHandler.resolveURI(reqMaps, req, > null).get()); > + > + when(req.getPathInfo()).thenReturn("/bar"); > + when(req.getMethod()).thenReturn("PUT"); > + assertSame(bar, RequestHandler.resolveURI(reqMaps, req, > null).get()); > + } > + > + @Test > + public void resolveURICatchAll() throws RequestHandlerException { > + when(req.getPathInfo()).thenReturn("/foo"); > + RequestMap foo = new RequestMap(dummyElement); > + when(req.getMethod()).thenReturn("get"); > + assertFalse(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + when(req.getMethod()).thenReturn("post"); > + assertFalse(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + when(req.getMethod()).thenReturn("put"); > + assertFalse(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + when(req.getMethod()).thenReturn("delete"); > + assertFalse(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + > + reqMaps.putSingle("foo", foo); > + when(req.getMethod()).thenReturn("get"); > + assertTrue(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + when(req.getMethod()).thenReturn("post"); > + assertTrue(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + when(req.getMethod()).thenReturn("put"); > + assertTrue(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + when(req.getMethod()).thenReturn("delete"); > + assertTrue(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + } > + > + @Test > + public void resolveURISegregate() throws RequestHandlerException { > + when(dummyElement.getAttribute("method")).thenReturn("put"); > + RequestMap fooPut = new RequestMap(dummyElement); > + when(dummyElement.getAttribute("method")).thenReturn("all"); > + RequestMap fooAll = new RequestMap(dummyElement); > + reqMaps.putSingle("foo", fooAll); > + reqMaps.add("foo", fooPut); > + > + when(req.getPathInfo()).thenReturn("/foo"); > + when(req.getMethod()).thenReturn("put"); > + assertSame(fooPut, RequestHandler.resolveURI(reqMaps, req, > null).get()); > + when(req.getMethod()).thenReturn("get"); > + assertSame(fooAll, RequestHandler.resolveURI(reqMaps, req, > null).get()); > + } > + > + @Test > + public void resolveURIDefault() throws Exception { > + RequestMap foo = new RequestMap(dummyElement); > + RequestMap bar = new RequestMap(dummyElement); > + reqMaps.putSingle("foo", foo); > + reqMaps.putSingle("bar", bar); > + > + when(req.getPathInfo()).thenReturn("/bar"); > + when(req.getMethod()).thenReturn("get"); > + assertSame(bar, RequestHandler.resolveURI(reqMaps, req, > "bar").get()); > + } > + > + @Test > + public void resolveURINoDefault() throws Exception { > + RequestMap foo = new RequestMap(dummyElement); > + RequestMap bar = new RequestMap(dummyElement); > + reqMaps.putSingle("foo", foo); > + reqMaps.putSingle("bar", bar); > + > + when(req.getPathInfo()).thenReturn("/baz"); > + when(req.getMethod()).thenReturn("get"); > + assertFalse(RequestHandler.resolveURI(reqMaps, req, > null).isPresent()); > + } > + } > +} > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java > ------------------------------------------------------------------------------ > svn:eol-style = native > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java > ------------------------------------------------------------------------------ > svn:keywords = Date Rev Author URL Id > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java > ------------------------------------------------------------------------------ > svn:mime-type = text/plain > >
