Updated Report-name Regex (FINERACT-1156)

[thesmallstar]

Refer: https://issues.apache.org/jira/browse/FINERACT-1156

I got this issue in the email thread, no jira Ticket:

The report names with "(" threw SQL injection error, I identified the mistake to be incorrect Regex, I am not sure how this worked in the past.

Still testing this.

cc: @edcable @bharathc27

[vorburger]

I got this issue in the email thread

Which email thread? 😄

no jira Ticket:

It would be great to create a JIRA issue, describing the problem, with the error message. With text, so that it's searchable, instead of an image. It helps when we look for it again in the future, when a user reports the same error on the mailing list later.

I am not sure how this worked in the past.

It's possible that it hasn't worked in a long time and was just broken... 😅

Would it be possible to add a simple integration test for this?

[thesmallstar]

My bad :P
Let me quickly make a jira ticket for this explaining the entire issue.

[thesmallstar]

+1 for the integration test as well.

[thesmallstar]

@edcable @bharathc27 I have tested this, not getting SQL injection errors anymore, will you like to test this locally with the community app? If that looks we can add an integration test and merge this?

[edcable]

@bharathc27 have you had a chance to test yet?

[edcable]

@francisguchie are you able to test this locally from your side? @thesmallstar @vorburger is it possible to merge this so @bharathc27 could test against fineract.dev - he doesn't have a local build of Fineract running on his current device.

[thesmallstar]

@edcable Let me try to setup community app locally (Somehow it is not working), I will give an update soon. It will be better if we test and merge this.

@vorburger just an idea, can we do something to host a particular branch/PR whenever required online? It will be so much handy to test the PRs that way, should I make an issue for this?

[vorburger]

@thesmallstar @vorburger is it possible to merge this so @bharathc27 could test against fineract.dev

sure, let's just merge this as-is now. @thesmallstar you can add the IT in a follow-up PR.

@edcable Let me try to setup community app locally (Somehow it is not working), I will give an update soon. It will be better if we test and merge this.

@thesmallstar if you have some JS issue, just FYI you could technically use https://cui.fineract.dev with a baseApiUrl to localhost. But cui.fineract.dev is not auto updated - watch openMF/community-app#3309 about that.

@vorburger just an idea, can we do something to host a particular branch/PR whenever required online? It will be so much handy to test the PRs that way, should I make an issue for this?

That's... not that easy. I thought about it as well. But running a server instance costs. Running a server for every open PR is... non-negligeable. We sure could have some Bot, with some command, with some timeout that "tears down" per-PR servers, but.. this is a amount of work I don't have the time for to spend, unfortunately.

However, I think it's acceptable to sometimes merge PRs so that people can test things on demo.fineract.dev. If things are broken, then follow-up PRs can add further fixes. (The clean thing to do in theory would actually a PR with a git revert and then the proper fix.)