unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* [PATCH] web: send capitalized authorization header scheme
@ 2022-06-24 16:34 Aleix Conchillo Flaqué
  2022-06-24 19:51 ` Maxime Devos
  0 siblings, 1 reply; 5+ messages in thread
From: Aleix Conchillo Flaqué @ 2022-06-24 16:34 UTC (permalink / raw)
  To: guile-devel; +Cc: 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] web: send capitalized authorization header scheme
  2022-06-24 16:34 [PATCH] web: send capitalized authorization header scheme Aleix Conchillo Flaqué
@ 2022-06-24 19:51 ` Maxime Devos
  0 siblings, 0 replies; 5+ messages in thread
From: Maxime Devos @ 2022-06-24 19:51 UTC (permalink / raw)
  To: Aleix Conchillo Flaqué, guile-devel

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] web: send capitalized authorization header scheme
  2022-06-24 16:28 ` Maxime Devos
@ 2022-06-24 16:35   ` Aleix Conchillo Flaqué
  0 siblings, 0 replies; 5+ messages in thread
From: Aleix Conchillo Flaqué @ 2022-06-24 16:35 UTC (permalink / raw)
  To: Maxime Devos; +Cc: guile-devel

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] web: send capitalized authorization header scheme
  2022-06-24 16:05 Aleix Conchillo Flaqué
@ 2022-06-24 16:28 ` Maxime Devos
  2022-06-24 16:35   ` Aleix Conchillo Flaqué
  0 siblings, 1 reply; 5+ messages in thread
From: Maxime Devos @ 2022-06-24 16:28 UTC (permalink / raw)
  To: Aleix Conchillo Flaqué, guile-devel

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] web: send capitalized authorization header scheme
@ 2022-06-24 16:05 Aleix Conchillo Flaqué
  2022-06-24 16:28 ` Maxime Devos
  0 siblings, 1 reply; 5+ messages in thread
From: Aleix Conchillo Flaqué @ 2022-06-24 16:05 UTC (permalink / raw)
  To: guile-devel; +Cc: 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-06-24 19:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-24 16:34 [PATCH] web: send capitalized authorization header scheme Aleix Conchillo Flaqué
2022-06-24 19:51 ` Maxime Devos
  -- strict thread matches above, loose matches on Subject: below --
2022-06-24 16:05 Aleix Conchillo Flaqué
2022-06-24 16:28 ` Maxime Devos
2022-06-24 16:35   ` Aleix Conchillo Flaqué

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).