Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
oscarngncc opened this issue Apr 17, 2020 · 7 comments
Closed

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

oscarngncc opened this issue Apr 17, 2020 · 7 comments
Labels
invalid This doesn't seem right TA - Not a bug

Comments

@oscarngncc
Copy link

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
Copy link
Owner

elise-ng commented Apr 17, 2020

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.

@elise-ng elise-ng added the invalid This doesn't seem right label Apr 17, 2020
@oscarngncc
Copy link
Author

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 oscarngncc removed the invalid This doesn't seem right label Apr 17, 2020
@oscarngncc
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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

@elise-ng
Copy link
Owner

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right TA - Not a bug
Projects
None yet
Development

No branches or pull requests

3 participants