Thanks, Girish for clearly explaining my concern and everyone for your inputs.
I have created the ticket ( OFBIZ-10539 <https://issues.apache.org/jira/browse/OFBIZ-10539>). Soon, I will be providing the solution patch for review. On Sun, Aug 26, 2018 at 3:36 PM Michael Brohl <michael.br...@ecomify.de> wrote: > OFBiz handles multiple request parameters with the same name well if you > use POST requests. > > For GET requests, there seem to be no standard for multiple parameters > with the same name, see [1]. I thinks it would be good to investigate > further and see if we can fix this. > > I don't see a problem with REST here, the data will most probably > transferred in some kind of JSON structure. > > [1] > > https://stackoverflow.com/questions/24059773/correct-way-to-pass-multiple-values-for-same-parameter-name-in-get-request#24728298 > > Regards, > > Michael > > > Am 26.08.18 um 07:09 schrieb Taher Alkhateeb: > > Hmmm, based on Girish's feedback I think perhaps the next step should be > to > > open a JIRA and further investigate. Thinking about it some more, maybe > > this would have an impact on the REST API project we're working on. > > > > On Sun, Aug 26, 2018, 7:57 AM Girish Vasmatkar < > > girish.vasmat...@hotwaxsystems.com> wrote: > > > >> Hi Ritesh > >> > >> It does look like an issue to me. > >> > >> I believe (correct me if I am wrong) it is not so much about whether > GET is > >> appropriate here, it is more about that the framework is unable to > handle > >> multiple request parameters with same name, which is a common case when > we > >> talk about multiple check boxes on a form representing a single entity. > The > >> fact that GET is doing it's job correctly when the user is logged in > (means > >> when you change the method from POST to GET and the user is already > logged > >> in) and not so much when the user is logged out and the request is made > via > >> bookmark, shows that the code is not working properly. > >> > >> Then, there is also an issue with URL encoding and decoding that becomes > >> apparent with executing your scenario. Whether it is correct to change > the > >> form method is arguable, but the code should be able to handle it. If a > >> form were to be designed with GET method and the only elements present > on > >> the form are two check boxes and a text field and if you were to select > >> both check boxes and have value with spaces in the text box, this > scenario > >> would still fail. > >> > >> I think the simplest approach would have been to just store query string > >> (request.getQueryString()) in the session attribute instead of Map (but > I > >> think it was well pondered upon to use Map) and then just redirecting to > >> the saved URL once the user loges in. I did it on my local workstation > and > >> it just worked perfectly. May be one reason why a map was used instead > of > >> storing query string was to handle unencoded requests coming from > browsers > >> such as IE. I may be wrong so please correct me if anyone has an idea > >> around this piece of code as to why Map was used instead of storing the > >> query string in session to be used later on. > >> > >> If you can do something to fix the existing code, that would be better > >> approach, IMO. > >> > >> May be it is not a major issue but certainly worthy of having a > dedicated > >> JIRA for. Everybody, please chime in and provide your thoughts. > >> > >> Thanks and Best regards, > >> Girish Vasmatkar > >> HotWax Systems > >> > >> On Sat, Aug 25, 2018 at 8:03 PM Taher Alkhateeb < > >> slidingfilame...@gmail.com> > >> wrote: > >> > >>> Okay, I understand this issue. I don't think it is possible to > >>> abstract away a complex search screen with http GET method for > >>> bookmarks. The performFind service is quite complex and it is > >>> difficult to replicate the requirements using GET. GET is not designed > >>> to handle multiple languages, spaces, and other peculiarities that are > >>> needed for such a screen to work. > >>> > >>> There are multiple solutions that I can see here. One of them is > >>> simply to create a new entity, let's call it SearchFilter, that saves > >>> search parameters, which can be applied later on. Either way, you need > >>> to customize, your problem is not OFBiz, your problem is http GET > >>> limitations. > >>> On Fri, Aug 24, 2018 at 12:35 PM Ritesh Kumar > >>> <ritesh.ku...@hotwaxsystems.com> wrote: > >>>> Using the POST method does not append form data to the URL, i.e, the > >>>> parameters will not be visible in the URL. > >>>> For example, take a Find Screen (say, FindWorkEffort) which send data > >>>> through a form with POST method. Apply some filters (say, status). No > >>>> applied filters appear in the URL. Bookmark this page. Next time when > >> I > >>>> open this bookmark, those applied filters will not be there as this > >> page > >>> is > >>>> being rendered using data from the URL and since the applied filters > >> were > >>>> not there in the bookmarked URL, this page is rendered without the > >>> applied > >>>> filters. That is why I used GET form method so that I am able to get > >> the > >>>> page with applied filters when I open a bookmarked page. > >>>> > >>>> The bug here is (supposing the GET method is used) > >>>> 1. On opening the bookmark, the page is rendered with double encoding > >> (if > >>>> the value had a space character initially, the space character was > >>> already > >>>> encoded into '+' in the URL and when this bookmark is opened, this '+' > >> is > >>>> again encoded). > >>>> 2. Suppose the bookmarked URL had multiple values from the same filter > >>>> (say, Cancelled and Declined status), it renders with just one of the > >>>> statutes applied. It is because the request handler prepares a Map of > >>>> parameters from the query string and as is the property of Map to > >> replace > >>>> the old value if a new value is being added with the same key (in this > >>>> example, first Cancelled status is put in this Map and then Declined), > >>> only > >>>> Declined status is put in this Map. > >>>> > >>>> Hope, this clears the confusion. I will be happy to provide more > >>>> information if needed. > >>>> > >>>> On Fri, Aug 24, 2018 at 1:46 PM Taher Alkhateeb < > >>> slidingfilame...@gmail.com> > >>>> wrote: > >>>> > >>>>> Not enough information. What happens exactly? What is the bug? What > >> do > >>> you > >>>>> mean by it does not let us do that? > >>>>> > >>>>> On Fri, Aug 24, 2018, 11:09 AM Ritesh Kumar < > >>>>> ritesh.ku...@hotwaxsystems.com> > >>>>> wrote: > >>>>> > >>>>>> Hello Taher, > >>>>>> > >>>>>> Changing form method to GET is just to make the query parameters > >>> visible > >>>>> in > >>>>>> the URL so that a user is able to bookmark or share it. Using the > >>> POST > >>>>>> method does not let us do that. > >>>>>> > >>>>>> On Fri, Aug 24, 2018 at 11:54 AM Taher Alkhateeb < > >>>>>> slidingfilame...@gmail.com> > >>>>>> wrote: > >>>>>> > >>>>>>> Why did you change the method to GET? > >>>>>>> > >>>>>>> On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < > >>>>>> ritesh.ku...@hotwaxsystems.com > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Just to put my point more clearly, let me add the steps to > >>> generate > >>>>> the > >>>>>>>> above-mentioned case. Please refer demo-trunk > >>>>>>>> <https://demo-trunk.ofbiz.apache.org/webtools/control/main>. > >>>>>>>> > >>>>>>>> 1. Open this link, FindWorkEffort > >>>>>>>> < > >> https://demo-trunk.ofbiz.apache.org/workeffort/control/FindWorkEffort > >>>>>>> . > >>>>>>>> Find Work Effort screen will be rendered. > >>>>>>>> 2. Inspect and change the form method to "GET". > >>>>>>>> 3. Apply any of the two statuses (say, Cancelled and Declined). > >>> Click > >>>>>> on > >>>>>>>> Find. > >>>>>>>> 4. Records will be fetched according to the applied filters. > >>>>>>>> 5. Check the URL. Cancelled and Declined statuses must be there > >>> in > >>>>> the > >>>>>>> URL. > >>>>>>>> 6. Bookmark this page and log out. > >>>>>>>> 7. Now, open the bookmark. > >>>>>>>> 8. The login page will be rendered. Check the URL here. It will > >>> be > >>>>> the > >>>>>>> same > >>>>>>>> as it was when the page was being bookmarked. > >>>>>>>> 9. Type in the credentials and log in. > >>>>>>>> 10. The result may be different. Check the URL. One of the > >>> statuses > >>>>> is > >>>>>>>> gone. > >>>>>>>> > >>>>>>>> Due to business requirement, I need to show query parameters in > >>> the > >>>>> URL > >>>>>>> so > >>>>>>>> that the user is able to bookmark the page. And, we normally > >>> pass Id > >>>>> in > >>>>>>> the > >>>>>>>> parameters, but, due to some reason, I may have to pass values > >>> with > >>>>>> space > >>>>>>>> characters. > >>>>>>>> > >>>>>>>> I hope, this demo puts forth my concern. > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On Thu, Aug 23, 2018 at 6:27 PM Ritesh Kumar < > >>>>>>>> ritesh.ku...@hotwaxsystems.com> > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>>> Hello All, > >>>>>>>>> > >>>>>>>>> I faced an issue while trying to open a bookmarked page with > >>> OFBiz. > >>>>>>>>> Suppose, the URL of this bookmarked page contains a parameter > >>> with > >>>>>>>>> multiple values and the value may have space character. The > >>> query > >>>>>>> string > >>>>>>>> in > >>>>>>>>> the URL looks somewhat like this > >>>>>>>>> > >>>>>>>>> > >> > "?categoryHierarchy=3%2FCompany+Catalog%2FBrowse+Root%2FCloths%2FMen%2F"&statusId=approved&statusId=created". > >>>>>>>>> The "%2F" and "+" are encoded value of "/", a separator and > >>> space > >>>>>>>>> character respectively. The status id parameter appears twice > >>> and > >>>>> the > >>>>>>>>> category hierarchy value has space character. > >>>>>>>>> > >>>>>>>>> The user is logged out at this instance and this bookmarked > >>> page is > >>>>>>>>> opened. Since the user is not logged in, the login page is > >>>>> rendered. > >>>>>> I > >>>>>>>> feed > >>>>>>>>> in the credentials and the intended URL is hit. Here, I do > >> not > >>> get > >>>>>> the > >>>>>>>>> required result. > >>>>>>>>> > >>>>>>>>> When I check the URL, the parameter with multiple values just > >>> has > >>>>> the > >>>>>>>> last > >>>>>>>>> value of the list and "+" is encoded into "%2B". The URL now > >> is > >>>>>>>>> > >> > "?categoryHierarchy=3%2FCompany%2BCatalog%2FBrowse%2BRoot%2FCloths%2FMen%2F"&statusId==created." > >>>>>>>>> I did some digging and found out that > >> LoginWorker.checkLogin() > >>>>> comes > >>>>>>> into > >>>>>>>>> action and what it does is that it creates a new session > >> object > >>>>>>> (because > >>>>>>>>> the previous session becomes invalid) and in the session > >>> object, it > >>>>>>> puts > >>>>>>>>> the previous URL parameters. This previous URL parameters are > >>>>> fetched > >>>>>>>> using > >>>>>>>>> UtilHttp.getUrlOnlyParameterMap(request) which internally > >> calls > >>>>>>>>> getQueryStringOnlyParameterMap(). This method returns a map > >> by > >>>>>> breaking > >>>>>>>> the > >>>>>>>>> query string into key and value pair. A map can not have > >>> duplicate > >>>>>> keys > >>>>>>>> (in > >>>>>>>>> this case removes the approved status) and the value is not > >>> decoded > >>>>>>>> before > >>>>>>>>> putting it into the map ('+' is not decoded). This map is > >> then > >>> used > >>>>>> to > >>>>>>>>> create an encoded ('+' is encoded into '%2B' ) redirect > >> target > >>> and > >>>>>> then > >>>>>>>>> callRedirect() is called on this new redirect target, ending > >> up > >>>>> with > >>>>>>>>> unintended URL (inside RequestHandler.doRequest()). > >>>>>>>>> > >>>>>>>>> I could resolve this issue by decoding the already encoded > >>> value > >>>>>> before > >>>>>>>>> putting it into the Map and if the key is already present in > >>> the > >>>>> Map, > >>>>>>> it > >>>>>>>>> must create a list of the values. > >>>>>>>>> > >>>>>>>>> Am I missing something or is this really a bug and needs to > >> be > >>>>>>> addressed > >>>>>>>>> OOTB? > >>>>>>>>> If this is a bug, is proposed solution the right one? > >>>>>>>>> > >>>>>>>>> -- > >>>>>>>>> Best, > >>>>>>>>> Ritesh Kumar > >>>>>>>>> > >>>>>>>>> > > >
