Do not escape $ in URI paths

[dgdavid]

Problem

When editing a repository using the "Edit Parts of the URL" mode, YaST does not handle the dollar sign properly (escaping it to "%24").

• Trello (internal link): https://trello.com/c/BnMnGsu9/
• Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1187581

Solution

Just include $ into URLRecode.PATH_SAFE_CHARS.

I evaluated (and discarded) the possibility to refactor URLRecode for making it RFC3986 compliant, but it does not pay off.

Notes

Adding changes from SP1 on

Having a look at skelcds it could be checked that $releasever was used from openSUSE Leap 15.1 on. That's why the change is going to be included in SLE-15-SP1+ branches.

Revisiting URI parts

         foo://example.com:8042/over/there?name=ferret#nose
         \_/   \______________/\_________/ \_________/ \__/
          |           |            |            |        |
       scheme     authority       path        query   fragment

Allowed chars for the URI path according to RFC3986

As the RFC states (see Appendix A for a quick view), an already percentage encoded (%\h\h) and /a-zA-Z0-9-._~!$&'()*+,;=:@ are perfectly valid chars for the URI path. This can be expressed with below regexp

/%\h{2}|[\/a-zA-Z0-9-._~!$&'()*+,;=:@]/

However, refactoring URLRecode for being compliant with RFC3986 not only when escaping the path but when doing so for userinfo and query is not as easy as it might appear at first sight. Furthermore, is not even necessary for the limited YaST URI handling so far.

Discouraging the use of URLRecode (2021-07-28)

We have added a kind of deprecation note from SLE-15-SP3 on, where a Y2Packager::ZyppUrl class was added for dealing with libzypp urls. See #1190 and #1191

Tests

• Updated existing unit tests
• Added a concrete unit test to check that $ is not being escaped by URLRecode.EscapePath
• Tested manually using an openSUSE Leap 15.3

---

MR: https://build.suse.de/request/show/245221 (after fixing a broken test in #1185)

[shundhammer]

Just a dumb question: Would it make sense to add $ to those other _SAFE_CHARS as well? Or is it really exclusively usefull for PATH_SAFE_CHARS?

[dgdavid]

According to the RFC, it is valid for query part too, but not for userinfo (part of authority). I'm simply keeping changes at minimum.

[lslezak]

Usually the variables are only used in the path part, in theory it could be included in the query but I have never seen that. For that you would need to implement some server side processing logic instead of just simply serving static files...

[shundhammer]

LGTM; but it might be safer to let this also be reviewed by some real experts on those URL specifics (if we have somebody like that).

[lslezak]

For the future I'd suggest to drop the "Edit Parts of the URL" feature completely. I guess in most cases the users just copy&paste a whole URL anyway. Building URLs manually from separate parts is a bit overkill IMHO and not used much. If you need to edit the URL you can edit whole URL, just like in a web browser...

[lslezak]

LGTM, let's keep the changes at minimum.