Fix escaping filter_key in prometheus output

[u5surf]

• Fixes Invalid UTF-8 label -- Prometheus Error with VTS Exporter.  #142
• it can be escaped the 2 - 4 bytes character
  in detail, please read below.

#142 (comment)

[vozlt]

I'll trust you and leave it to you. The ngx_http_vhost_traffic_status_escape_prometheus() function has been merged from this PR.

[u5surf]

@jongiddy Hi, I heard you implement this function, please you could review on this patch🙏

[jongiddy]

The Prometheus exposition format is UTF-8 encoded so valid UTF-8 characters do not need to be encoded further.

Encoding valid UTF-8 characters to percent-encoding would be surprising. Why did you choose this encoding? The format does not mention this form of escape.

There are 2 problems here:

1. \xff is an invalid UTF-8 character but ngx_utf8_decode does not detect this.
2. Even if did detect it was invalid, and it tried to use the \xff encoding that it uses for dec > 0x10ffff that encoding is not supported.

I would suggest a fix to ngx_utf8_decode to detect more invalid UTF-8 characters, including \xff. This allows valid UTF-8 to continue to be sent literally.

Change the handling of invalid UTF-8 to do something other than \x... Maybe percent-encoding is the best we can do.

[u5surf]

@jongiddy

I would suggest a fix to ngx_utf8_decode to detect more invalid UTF-8 characters,

I agree with your suggestion. Right away I send the patch to nginx-devel team.
https://mailman.nginx.org/pipermail/nginx-devel/2023-February/34K6WOSI5JSIRESP32TY73EIIUHATOM5.html

After patch merged. to follow backwards compatibility, we suppose to prepare the branches between versions sometime soon.

[jongiddy]

Great. Thanks for creating the nginx patch.

Also, I retract problem 2 in my comment above. My initial reading was that it would replace an invalid character with \xff in the exposition format, which Prometheus does not recognize.

Taking a second look, it sends two backslashes (\\xff) which is valid in the exposition format and the final text shown to users will be \xfftest-api.

Maybe %FFtest-api would have been better. It would allow the code to allocate 3 bytes instead of 5 for the remaining characters. But I'm not sure it is worth changing behavior that someone might be expecting.

[u5surf]

Maybe %FFtest-api would have been better. It would allow the code to allocate 3 bytes instead of 5 for the remaining characters. But I'm not sure it is worth changing behavior that someone might be expecting.

@jongiddy
I consider that your opinion is true in this aspect, but @axingblog @topicgit had said that the below behavior is not expecting because it can be occurred the invalid format error in promethes(#142 (comment)). I don't see wether it is true or not, once I heard to them what is happened but none of response from them(#142 (comment)). Thus they seem to hope that their character get to be encoding something expression format. So I choice the percent encoding which is adopted generally in RFC 2396 https://www.ietf.org/rfc/rfc2396.txt
Also I was hearing in these points, but they had not any response yet(#142 (comment)).

[u5surf]

@jongiddy
The nginx patch has merged and I consider that it may release next versions!
nginx/nginx@2c5fccd
For compatibility, We consider that it should return as an invalid character which starts with above 0xf8 following modified nginx decode rules. I changed this patch such that. Can you check it?

[jongiddy]

Suggested change

	if (*char_end > 0xf8 || ngx_utf8_decode(&char_end, last - pa) > 0x10ffff) {
	if (*pa >= 0xf8 || ngx_utf8_decode(&char_end, last - pa) > 0x10ffff) {

This is a good approach, but 0xf8 is invalid too, so you need to use >=.

I also suggest using *pa as this is the pointer to the current character. char_end happens to have the same value, but it has a different meaning associated with its use in the ngx_utf8_decode function.

[u5surf]

@jongiddy Thanks! I fixed it.

[jongiddy]

Same as above.

[u5surf]

@jongiddy Also fixed