* module/web/http.scm (write-credentials): capitalize authorization header scheme. See, for example, https://datatracker.ietf.org/doc/html/rfc7617#section-2 --- module/web/http.scm | 4 ++-- 1 file changed, 2 insertions(+), 2 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)))) -- 2.34.1
* module/web/http.scm (write-credentials): capitalize authorization header scheme. See, for example, https://datatracker.ietf.org/doc/html/rfc7617#section-2 --- module/web/http.scm | 4 ++-- test-suite/tests/web-http.test | 7 ++++--- 2 files changed, 6 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..df25030de 100644 --- a/test-suite/tests/web-http.test +++ b/test-suite/tests/web-http.test @@ -336,9 +336,10 @@ (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-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
[-- Attachment #1: Type: text/plain, Size: 2506 bytes --] Sorry, forgot to fix tests in my original email. This is actually an important bug fix since some servers won't accept lowercase Authorization header schemes and there's no way around this in Guile, AFAIK. Here are a few RFC where they explicitly mention capitalized strings: "Basic" scheme: https://datatracker.ietf.org/doc/html/rfc7617#section-2 "Bearer" scheme: https://datatracker.ietf.org/doc/html/rfc6750#section-2.1 Aleix On Thu, Jun 23, 2022 at 1:28 PM Aleix Conchillo Flaqué <aconchillo@gmail.com> wrote: > * module/web/http.scm (write-credentials): capitalize authorization > header scheme. See, for example, > https://datatracker.ietf.org/doc/html/rfc7617#section-2 > --- > module/web/http.scm | 4 ++-- > test-suite/tests/web-http.test | 7 ++++--- > 2 files changed, 6 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..df25030de 100644 > --- a/test-suite/tests/web-http.test > +++ b/test-suite/tests/web-http.test > @@ -336,9 +336,10 @@ > (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-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 > > [-- Attachment #2: Type: text/html, Size: 4139 bytes --]
[-- Attachment #1: Type: text/plain, Size: 979 bytes --] Aleix Conchillo Flaqué schreef op do 23-06-2022 om 13:27 [-0700]: > + (put-string port (string-titlecase (symbol->string scheme))) I'd add a little explanation in a comment (e.g.: ;; 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]/> I think it's reasonable to do some changes in Guile to work-around potential bugs in other software that Guile has no control over or even knows about, at the same time Guile seems to be just following the spec, the compatibility bug seems to be in the other software, so to help the other software a bit, I think it would be best to report things in the buggy software too. Greetings, Maxime. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 260 bytes --]
[-- Attachment #1: Type: text/plain, Size: 1202 bytes --] On Thu, Jun 23, 2022 at 1:40 PM Maxime Devos <maximedevos@telenet.be> wrote: > Aleix Conchillo Flaqué schreef op do 23-06-2022 om 13:27 [-0700]: > > + (put-string port (string-titlecase (symbol->string scheme))) > > I'd add a little explanation in a comment (e.g.: > > ;; 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]/> > > I think it's reasonable to do some changes in Guile to work-around > potential bugs in other software that Guile has no control over or even > knows about, at the same time Guile seems to be just following the > spec, the compatibility bug seems to be in the other software, so to > help the other software a bit, I think it would be best to report > things in the buggy software too. > > > I think you are right. The spec says clearly that it should be case insensitive. So, disregard this patch. Aleix [-- Attachment #2: Type: text/html, Size: 2183 bytes --]
[-- Attachment #1: Type: text/plain, Size: 467 bytes --] Aleix Conchillo Flaqué schreef op do 23-06-2022 om 13:42 [-0700]: > > [...] > I think you are right. The spec says clearly that it should be case > insensitive. > > So, disregard this patch. TBC, did you encounter problems in the wild that made you write the patch? Also, there's still a potential patch to be had, e.g. you could add a test checking that Guile properly supports schemes in other cases (if not done already). Greetings, Maxime. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 260 bytes --]
[-- Attachment #1: Type: text/plain, Size: 747 bytes --] On Thu, Jun 23, 2022 at 1:45 PM Maxime Devos <maximedevos@telenet.be> wrote: > Aleix Conchillo Flaqué schreef op do 23-06-2022 om 13:42 [-0700]: > > > > [...] > > I think you are right. The spec says clearly that it should be case > > insensitive. > > > > So, disregard this patch. > > TBC, did you encounter problems in the wild that made you write the > patch? > > Yes, and I just posted it here: https://community.spotify.com/t5/Spotify-for-Developers/API-Authorization-header-doesn-t-follow-HTTP-spec/m-p/5397381#M4917 > Also, there's still a potential patch to be had, e.g. you could add a > test checking that Guile properly supports schemes in other cases (if > not done already). > > What do you mean? Aleix [-- Attachment #2: Type: text/html, Size: 2039 bytes --]
[-- Attachment #1: Type: text/plain, Size: 705 bytes --] Aleix Conchillo Flaqué schreef op do 23-06-2022 om 14:13 [-0700]: > https://community.spotify.com/t5/Spotify-for-Developers/API-Authorization-header-doesn-t-follow-HTTP-spec/m-p/5397381#M4917 > > Also, there's still a potential patch to be had, e.g. you could add > > a test checking that Guile properly supports schemes in other cases > > (if not done already). > > What do you mean? Even if there is nothing that _has_ to be done in Guile, there's still thing that _can_ be done in Guile to improve Guile's test suite -- in this case, a test in the test suite that the Guile's web code understands both lowercase and uppercase and titlecase authorisation schemes. Greetings, Maxime. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 260 bytes --]
[-- Attachment #1: Type: text/plain, Size: 1245 bytes --] On Thu, Jun 23, 2022 at 3:20 PM Maxime Devos <maximedevos@telenet.be> wrote: > Aleix Conchillo Flaqué schreef op do 23-06-2022 om 14:13 [-0700]: > > > https://community.spotify.com/t5/Spotify-for-Developers/API-Authorization-header-doesn-t-follow-HTTP-spec/m-p/5397381#M4917 > > > Also, there's still a potential patch to be had, e.g. you could add > > > a test checking that Guile properly supports schemes in other cases > > > (if not done already). > > > > What do you mean? > > Even if there is nothing that _has_ to be done in Guile, there's still > thing that _can_ be done in Guile to improve Guile's test suite -- in > this case, a test in the test suite that the Guile's web code > understands both lowercase and uppercase and titlecase authorisation > schemes. > > Ah, got it. Yes, that would make sense. I was thinking about it again. I know that Guile complies with the standard but since, I would say, capitalized schemes is what most libraries use, would it make sense to switch to that? I don't really expect big companies to fix this kind of stuff fast and in the meantime we can't use Guile for certain things. I have to say I've never seen lowercase Authorization header schemes. Best, Aleix [-- Attachment #2: Type: text/html, Size: 2445 bytes --]
[-- Attachment #1: Type: text/plain, Size: 782 bytes --] Aleix Conchillo Flaqué schreef op do 23-06-2022 om 18:46 [-0700]: > Ah, got it. Yes, that would make sense. > > I was thinking about it again. I know that Guile complies with the > standard but since, I would say, capitalized schemes is what most > libraries use, would it make sense to switch to that? I don't really > expect big companies to fix this kind of stuff fast and in the > meantime we can't use Guile for certain things. I have to say I've > never seen lowercase Authorization header schemes. As long as we remember to _also_ report the bug in the buggy ‘consumers’ of the header (in this case, those big companies) and remember that lower-case isn't buggy, switching to the more conventional title-case seems sensible to me. Greetings, Maxime. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 260 bytes --]
[-- Attachment #1: Type: text/plain, Size: 2123 bytes --] Aleix Conchillo Flaqué <aconchillo@gmail.com> writes: > On Thu, Jun 23, 2022 at 3:20 PM Maxime Devos <maximedevos@telenet.be> wrote: > > Aleix Conchillo Flaqué schreef op do 23-06-2022 om 14:13 [-0700]: > > https://community.spotify.com/t5/Spotify-for-Developers/API-Authorization-header-doesn-t-follow-HTTP-spec/m-p/5397381#M4917 > > > Also, there's still a potential patch to be had, e.g. you could add > > > a test checking that Guile properly supports schemes in other cases > > > (if not done already). > > > > What do you mean? > > Even if there is nothing that _has_ to be done in Guile, there's still > thing that _can_ be done in Guile to improve Guile's test suite -- in > this case, a test in the test suite that the Guile's web code > understands both lowercase and uppercase and titlecase authorisation > schemes. > > Ah, got it. Yes, that would make sense. > > I was thinking about it again. I know that Guile complies with the standard but since, I would say, capitalized schemes is what most libraries use, would > it make sense to switch to that? I don't really expect big companies to fix this kind of stuff fast and in the meantime we can't use Guile for certain > things. I have to say I've never seen lowercase Authorization header schemes. I think that it makes sense to ensure that Guile works with other libraries. It’s this kind of compatibility code that makes the difference between a tool that’s good in theory and one that works in practice. The robustness principle applies here:¹ Be lenient in what you accept and strict in what you send — sending the header in lowercase requires others to be lenient which cannot work. Best wishes, Arne ¹: While the robustness principle can be harmful when you’re the one mantaining the spec, because it can prevent required fixes in the spec (<https://www.ietf.org/archive/id/draft-iab-protocol-maintenance-05.html>), it applies here, because we cannot change the spec or what others accept. - Unpolitisch sein heißt politisch sein, ohne es zu merken. draketo.de [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 1125 bytes --]
[-- Attachment #1: Type: text/plain, Size: 971 bytes --] On Fri, Jun 24, 2022 at 1:35 AM Maxime Devos <maximedevos@telenet.be> wrote: > Aleix Conchillo Flaqué schreef op do 23-06-2022 om 18:46 [-0700]: > > Ah, got it. Yes, that would make sense. > > > > I was thinking about it again. I know that Guile complies with the > > standard but since, I would say, capitalized schemes is what most > > libraries use, would it make sense to switch to that? I don't really > > expect big companies to fix this kind of stuff fast and in the > > meantime we can't use Guile for certain things. I have to say I've > > never seen lowercase Authorization header schemes. > > As long as we remember to _also_ report the bug in the buggy > ‘consumers’ of the header (in this case, those big companies) and > remember that lower-case isn't buggy, switching to the more > conventional title-case seems sensible to me. > > Great. I'll re-word the patch then and add your suggested additional tests. Thank you! Aleix [-- Attachment #2: Type: text/html, Size: 1843 bytes --]