🐛 N°8345 - Fix double URL encoding in search URLs

[Molkobain]

Base information

Question	Answer
Related to a SourceForge thead / Another PR / Combodo ticket?	No
Type of change?	Bug fix

Symptom (bug)

Symptom only appears on some webservers, it seems to be an edge case. But the cause and fix seem legit.

When trying to open a backoffice URL pointing to a search page, the following error occurs everytime:

Service Unavailable
The server is temporarily unable to service your request due to maintenance downtime or capacity problems. Please try again later.

It happens when the URL is generated by:

• A dashlet (badge, dynamic, pie chart, bar chart, objects list)
• An objects list (search page, linkset, ...)
• An error during CSV import

Unfortunately, I can't find a more explicit error message. There is nothing in iTop's nor Apache's error logs. 🫤

Reproduction procedure (bug)

I can't find a reproduction procedure that works everywhere. It only happens on my ionos webserver on which I don't have access to the Apache / PHP configuration. 😭😭😭

1. Go on http://mbc.itop.molkobain.com/pages/UI.php
2. Log with admin / admin
3. Click on the first "objects list" dashlet subtitle "Total: 1 objects."
4. See the error occurring

P.S: Mind that I patched the badge and dynamic dashlets code in order to verify cause / fix.

Cause (bug)

As this only happens on my ionos webservers (but on all 3 of them, with different iTop instances / versions), I don't know what the system cause is.

Nonetheless, there is a issue with how search URLs are generated in the backoffice. If you look at the filter param, you'll see that it is URL encoded twice, hence the 4 hexa characters after the %.

http://mbc.itop.molkobain.com/pages/UI.php?operation=search&&filter=%255B%2522SELECT%2B%2560Rack%2560%2BFROM%2BRack%2BAS%2B%2560Rack%2560%2BWHERE%2B1%2522%252C%255B%255D%252C%255B%255D%255D&aParams=%7B%22table_id%22%3A%22DashletCUSTOM_WelcomeMenuPage_ID_row1_col0_16%22%7D

Why is it encoded twice?

• First time occurs when serializing the filter in \DBSearch::serialize()
• Second time occurs when building the URL manually, some parts of the code do something like rawurlencode($oFilter->serialize())

Changes seem to have been done during the development of the 3.0 and its UI refacto. The complexity of this refacto and its multiple iterations could explain the mixup.

Proposed solution (bug and enhancement)

• Rely on the encoding done by \DBSearch::serialize()
• Remove rawurlencode() when \DBSearch::serialize() is used (e.g. rawurlencode($oFilter->serialize()))

Mind that in some places, something like rawurlencode($oFilter->ToOQL()) is used. It could be replaced by $oFilter->serialize() for better robustness (Example). I have not made these changes, but I'll be happy to if that what you want.

Checklist before requesting a review

• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand digging in the code?

Checklist of things to do before PR is ready to merge

• Check with Combodo if they would like a unit test or if E2E is preferrable, and discuss how they would like it to be done