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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >