Correctly encode percent signs in URIs

[RubenVerborgh]

Context

Suppose we have the following template:

/{?uri}

and the following variables:

uri := http://example.org/?ref=http%3A%2F%2Fother.org%2F

So note here that the ref parameter in the URI's string is equal to http://other.org/ (URL encoded).

What we expect is that the expansion of this templates equals:

/?uri=http%3A%2F%2Fexample.org%2F%3Fref%3Dhttp%253A%252F%252Fother.org%252F

Note the double escaping for the uri parameter. This is logical, since decoding this parameter should result in the original variable value:

http://example.org/?ref=http%3A%2F%2Fother.org%2F

Bug

However, with this library, the result is:

/?uri=http%3A%2F%2Fexample.org%2F%3Fref%3Dhttp%3A%2F%2Fother.org%2F

As you can see, percent signs are not encoded. This is incorrect, since RFC6570 reads:

Note that the percent character ("%") is only allowed as part of a pct-encoded triplet and only for reserved/fragment expansion: in all other cases, a value character of "%" MUST be pct- encoded as "%25" by variable expansion

Furthermore, decoding results in:

http://example.org/?ref=http://other.org/

which is different from the original value and thus wrong.

Pull request

This pull request fixes this by:

• Adding % as a reserved character
• Fixing LiteralExpression to not decode the literal
• Fixing encodingHelper to not decode the literal

Furthermore, the tests have been updated to incorporate this issue.

[fxa]

Wow, thank you for the detailled analysis and patches. I will have to check the rfc and will make a pull request of your testcase to https://github.com/uri-templates/uritemplate-test

[RubenVerborgh]

Great! The pull request is right here: uri-templates/uritemplate-test#26

[fxa]

I have merged to fast. There are samples, where % is allowed, see template {+half}, data {"half": "50%"} should expand to "50%25" (from spec-examples-by-section.json)

[RubenVerborgh]

No, it's fine :-) A % character is indeed allowed, but it must be escaped (as your example shows). This works and is tested. The original issue was that % was not escaped if it was followed by a two-digit number, which this pull request fixes.

[RubenVerborgh]

Note that the test suite has been updated to include this test.

[fxa]

Thank you, I have seen it. I will rebase the submodule, not a trivial task with github