[PATCH] web: send capitalized authorization header scheme

[PATCH] web: send capitalized authorization header scheme

[Aleix Conchillo Flaqué]

* module/web/http.scm (write-credentials): capitalize authorization
header scheme. The standard allows the scheme to be case-insensitive,
however most libraries out there expect the scheme to be capitalized,
which is what it is actually used in RFC
docs (e.g. https://datatracker.ietf.org/doc/html/rfc7617#section-2). Some
libraries even reject lowercase scheme making Guile incompatible.
---
 module/web/http.scm            |  4 ++--
 test-suite/tests/web-http.test | 11 ++++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/module/web/http.scm b/module/web/http.scm
index 4276e1744..312c28934 100644
--- a/module/web/http.scm
+++ b/module/web/http.scm
@@ -965,10 +965,10 @@ as an ordered alist."
 (define (write-credentials val port)
   (match val
     (('basic . cred)
-     (put-string port "basic ")
+     (put-string port "Basic ")
      (put-string port cred))
     ((scheme . params)
-     (put-symbol port scheme)
+     (put-string port (string-titlecase (symbol->string scheme)))
      (put-char port #\space)
      (write-key-value-list params port))))
 
diff --git a/test-suite/tests/web-http.test b/test-suite/tests/web-http.test
index 63377349c..5c6a954b9 100644
--- a/test-suite/tests/web-http.test
+++ b/test-suite/tests/web-http.test
@@ -336,9 +336,14 @@
   (pass-if-parse authorization "Digest foooo" '(digest foooo))
   (pass-if-parse authorization "Digest foo=bar,baz=qux"
                  '(digest (foo . "bar") (baz . "qux")))
-  (pass-if-round-trip "Authorization: basic foooo\r\n")
-  (pass-if-round-trip "Authorization: digest foooo\r\n")
-  (pass-if-round-trip "Authorization: digest foo=bar, baz=qux\r\n")
+  (pass-if-parse authorization "basic foooo" '(basic . "foooo"))
+  (pass-if-parse authorization "digest foooo" '(digest foooo))
+  (pass-if-parse authorization "digest foo=bar,baz=qux"
+                 '(digest (foo . "bar") (baz . "qux")))
+  (pass-if-round-trip "Authorization: Basic foooo\r\n")
+  (pass-if-round-trip "Authorization: Bearer token\r\n")
+  (pass-if-round-trip "Authorization: Digest foooo\r\n")
+  (pass-if-round-trip "Authorization: Digest foo=bar, baz=qux\r\n")
   (pass-if-parse expect "100-continue, foo" '((100-continue) (foo)))
   (pass-if-parse from "foo@bar" "foo@bar")
   (pass-if-parse host "qux" '("qux" . #f))
-- 
2.34.1

Re: [PATCH] web: send capitalized authorization header scheme

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]

Aleix Conchillo Flaqué schreef op vr 24-06-2022 om 09:05 [-0700]:
> * module/web/http.scm (write-credentials): capitalize authorization
> header scheme. The standard allows the scheme to be case-insensitive,
> however most libraries out there expect the scheme to be capitalized,
> which is what it is actually used in RFC
> docs (e.g. https://datatracker.ietf.org/doc/html/rfc7617#section-2). Some
> libraries even reject lowercase scheme making Guile incompatible.

This comment looks more useful to me to put in the source code, to help
future readers of the source code, otherwise they would have to dig
through the git history.  As mentioned previously, this could be
something like:

   ;; While according to RFC 7617 Schemes are case-insensitive:
   ;;
   ;; ‘Note that both scheme and parameter names are matched
   ;; case-insensitive’
   ;;
   ;; some software (*) incorrectly assumes title case for scheme
   ;; names, so use the more titlecase.
   ;;
   ;; (*): See, e.g.,
   ;; <https://[bug report 1]/>
   ;; <https://[bug report 2]/>

which would also address the issue of not forgetting that Guile's old
behaviour is correct, it's the other party that's not following the
specification.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[PATCH] web: send capitalized authorization header scheme

[Aleix Conchillo Flaqué]

* module/web/http.scm (write-credentials): capitalize authorization
header scheme. The standard allows the scheme to be case-insensitive,
however most libraries out there expect the scheme to be capitalized,
which is what it is actually used in RFC
docs (e.g. https://datatracker.ietf.org/doc/html/rfc7617#section-2). Some
libraries even reject lowercase scheme making Guile incompatible.
---
 module/web/http.scm            | 14 ++++++++++++--
 test-suite/tests/web-http.test | 11 ++++++++---
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/module/web/http.scm b/module/web/http.scm
index 4276e1744..6af790384 100644
--- a/module/web/http.scm
+++ b/module/web/http.scm
@@ -962,13 +962,23 @@ as an ordered alist."
     (((? symbol?) . (? key-value-list?)) #t)
     (_ #f)))
 
+;; While according to RFC 7617 Schemes are case-insensitive:
+;;
+;; 'Note that both scheme and parameter names are matched
+;; case-insensitive'
+;;
+;; some software (*) incorrectly assumes title case for scheme
+;; names, so use the more titlecase.
+;;
+;; (*): See, e.g.,
+;; https://community.spotify.com/t5/Spotify-for-Developers/API-Authorization-header-doesn-t-follow-HTTP-spec/m-p/5397381#M4917
 (define (write-credentials val port)
   (match val
     (('basic . cred)
-     (put-string port "basic ")
+     (put-string port "Basic ")
      (put-string port cred))
     ((scheme . params)
-     (put-symbol port scheme)
+     (put-string port (string-titlecase (symbol->string scheme)))
      (put-char port #\space)
      (write-key-value-list params port))))
 
diff --git a/test-suite/tests/web-http.test b/test-suite/tests/web-http.test
index 63377349c..5c6a954b9 100644
--- a/test-suite/tests/web-http.test
+++ b/test-suite/tests/web-http.test
@@ -336,9 +336,14 @@
   (pass-if-parse authorization "Digest foooo" '(digest foooo))
   (pass-if-parse authorization "Digest foo=bar,baz=qux"
                  '(digest (foo . "bar") (baz . "qux")))
-  (pass-if-round-trip "Authorization: basic foooo\r\n")
-  (pass-if-round-trip "Authorization: digest foooo\r\n")
-  (pass-if-round-trip "Authorization: digest foo=bar, baz=qux\r\n")
+  (pass-if-parse authorization "basic foooo" '(basic . "foooo"))
+  (pass-if-parse authorization "digest foooo" '(digest foooo))
+  (pass-if-parse authorization "digest foo=bar,baz=qux"
+                 '(digest (foo . "bar") (baz . "qux")))
+  (pass-if-round-trip "Authorization: Basic foooo\r\n")
+  (pass-if-round-trip "Authorization: Bearer token\r\n")
+  (pass-if-round-trip "Authorization: Digest foooo\r\n")
+  (pass-if-round-trip "Authorization: Digest foo=bar, baz=qux\r\n")
   (pass-if-parse expect "100-continue, foo" '((100-continue) (foo)))
   (pass-if-parse from "foo@bar" "foo@bar")
   (pass-if-parse host "qux" '("qux" . #f))
-- 
2.34.1

Re: [PATCH] web: send capitalized authorization header scheme

[Aleix Conchillo Flaqué]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

On Fri, Jun 24, 2022 at 9:28 AM Maxime Devos <maximedevos@telenet.be> wrote:

> Aleix Conchillo Flaqué schreef op vr 24-06-2022 om 09:05 [-0700]:
> > * module/web/http.scm (write-credentials): capitalize authorization
> > header scheme. The standard allows the scheme to be case-insensitive,
> > however most libraries out there expect the scheme to be capitalized,
> > which is what it is actually used in RFC
> > docs (e.g. https://datatracker.ietf.org/doc/html/rfc7617#section-2).
> Some
> > libraries even reject lowercase scheme making Guile incompatible.
>
> This comment looks more useful to me to put in the source code, to help
> future readers of the source code, otherwise they would have to dig
> through the git history.  As mentioned previously, this could be
> something like:
>
>    ;; While according to RFC 7617 Schemes are case-insensitive:
>    ;;
>    ;; ‘Note that both scheme and parameter names are matched
>    ;; case-insensitive’
>    ;;
>    ;; some software (*) incorrectly assumes title case for scheme
>    ;; names, so use the more titlecase.
>    ;;
>    ;; (*): See, e.g.,
>    ;; <https://[bug report 1]/>
>    ;; <https://[bug report 2]/>
>
> which would also address the issue of not forgetting that Guile's old
> behaviour is correct, it's the other party that's not following the
> specification.
>
>
This makes sense. I left the comment in the commit log as well. Sent again.

Thank you!

Aleix

[-- Attachment #2: Type: text/html, Size: 2496 bytes --]

Re: [PATCH] web: send capitalized authorization header scheme

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

Aleix Conchillo Flaqué schreef op vr 24-06-2022 om 09:34 [-0700]:
> * module/web/http.scm (write-credentials): capitalize authorization
> header scheme. The standard allows the scheme to be case-insensitive,
> however most libraries out there expect the scheme to be capitalized,
> which is what it is actually used in RFC
> docs (e.g. https://datatracker.ietf.org/doc/html/rfc7617#section-2). Some
> libraries even reject lowercase scheme making Guile incompatible.
> ---

Patch LGTM (untested, though probably good!), though I want to remind
you I'm not a maintainer, I just sometimes review things on guile-
devel@, so I can't commit it to savannah.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]