search

elise-ng / COMP4111_Project

0 stars 0 forks source link

[Bug] Able to get book with the wrong URL #11

Closed oscarngncc closed 4 years ago

oscarngncc commented 4 years ago

correct url for searching book should be /BookManagementService/books?token=. However, it's also possible to use /BookManagementService/books/30?token= as long as it is a GET request. This wrong url should only be used for loaning.

image

elise-ng commented 4 years ago

The spec did not specify that undocumented endpoints/parameters should be blocked. In this case the documented functionality of search and loaning is not affected at all.

The current behavior is in the principle that undocumented parameters (/${id} in this case) are discarded and not handled.

oscarngncc commented 4 years ago

Disagree.

The specification does explicitly specify both paths "books/id" and "books" to have different functionality, in which being able to loan and get book record respectively.

Noted that /${id} is not a parameter, but rather a path instead. Query parameters are query after "?". Able to access different functionality through the wrong path is incorrect, especially when the incorrect path is also in use.

Currently removed the invalid label for this and #12 .

Would like @comp4111ta to make the judgement

oscarngncc commented 4 years ago

Perhaps my example is confusing. But I am talking about the path, not "id=30". It just shows that how filter function still works even for the wrong path. This may give a better picture in showing how access the wrong path "/books/40" can still do what "/books" should achieve

image

elise-ng commented 4 years ago

The said “path” usage is only documented under PUT method, not GET. Functionally speaking /${id} is for specifying id of resource, and thus can be viewed as a parameter to the system. This is just a play on words.

Also, please kindly respect the rule that only developers of the repo is responsible for tagging labels.

oscarngncc commented 4 years ago

sorry for removing the tagging label. Would rather not confuse @comp4111ta since evaluation should be based on TA

elise-ng commented 4 years ago

I agree that this looks confusing, but again, the project specification does not require blocking of undocumented endpoints/usages/parameters, and this issue does not affect endpoints required by the spec at all. This issue should be ruled as out of scope.

comp4111ta commented 4 years ago

TA Verified: This is not considered as a bug. (Undefined bug by spec)