10188
Reporter: kbraak
Assignee: jcuadra
Type: Bug
Summary: Negative paging offset causes PSQLException: ERROR
Priority: Major
Resolution: Fixed
Status: Closed
Created: 2011-11-11 12:02:10.023
Updated: 2013-08-29 14:44:56.009
Resolved: 2012-11-05 14:46:03.21
Description: http://localhost:8080/species/103350120/typespecimens = 8 results
http://localhost:8080/species/103350120/typespecimens?offset=3 = 5 results with previous button showing on page
the paging macro loads the previous button with URL = http://localhost:8080/species/103350120/typespecimens?offset=-22&limit=25
This causes a PSQLException: ERROR: OFFSET must not be negative exception being thrown.
Both the macro and the mybatis layer should be careful handling negative offsets and avoid the exception stacktrace being thrown. ]]>
Author: kbraak@gbif.org
Created: 2011-11-11 12:06:53.592
Updated: 2011-11-11 12:07:12.604
Another case, this time on the dataset search page
Full stacktrace included in attachment.
Author: jcuadra@gbif.org
Created: 2012-10-18 16:30:33.059
Updated: 2012-10-18 16:30:33.059
The problem is gone already, by a previous commit presumably. Sorry, don't have the commit # that fixed it.
But I see from here -> http://code.google.com/p/gbif-portal/source/detail?spec=svn258&r=258 that the "limit" parameter was removed as a possible query parameter. No idea why this decision was made.
But, if you try
http://staging.gbif.org:8080/portal-web-dynamic/species/103350120/typespecimens?offset=3
youll see that the PREVIOUS PAGE button already has correct values.
Author: mdoering@gbif.org
Created: 2012-10-18 18:43:21.821
Updated: 2012-10-18 18:43:21.821
The problem doesnt appear on the portal immediately anymore, but the real problem still exists and you get errors with negative offsets. That shouldnt be, but it should probably default to zero offset in those cases.
see:
http://staging.gbif.org:8080/portal-web-dynamic/species/216/vernaculars?offset=-10
Author: mdoering@gbif.org
Created: 2012-10-18 18:45:41.814
Updated: 2012-10-18 18:45:41.814
At least the registry webservices default to zero:
http://staging.gbif.org:8080/registry-ws/dataset?limit=2&offset=-12
http://staging.gbif.org:8080/checklistbank-ws/name_usage?limit=2&offset=-10
so it must be a portal or client issue
Author: jcuadra@gbif.org
Created: 2012-10-19 09:46:27.955
Updated: 2012-10-19 09:46:27.955
There was a discussion about this on the dev-chat, sorry for not cross posting here, but here it is:
===========================================
[10/18/12 3:47:58 PM] Jose Cuadra: Have a question for you guys, to see which is the best option - maybe not the best place to discuss it but Ill present the case
When on the portal, somebody specifies a negative offset and/or a negative limit on the URL, we get an error page triggered because on the PageableBase (http://tinyurl.com/pageable) we throw an IllegalArgumentException if any of these two values are negative, that later in the chain gets converted to a 404 at the webapp level.
Error --> http://staging.gbif.org:8080/portal-web-dynamic/species/103350120/typespecimens?offset=-22&limit=25
My question (and options) are:
1. Instead of an IllegalArgumentExc, reset the problematic value(s) to 0, if negative values are provided. - this would mean obviously an API change on the PageableBase's setter methods - which in turn gets rid of the problem from the root
2. if not, then manage this at the struts Action level - there are 2 or 3 Action classes that share this behaviour and might throw this same error so I would need to add code to check this condition to each one
3. modify somewhere the code (Im assumming the interceptors) to display an error page, but not a generic one like we have right now. Maybe an error page which states that offset&limit should be non-negative.
(btw, google results, when you set their "start" flag (analogous to our "offset" parameter) to negative, it resets it to 0)
------
https://www.google.dk/#q=example&start=-10
same as
https://www.google.dk/#q=example&start=0
[10/18/12 3:49:29 PM] GBIF - Tim: http://staging.gbif.org:8080/portal-web-dynamic/species/103350120/typespecimens?offset=-22&limit=25
Is that really the wrong thing to return if someone provides a nonesense url?
[10/18/12 3:50:14 PM] Jose Cuadra: the user is not presented with something useful to know what he/she did wrong
[10/18/12 3:50:48 PM] GBIF - Tim: To people who are URL hacking, I am not sure that is an issue though
[10/18/12 3:51:33 PM] Jose Cuadra: so can I assume this error is ok to display ? its an open jira
[10/18/12 3:52:12 PM] GBIF - Tim: personally I would say yes
[10/18/12 3:54:05 PM] Jose Cuadra: ok. I would agree as well, to get this problem it needs url hacking
[10/18/12 3:55:02 PM] GBIF - Lars Francke: I agree too
Author: jcuadra@gbif.org
Comment: Markus, if you think it is still a problem, we would then need to discuss it amongst the group.
Created: 2012-10-19 09:47:34.673
Updated: 2012-10-19 09:47:34.673
Author: mdoering@gbif.org
Created: 2012-10-19 09:55:53.189
Updated: 2012-10-19 09:55:53.189
I dont think its a problem, but I do think its not nice and an easy fix too.
The webservices also behave differently.
Id prefer either of your suggestions:
- reset to zero like we do in WS
- catch the IllegalArgumentException and present a generic error page for a "bad request" (we might have various sources for this exception)
Author: jcuadra@gbif.org
Created: 2012-11-05 14:46:03.242
Updated: 2012-11-05 14:46:03.242
Solved with enhancements on
http://code.google.com/p/gbif-portal/source/detail?r=1135
