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 |
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 <[hidden email]> 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 > > |
Why did you change the method to GET?
On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar <[hidden email]> 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 < > [hidden email]> > 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 > > > > > |
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 <[hidden email]> wrote: > Why did you change the method to GET? > > On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar <[hidden email] > > > 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 < > > [hidden email]> > > 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 > > > > > > > > > |
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 <[hidden email]> 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 < > [hidden email]> > wrote: > > > Why did you change the method to GET? > > > > On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < > [hidden email] > > > > > 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 < > > > [hidden email]> > > > 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 > > > > > > > > > > > > > > |
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 <[hidden email]> 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 < > [hidden email]> > 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 < > > [hidden email]> > > wrote: > > > > > Why did you change the method to GET? > > > > > > On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < > > [hidden email] > > > > > > > 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 < > > > > [hidden email]> > > > > 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 > > > > > > > > > > > > > > > > > > > > |
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 <[hidden email]> 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 <[hidden email]> > 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 < > > [hidden email]> > > 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 < > > > [hidden email]> > > > wrote: > > > > > > > Why did you change the method to GET? > > > > > > > > On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < > > > [hidden email] > > > > > > > > > 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 < > > > > > [hidden email]> > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > |
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 <[hidden email]> 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 > <[hidden email]> 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 < > [hidden email]> > > 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 < > > > [hidden email]> > > > 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 < > > > > [hidden email]> > > > > wrote: > > > > > > > > > Why did you change the method to GET? > > > > > > > > > > On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < > > > > [hidden email] > > > > > > > > > > > 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 < > > > > > > [hidden email]> > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
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 < [hidden email]> 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 < > [hidden email]> > 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 > > <[hidden email]> 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 < > > [hidden email]> > > > 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 < > > > > [hidden email]> > > > > 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 < > > > > > [hidden email]> > > > > > wrote: > > > > > > > > > > > Why did you change the method to GET? > > > > > > > > > > > > On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < > > > > > [hidden email] > > > > > > > > > > > > > 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 < > > > > > > > [hidden email]> > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
Taher Alkhateeb <[hidden email]> writes:
> 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. The issue seems indeed related. +1 for a opening a ticket. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37 |
Administrator
|
Le 26/08/2018 à 11:20, Mathieu Lirzin a écrit :
> Taher Alkhateeb <[hidden email]> writes: > >> 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. > The issue seems indeed related. > > +1 for a opening a ticket. > +1 Jacques |
Administrator
|
In reply to this post by grv
Thanks Girish for your detailed analysis!
Jacques Le 26/08/2018 à 06:57, Girish Vasmatkar a écrit : > 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 <[hidden email]> > 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 >> <[hidden email]> 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 < >> [hidden email]> >>> 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 < >>>> [hidden email]> >>>> 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 < >>>>> [hidden email]> >>>>> wrote: >>>>> >>>>>> Why did you change the method to GET? >>>>>> >>>>>> On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < >>>>> [hidden email] >>>>>> 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 < >>>>>>> [hidden email]> >>>>>>> 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 >>>>>>>> >>>>>>>> |
In reply to this post by taher
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 < > [hidden email]> 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 < >> [hidden email]> >> 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 >>> <[hidden email]> 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 < >>> [hidden email]> >>>> 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 < >>>>> [hidden email]> >>>>> 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 < >>>>>> [hidden email]> >>>>>> wrote: >>>>>> >>>>>>> Why did you change the method to GET? >>>>>>> >>>>>>> On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < >>>>>> [hidden email] >>>>>>> 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 < >>>>>>>> [hidden email]> >>>>>>>> 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 >>>>>>>>> >>>>>>>>> smime.p7s (5K) Download Attachment |
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 <[hidden email]> 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 < > > [hidden email]> 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 < > >> [hidden email]> > >> 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 > >>> <[hidden email]> 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 < > >>> [hidden email]> > >>>> 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 < > >>>>> [hidden email]> > >>>>> 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 < > >>>>>> [hidden email]> > >>>>>> wrote: > >>>>>> > >>>>>>> Why did you change the method to GET? > >>>>>>> > >>>>>>> On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < > >>>>>> [hidden email] > >>>>>>> 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 < > >>>>>>>> [hidden email]> > >>>>>>>> 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 > >>>>>>>>> > >>>>>>>>> > > > |
Hello team,
Thanks, everyone for your inputs. I have attached the patch on the ticket OFBIZ-10539 <https://issues.apache.org/jira/browse/OFBIZ-10539> to fix the issue. Kindly have a look into this and let me know if there are any additional inputs. On Mon, Aug 27, 2018 at 11:20 AM Ritesh Kumar < [hidden email]> wrote: > 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 <[hidden email]> > 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 < >> > [hidden email]> 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 < >> >> [hidden email]> >> >> 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 >> >>> <[hidden email]> 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 < >> >>> [hidden email]> >> >>>> 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 < >> >>>>> [hidden email]> >> >>>>> 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 < >> >>>>>> [hidden email]> >> >>>>>> wrote: >> >>>>>> >> >>>>>>> Why did you change the method to GET? >> >>>>>>> >> >>>>>>> On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar < >> >>>>>> [hidden email] >> >>>>>>> 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 < >> >>>>>>>> [hidden email]> >> >>>>>>>> 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 >> >>>>>>>>> >> >>>>>>>>> >> >> >> |
Free forum by Nabble | Edit this page |