Re: PATCH: Add uri percent-encoding for binary data
> On 1 Jul 2020, at 16:58, Alvaro Herrera wrote: > > On 2020-Jul-01, Daniel Gustafsson wrote: > >>> On 19 Mar 2020, at 08:55, Daniel Gustafsson wrote: >> >>> With no response for 2 weeks during the commitfest, I propose to move this >>> to >>> the next CF to allow time for discussions. >> >> This patch no longer applies, the failing hunk being in the docs part. As >> stated in my review earlier in the thread I don't think this feature is >> complete enough in its current form; having hacked on it a bit, what are your >> thoughts Alvaro? > > If the author (or some other person interested in the feature) submits a > version addressing the feedback, by all means let's consider it further; > but if nothing happens during this commitfest, I'd say we close as RwF > at end of July. As per discussion, this entry is closed as "Returned with Feedback". cheers ./daniel
Re: PATCH: Add uri percent-encoding for binary data
On 2020-Jul-01, Daniel Gustafsson wrote: > > On 19 Mar 2020, at 08:55, Daniel Gustafsson wrote: > > > With no response for 2 weeks during the commitfest, I propose to move this > > to > > the next CF to allow time for discussions. > > This patch no longer applies, the failing hunk being in the docs part. As > stated in my review earlier in the thread I don't think this feature is > complete enough in its current form; having hacked on it a bit, what are your > thoughts Alvaro? If the author (or some other person interested in the feature) submits a version addressing the feedback, by all means let's consider it further; but if nothing happens during this commitfest, I'd say we close as RwF at end of July. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: PATCH: Add uri percent-encoding for binary data
> On 19 Mar 2020, at 08:55, Daniel Gustafsson wrote: > With no response for 2 weeks during the commitfest, I propose to move this to > the next CF to allow time for discussions. This patch no longer applies, the failing hunk being in the docs part. As stated in my review earlier in the thread I don't think this feature is complete enough in its current form; having hacked on it a bit, what are your thoughts Alvaro? Marking as Waiting on Author for now. cheers ./daniel
Re: PATCH: Add uri percent-encoding for binary data
> On 4 Mar 2020, at 12:25, Daniel Gustafsson wrote: > >> On 20 Feb 2020, at 23:27, Alvaro Herrera wrote: >> >> On 2019-Oct-07, Anders Åstrand wrote: >> >>> Attached is a patch for adding uri as an encoding option for >>> encode/decode. It uses what's called "percent-encoding" in rfc3986 >>> (https://tools.ietf.org/html/rfc3986#section-2.1). >> >> Thanks. Seems useful. I made a few cosmetic tweaks and it looks almost >> ready to me; > > I agree that uri decoding/encoding would be useful, but I'm not convinced that > this patch does the functionality justice enough to be useful. What is the > usecase we envision to solve when not taking scheme into consideration? > > Reserved characters have different meaning based on context and scheme, and > should not be encoded when used as a delimiter. This does make the patch a > lot > more complicated, but if we provide a uri encoding which percent-encode the > delimiters in https:// I would expect that to be reported to pgsql-bugs@ > repeatedly. Adding URIs with userinfo makes it even more problematic, as > encoding the @ delimiter will break it. > > Further, RFC6874 specifies that ipv6 URIs with zone identifiers are written > as: > IPv6address "%25" ZoneID. With this patch it would be encoded %2525 ZoneID > which is incorrect. > > That being said, if we do look at the scheme then we'll need to decide which > URI standard we want to stick to as RFC3986 and WHATWG URL-spec aren't > compatible. > > Perhaps not calling it 'uri' and instead renaming it to 'percent-encoding' can > make it clearer, while sticking to the proposed feature? With no response for 2 weeks during the commitfest, I propose to move this to the next CF to allow time for discussions. cheers ./daniel
Re: PATCH: Add uri percent-encoding for binary data
> On 20 Feb 2020, at 23:27, Alvaro Herrera wrote: > > On 2019-Oct-07, Anders Åstrand wrote: > >> Attached is a patch for adding uri as an encoding option for >> encode/decode. It uses what's called "percent-encoding" in rfc3986 >> (https://tools.ietf.org/html/rfc3986#section-2.1). > > Thanks. Seems useful. I made a few cosmetic tweaks and it looks almost > ready to me; I agree that uri decoding/encoding would be useful, but I'm not convinced that this patch does the functionality justice enough to be useful. What is the usecase we envision to solve when not taking scheme into consideration? Reserved characters have different meaning based on context and scheme, and should not be encoded when used as a delimiter. This does make the patch a lot more complicated, but if we provide a uri encoding which percent-encode the delimiters in https:// I would expect that to be reported to pgsql-bugs@ repeatedly. Adding URIs with userinfo makes it even more problematic, as encoding the @ delimiter will break it. Further, RFC6874 specifies that ipv6 URIs with zone identifiers are written as: IPv6address "%25" ZoneID. With this patch it would be encoded %2525 ZoneID which is incorrect. That being said, if we do look at the scheme then we'll need to decide which URI standard we want to stick to as RFC3986 and WHATWG URL-spec aren't compatible. Perhaps not calling it 'uri' and instead renaming it to 'percent-encoding' can make it clearer, while sticking to the proposed feature? cheers ./daniel
Re: PATCH: Add uri percent-encoding for binary data
Thanks for keeping this alive even though I disappeared after submitting it!
I can write documentation this weekend.
Thanks again.
//Anders
On Thu, 20 Feb 2020, 23:28 Alvaro Herrera, wrote:
> On 2019-Oct-07, Anders Åstrand wrote:
>
> > Attached is a patch for adding uri as an encoding option for
> > encode/decode. It uses what's called "percent-encoding" in rfc3986
> > (https://tools.ietf.org/html/rfc3986#section-2.1).
>
> Thanks. Seems useful. I made a few cosmetic tweaks and it looks almost
> ready to me; however, documentation is missing. I added a stub; can you
> please complete that?
>
> To answer Arthur Zakirov's question: yes, the standard recommends
> ("should") to use uppercase characters:
>
> : For consistency, URI producers and
> : normalizers should use uppercase hexadecimal digits for all percent-
> : encodings.
>
> Thanks,
>
> --
> Álvaro Herrerahttps://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
Re: PATCH: Add uri percent-encoding for binary data
On 2019-Oct-07, Anders Åstrand wrote:
> Attached is a patch for adding uri as an encoding option for
> encode/decode. It uses what's called "percent-encoding" in rfc3986
> (https://tools.ietf.org/html/rfc3986#section-2.1).
Thanks. Seems useful. I made a few cosmetic tweaks and it looks almost
ready to me; however, documentation is missing. I added a stub; can you
please complete that?
To answer Arthur Zakirov's question: yes, the standard recommends
("should") to use uppercase characters:
: For consistency, URI producers and
: normalizers should use uppercase hexadecimal digits for all percent-
: encodings.
Thanks,
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 44475f709762ba1a2a881d20345cc6a4cb086f01 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
Date: Thu, 20 Feb 2020 18:46:15 -0300
Subject: [PATCH v2] URI encode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Author: Anders Åstrand
Discussion: https://postgr.es/m/APwPebtwJnjjt=euusml1zz6w3jvna1cvjezhbouccytjc9...@mail.gmail.com
---
doc/src/sgml/func.sgml| 16 +++-
src/backend/utils/adt/encode.c| 129 ++
src/test/regress/expected/strings.out | 21 +
src/test/regress/sql/strings.sql | 7 ++
4 files changed, 172 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ceda48e0fc..c60ad4f4e2 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -3180,7 +3180,8 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
format values are:
base64,
escape,
- hex
+ hex,
+ uri
encode('123\000\001', 'base64')
MTIzAAE=
@@ -3274,6 +3275,19 @@ SELECT format('Testing %3$s, %2$s, %s', 'one', 'two', 'three');
+
+
+ uri
+
+ uri format
+
+
+
+ The uri format represents ...
+
+
+
+
diff --git a/src/backend/utils/adt/encode.c b/src/backend/utils/adt/encode.c
index b8d9ec7e00..81d4ea8400 100644
--- a/src/backend/utils/adt/encode.c
+++ b/src/backend/utils/adt/encode.c
@@ -110,6 +110,7 @@ binary_decode(PG_FUNCTION_ARGS)
*/
static const char hextbl[] = "0123456789abcdef";
+static const char hextbl_upper[] = "0123456789ABCDEF";
static const int8 hexlookup[128] = {
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
@@ -512,6 +513,128 @@ esc_dec_len(const char *src, unsigned srclen)
return len;
}
+/*
+ * URI percent encoding
+ *
+ * Percent encodes all byte values except the unreserved ASCII characters as
+ * per RFC3986.
+ */
+
+static unsigned
+uri_encode(const char *src, unsigned srclen, char *dst)
+{
+ char *d = dst;
+
+ for (const char *s = src; s < src + srclen; s++)
+ {
+ /*
+ * RFC3986:
+ *
+ * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
+ */
+ if ((*s >= 'A' && *s <= 'Z') ||
+ (*s >= 'a' && *s <= 'z') ||
+ (*s >= '0' && *s <= '9') ||
+ *s == '-' ||
+ *s == '.' ||
+ *s == '_' ||
+ *s == '~')
+ {
+ *d++ = *s;
+ }
+ else
+ {
+ *d++ = '%';
+ *d++ = hextbl_upper[(*s >> 4) & 0xF];
+ *d++ = hextbl_upper[*s & 0xF];
+ }
+ }
+ return d - dst;
+}
+
+static unsigned
+uri_decode(const char *src, unsigned srclen, char *dst)
+{
+ const char *s = src;
+ const char *srcend = src + srclen;
+ char *d = dst;
+ char val;
+
+ while (s < srcend)
+ {
+ if (*s == '%')
+ {
+ /*
+ * Verify we have the needed bytes. This doesn't happen, since
+ * uri_dec_len already takes care of validation.
+ */
+ if (s > srcend - 3)
+elog(ERROR, "invalid uri percent encoding");
+
+ /* Skip '%' */
+ s++;
+
+ val = get_hex(*s++) << 4;
+ val += get_hex(*s++);
+ *d++ = val;
+ }
+ else
+ *d++ = *s++;
+ }
+ return d - dst;
+}
+
+static unsigned
+uri_enc_len(const char *src, unsigned srclen)
+{
+ int len = 0;
+
+ for (const char *s = src; s < src + srclen; s++)
+ {
+ if ((*s >= 'A' && *s <= 'Z') ||
+ (*s >= 'a' && *s <= 'z') ||
+ (*s >= '0' && *s <= '9') ||
+ *s == '-' ||
+ *s == '_' ||
+ *s == '.' ||
+ *s == '~')
+ {
+ len++;
+ }
+ else
+ len += 3;
+ }
+ return len;
+}
+
+static unsigned
+uri_dec_len(const char *src, unsigned srclen)
+{
+ const char *s = src;
+ const char *srcend = src + srclen;
+ int len = 0;
+
+ while (s < srcend)
+ {
+ if (*s == '%')
+ {
+ if (s > srcend - 3)
+ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid uri percent encoding"),
+ errhint("Input data ends prematurely.")));
+ s++;
+ get_hex(*s++);
+ get_hex(*s++);
+ }
+ else
+ s++;
+ len++;
+ }
+
+ return len;
+}
+
/*
* Common
*/
@@ -541,6 +664,12 @@ static const struct
esc_enc_len, esc_dec_len, esc_encode, esc_decode
}
},
+ {
+ "uri",
+ {
+ uri_enc_len, uri_dec_len, uri_encode, uri_decode
+ }
+ },
{
NULL,
{
Re: PATCH: Add uri percent-encoding for binary data
Hello, On 2019/10/07 16:14, Anders Åstrand wrote: Hello Attached is a patch for adding uri as an encoding option for encode/decode. It uses what's called "percent-encoding" in rfc3986 (https://tools.ietf.org/html/rfc3986#section-2.1). Thank you for the patch. I'm not very familiar with rfc3986. Is it insist that an output should have upper case characters? If not maybe it is good to reuse hextbl[] (which is in encode.c) instead of adding new upper_hex_digits[]. Also can you correct the documentation. encode() is mentioned here: https://www.postgresql.org/docs/current/functions-binarystring.html -- Arthur
Re: PATCH: Add uri percent-encoding for binary data
On Mon, Oct 7, 2019 at 11:38 PM Isaac Morland wrote:
>
> On Mon, 7 Oct 2019 at 03:15, Anders Åstrand wrote:
>>
>> Hello
>>
>> Attached is a patch for adding uri as an encoding option for
>> encode/decode. It uses what's called "percent-encoding" in rfc3986
>> (https://tools.ietf.org/html/rfc3986#section-2.1).
>>
>> The background for this patch is that I could easily build urls in
>> plpgsql, but doing the actual encoding of the url parts is painfully
>> slow. The list of available encodings for encode/decode looks quite
>> arbitrary to me, so I can't see any reason this one couldn't be in
>> there.
>>
>> In modern web scenarios one would probably most likely want to encode
>> the utf8 representation of a text string for inclusion in a url, in
>> which case correct invocation would be ENCODE(CONVERT_TO('some text in
>> database encoding goes here', 'UTF8'), 'uri'), but uri
>> percent-encoding can of course also be used for other text encodings
>> and arbitrary binary data.
>
>
> This seems like a useful idea to me. I've used the equivalent in Python and
> it provides more options:
>
> https://docs.python.org/3/library/urllib.parse.html#url-quoting
>
> I suggest reviewing that documentation there, because there are a few details
> that need to be checked carefully. Whether or not space should be encoded as
> plus and whether certain byte values should be exempt from %-encoding is
> something that depends on the application. Unfortunately, as far as I can
> tell there isn't a single version of URL encoding that satisfies all
> situations (thus explaining the complexity of the Python implementation). It
> might be feasible to suppress some of the Python options (I'm wondering about
> the safe= parameter) but I'm pretty sure you at least need the equivalent of
> quote and quote_plus.
Thanks a lot for your reply!
I agree that some (but not all) of the options available to that
python lib could be helpful for developers wanting to build urls
without having to encode the separate parts of it and stitching it
together, but not necessary for this patch to be useful. For generic
uri encoding the slash (/) must be percent encoded, because it has
special meaning in the standard. Some other extra characters may
appear unencoded though depending on context, but it's generally safer
to just encode them all and not hope that the encoder will know about
the context and skip over certain characters.
This does bring up an interesting point however. Maybe decode should
validate that only characters that are allowed unencoded appear in the
input?
Luckily, the plus-encoding of spaces are not part of the uri standard
at all but instead part of the format referred to as
application/x-www-form-urlencoded data. Fortunately that format is
close to dying now that forms more often post json.
Regards,
Anders
Re: PATCH: Add uri percent-encoding for binary data
On Mon, Oct 7, 2019 at 9:52 PM Bruce Momjian wrote: > > On Mon, Oct 7, 2019 at 09:14:38AM +0200, Anders Åstrand wrote: > > Hello > > > > Attached is a patch for adding uri as an encoding option for > > encode/decode. It uses what's called "percent-encoding" in rfc3986 > > (https://tools.ietf.org/html/rfc3986#section-2.1). > > Oh, that's a cool idea. Can you add it to the commit-fest? > > https://commitfest.postgresql.org/25/ > > Thanks for your reply! I added it but was unsure of what topic was appropriate and couldn't find a description of them anywhere. I went with Miscellaneous for now.
Re: PATCH: Add uri percent-encoding for binary data
On Mon, 7 Oct 2019 at 03:15, Anders Åstrand wrote:
> Hello
>
> Attached is a patch for adding uri as an encoding option for
> encode/decode. It uses what's called "percent-encoding" in rfc3986
> (https://tools.ietf.org/html/rfc3986#section-2.1).
>
> The background for this patch is that I could easily build urls in
> plpgsql, but doing the actual encoding of the url parts is painfully
> slow. The list of available encodings for encode/decode looks quite
> arbitrary to me, so I can't see any reason this one couldn't be in
> there.
>
> In modern web scenarios one would probably most likely want to encode
> the utf8 representation of a text string for inclusion in a url, in
> which case correct invocation would be ENCODE(CONVERT_TO('some text in
> database encoding goes here', 'UTF8'), 'uri'), but uri
> percent-encoding can of course also be used for other text encodings
> and arbitrary binary data.
>
This seems like a useful idea to me. I've used the equivalent in Python and
it provides more options:
https://docs.python.org/3/library/urllib.parse.html#url-quoting
I suggest reviewing that documentation there, because there are a few
details that need to be checked carefully. Whether or not space should be
encoded as plus and whether certain byte values should be exempt from
%-encoding is something that depends on the application. Unfortunately, as
far as I can tell there isn't a single version of URL encoding that
satisfies all situations (thus explaining the complexity of the Python
implementation). It might be feasible to suppress some of the Python
options (I'm wondering about the safe= parameter) but I'm pretty sure you
at least need the equivalent of quote and quote_plus.
Re: PATCH: Add uri percent-encoding for binary data
On Mon, Oct 7, 2019 at 09:14:38AM +0200, Anders Åstrand wrote:
> Hello
>
> Attached is a patch for adding uri as an encoding option for
> encode/decode. It uses what's called "percent-encoding" in rfc3986
> (https://tools.ietf.org/html/rfc3986#section-2.1).
Oh, that's a cool idea. Can you add it to the commit-fest?
https://commitfest.postgresql.org/25/
---
>
> The background for this patch is that I could easily build urls in
> plpgsql, but doing the actual encoding of the url parts is painfully
> slow. The list of available encodings for encode/decode looks quite
> arbitrary to me, so I can't see any reason this one couldn't be in
> there.
>
> In modern web scenarios one would probably most likely want to encode
> the utf8 representation of a text string for inclusion in a url, in
> which case correct invocation would be ENCODE(CONVERT_TO('some text in
> database encoding goes here', 'UTF8'), 'uri'), but uri
> percent-encoding can of course also be used for other text encodings
> and arbitrary binary data.
>
> Regards,
> Anders
> diff --git a/src/backend/utils/adt/encode.c b/src/backend/utils/adt/encode.c
> index 7293d66de5..33cf7bb57c 100644
> --- a/src/backend/utils/adt/encode.c
> +++ b/src/backend/utils/adt/encode.c
> @@ -512,6 +512,131 @@ esc_dec_len(const char *src, unsigned srclen)
> return len;
> }
>
> +/*
> + * URI percent encoding
> + *
> + * Percent encodes all byte values except the unreserved ASCII characters as
> per RFC3986.
> + */
> +
> +static const char upper_hex_digits[] = "0123456789ABCDEF";
> +
> +static unsigned
> +uri_encode(const char *src, unsigned srclen, char *dst)
> +{
> + char*d = dst;
> +
> + for (const char *s = src; s < src + srclen; s++)
> + {
> + if ((*s >= 'A' && *s <= 'Z') ||
> + (*s >= 'a' && *s <= 'z') ||
> + (*s >= '0' && *s <= '9') ||
> + *s == '-' ||
> + *s == '_' ||
> + *s == '.' ||
> + *s == '~')
> + {
> + *d++ = *s;
> + }
> + else
> + {
> + *d++ = '%';
> + *d++ = upper_hex_digits[(*s >> 4) & 0xF];
> + *d++ = upper_hex_digits[*s & 0xF];
> + }
> + }
> + return d - dst;
> +}
> +
> +static unsigned
> +uri_decode(const char *src, unsigned srclen, char *dst)
> +{
> + const char *s = src;
> + const char *srcend = src + srclen;
> + char*d = dst;
> + charval;
> +
> + while (s < srcend)
> + {
> + if (*s == '%')
> + {
> + if (s > srcend - 3) {
> + /* This will never get triggered since
> uri_dec_len already takes care of validation
> + */
> + ereport(ERROR,
> +
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("invalid uri percent
> encoding"),
> + errhint("Input data ends
> prematurely.")));
> + }
> +
> + /* Skip '%' */
> + s++;
> +
> + val = get_hex(*s++) << 4;
> + val += get_hex(*s++);
> + *d++ = val;
> + }
> + else
> + {
> + *d++ = *s++;
> + }
> + }
> + return d - dst;
> +}
> +
> +static unsigned
> +uri_enc_len(const char *src, unsigned srclen)
> +{
> + int len = 0;
> +
> + for (const char *s = src; s < src + srclen; s++)
> + {
> + if ((*s >= 'A' && *s <= 'Z') ||
> + (*s >= 'a' && *s <= 'z') ||
> + (*s >= '0' && *s <= '9') ||
> + *s == '-' ||
> + *s == '_' ||
> + *s == '.' ||
> + *s == '~')
> + {
> + len++;
> + }
> + else
> + {
> + len += 3;
> + }
> + }
> + return len;
> +}
> +
> +static unsigned
> +uri_dec_len(const char *src, unsigned srclen)
> +{
> + const char *s = src;
> + const char *srcend = src + srclen;
> + int len = 0;
> +
> + while (s < srcend)
> + {
> + if (*s == '%')
> + {
> + if (s > srcend - 3) {
> + ereport(ERROR,
> +
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("invalid uri percent
> encoding"),
> + errhint("Input data ends
>
