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, <[hidden email]> 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 > > |
On 26 June 2018 at 17:58, Taher Alkhateeb <[hidden email]>
wrote: > 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. > Hi all, I haven't found the specific issue, but wasn't there a major change several years ago from GET to POST to help guard against XSS attacks? Cheers Paul Foxworthy -- Coherent Software Australia Pty Ltd PO Box 2773 Cheltenham Vic 3192 Australia Phone: +61 3 9585 6788 Web: http://www.coherentsoftware.com.au/ Email: [hidden email]
--
Coherent Software Australia Pty Ltd http://www.coherentsoftware.com.au/ Bonsai ERP, the all-inclusive ERP system http://www.bonsaierp.com.au/ |
Administrator
|
Le 26/06/2018 à 13:31, Paul Foxworthy a écrit :
> On 26 June 2018 at 17:58, Taher Alkhateeb <[hidden email]> > wrote: > >> 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. >> > Hi all, > > I haven't found the specific issue, but wasn't there a major change several > years ago from GET to POST to help guard against XSS attacks? > > Cheers > > Paul Foxworthy > Actually Mathieu is notably working on REST in OFBiz. In REST GET, as others methods, are used. Normally, as its name "GET" shows, only for retrieval. During retrieval an XSS attack should not be feared if, as pointed out at https://www.owasp.org/index.php/REST_Security_Cheat_Sheet#Sensitive_information_in_HTTP_requests <<In GET requests sensitive data should be transferred in an HTTP Header>> Jacques |
Free forum by Nabble | Edit this page |