yucongkoo / pe

0 stars 0 forks source link

Tampered json file will cause the application to fail from launching #2

Open yucongkoo opened 11 months ago

yucongkoo commented 11 months ago

Steps to reproduce: First get the application to produce a data file. Then, edit the data file by replacing one of the entries with null. An example is given as follow by replacing the highlighted part with null. After saving the data file, try to launch the application using java -jar.

Before:

Screenshot 2023-11-17 at 4.11.40 PM.png

After:

Screenshot 2023-11-17 at 4.12.51 PM.png

Problematic behaviour: The application crashes and fails to launch with the following error. Screenshot 2023-11-17 at 4.13.26 PM.png

nus-se-bot commented 11 months ago

Team's Response

This is a deliberate attempt to sabotage. However, we acknowledge that there should be some mechanism set in place to prevent the attempt to tamper with the files. As this is not a requirement in v1.4, we will reject it

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: > This is a deliberate attempt to sabotage.

I think as a tester, it is our responsibility to try all the nasty things on your product, even though it might not be encountered in the actual usage. As discovering potential bugs at its development phase is always better than after releasing the product. Besides, for advanced users that have programming experience, it might not be their fault to use null to represent empty stuff(hence not necessarily a deliberate sabotage, might still apply to a very small group of users)

On top of that, as of the course context, we are supposed to support the same level of manual edits to data file as the inherited AB3, where the support provided by AB3 is launch the application with an empty data file if the data file is tampered an incorrect. As your product will fail to launch, I still think that this is a functionality bug as it is not performing what it is supposed to perform. There were multiple forum discussions going on regarding this issue, with this being one of it, that shows the app should not crash even under the case where the tester deliberately used the null value. Screenshot 2023-11-21 at 1.57.14 PM.png

However, considering that this error is not totally unrecoverable(the user can still remove the whole data file/the null value to get an empty data file), I classified this as a severity.Low bug which I think is fairly reasonable.

However, we acknowledge that there should be some mechanism set in place to prevent the attempt to tamper with the files. As this is not a requirement in v1.4, we will reject it

Thank you for acknowledging this possible improvement/enhancement to the product, but I am not very sure why you decided to reject this bug rather than classifying it as not in scope(it is indeed a good to have feature in the future right?) Hence I could not agree with your decision on rejecting this bug, as rejection of a bug seems like a thing you would not consider to fix or improve.