Horreum
Horreum copied to clipboard
Method `TestService.getByNameOrId` might return wrong result
Describe the bug
The endpoint /api/test/byName/{name}
might return wrong result if there exist a Test
having as name the id
of another test (which seems possible).
Taking a look at the implemention:
public Test getByNameOrId(String input){
TestDAO test = null;
if (input.matches("-?\\d+")) {
int id = Integer.parseInt(input);
// there could be some issue if name is numeric and corresponds to another test id
test = TestDAO.find("name = ?1 or id = ?2", input, id).firstResult();
}
if (test == null) {
test = TestDAO.find("name", input).firstResult();
}
if (test == null) {
throw ServiceException.notFound("No test with name or id " + input);
}
return TestMapper.from(test);
}
I see a couple of issues:
- If
name
matches theid
of another test, the query will give two results and the code will simply get the first one, which might be wrong! - I think the second if stmt is useless as the query that is executed is exactly the left side of the previous
or
statement, so if it was null before it will be also here.
To Reproduce
- Create a test and save the
id
- Create another test having as name the previous test's
id
- Get by name or id:
curl -X GET 'http://localhost:8080/api/test/byName/{name}'
- You should see that you might get wrong result
Version
What is the version of Horreum ?
0.12.2
and master
I think we should give a precedence to either name
or id
- maybe in the case the provided input string is full numeric let's give id precedence and if not found, fallback to the name.
Alternatively we could restrict Test name to be alphanumeric, this way we remove the possibility to have collision between ids and names.
Worth noting that this endpoint is not currently called by the UI, therefore I think this could be considered as low priority.